Skip to content

Add IconListComponent ViewComponent#9504

Merged
aduth merged 5 commits intomainfrom
aduth-icon-list-component
Nov 3, 2023
Merged

Add IconListComponent ViewComponent#9504
aduth merged 5 commits intomainfrom
aduth-icon-list-component

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Nov 1, 2023

🛠 Summary of changes

Adds a new IconListComponent component implementation.

Why?

  • Improve developer experience in using a design system component
  • Reduce size of main application stylesheet to improve performance
  • As a refactoring ahead of planned revisions to consent screen to remove ad-hoc icon list implementation

📜 Testing Plan

Repeat testing instructions from #8863 to verify no regressions in the display of "Your authentication methods".

See component preview:

aduth added 3 commits November 1, 2023 09:38
changelog: Internal, Performance, Reduce size of application stylesheet
Makefile Outdated
Comment on lines 118 to 123
# This enforces an asset size budget to ensure that download sizes are reasonable and to protect
# against accidentally importing large pieces of third-party libraries. If you're here debugging a
# failing build, check to ensure that you've not added more JavaScript or CSS than necessary, and
# you have no options to split that from the main bundles. If you need to increase this budget and
# accept the fact that this will force end-users to endure longer load times, you should set the new
# budget to within a few thousand bytes of the current production-compiled size.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect we indent this comment? Like in the optimize_svg task?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect we indent this comment? Like in the optimize_svg task?

Hmm for some reason I didn't think Make liked the syntax, but I see what you're saying about optimize_svg. Let me try again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a tabs vs spaces issue? Makefiles are sensitive to the difference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, yeah, it's quite likely it was a spacing issue. That or I didn't like how it would output the comment to the terminal (not a huge deal, but 🤷 ). It's not pretty with all the @, but 4bb1045 updates so that it's indented and doesn't output the comment.

Comment on lines 2 to 4
renders_many :items, ->(**kwargs, &block) {
IconListItemComponent.new(icon:, color:, **kwargs, &block)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we mostly use do....end for multi-line blocks and I confirmed with ruby-parse that this gives us the same results (sometimes ommitting parens means the block gets passed to renders_many instead of to the proc)

Suggested change
renders_many :items, ->(**kwargs, &block) {
IconListItemComponent.new(icon:, color:, **kwargs, &block)
}
renders_many :items, ->(**kwargs, &block) do
IconListItemComponent.new(icon:, color:, **kwargs, &block)
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah agreed 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we've got the do ... end syntax in StatusPageComponent, so I should expect that will work. I admit that I largely copied from ProcessListComponent for this, which uses the curly braces.

In 1f71d2e, I've updated them all to use the same multiline syntax.

@aduth aduth merged commit df17dd2 into main Nov 3, 2023
@aduth aduth deleted the aduth-icon-list-component branch November 3, 2023 12:08
@matthinz matthinz mentioned this pull request Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants