Reimplement icon component as SVG image mask#10065
Merged
Conversation
zachmargolis
reviewed
Feb 9, 2024
Contributor
There was a problem hiding this comment.
What if we used ActiveSupport's String#squish to let us author it with line breaks for readability but remove them for serialization?
Suggested change
| <%= "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); mask-size: 100%; -webkit-mask-size: 100%; background-color: currentColor; }" %> | |
| <%= <<-STR.squish | |
| #icon-#{unique_id} { | |
| mask-image: url(#{icon_path}); | |
| -webkit-mask-image: url(#{icon_path}); | |
| mask-size: 100%; | |
| -webkit-mask-size: 100%; | |
| background-color: currentColor; | |
| } | |
| STR | |
| %> |
Contributor
Author
There was a problem hiding this comment.
This motivated me toward #10067, which I'm hoping can fix the issue with component stylesheets so we can move the mask-size and background-color properties out and shorten it to something more like this, which hopefully reduces the readability concern a bit?
Suggested change
| <%= "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); mask-size: 100%; -webkit-mask-size: 100%; background-color: currentColor; }" %> | |
| <%= "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); }" %> |
Contributor
There was a problem hiding this comment.
I think it still might be nice with some line breaks, but that is a little clearer
This was referenced Feb 9, 2024
changelog: Internal, Performance, Reduce download size for icon images
Multiple calls per render
bd22a18 to
407968d
Compare
Previous assertion used XPath, where XPath text matchers don't correctly ignore inline style tag contents
Since it's already rendered as simple string, pass as argument in content_tag. Also avoids excess whitespace
For clarity, avoid ambiguity with "and" keyword Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>
Significant for use in ButtonComponent
zachmargolis
approved these changes
Feb 15, 2024
Contributor
Author
aduth
added a commit
that referenced
this pull request
Feb 20, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


🛠 Summary of changes
Updates
IconComonent's rendering of icons to reference the original SVG icon, colorized using a combination ofmask-imageandbackground-color: currentColor.This is related to the comment at #10056 (comment) .
USWDS's icon sprite has an advantage in being able to inherit the current text color of content and apply it to the SVG image, which cannot be done by default with an
<img>referencing an SVG resource. The approach here uses the CSSmask-imageproperty to emulate this effect.The goal is to try to avoid wasted data sent from the sprite for icons which aren't used in a page. The sprite image is ~22kb after compression, so it's not catastrophically large, but it's relatively quite large when not already present in a page. By comparison, the introduction of the "language" icon is about 0.5kb compressed, which is actually a net reduction of about 2kb compared to the current
globe-white.svgandglobe-blue.svgcurrently used in the footer.Notes on a few quirks / alternatives:
mask-image..usa-icon--icon-language, wheremask-imageis assigned for this. Since there's a lot of icons, this would become quite large, and include data for icons not in use.data-attributeattr()function, but there is not good widespread browser support for this outside thecontentstyle<svg><use>like the current icon markup, but referencing the icon directly. This will apparently be possible in SVG2 ("Allow 'use' to reference an external document's root element by omitting the fragment"), but this doesn't appear to be standardized yet or available in current browsers.There's a few non-"dynamic" styles which are also inlined, forEdit: This was fixed separately in Update base layout to be base-ier #10067mask-sizeandbackground-color. Ideally these would be extracted to a stylesheet associated with the component, but there's an existing issue where the automatic component asset loading doesn't work for components rendered in thebase.html.erbbase layout, due to howrender_stylesheet_once_tagsis called before components in the base layout are rendered.mask-imagewas only recently made available as standard (without vendor prefix) in Chrome 120 (12/2023), so the vendor prefixes need to be included here. For any of our standard stylesheets, this is handled automatically through our@18f/identity-build-sassbuild toolchain (Lightning CSS vendor prefixing)📜 Testing Plan
Verify there are no regressions in any place we currently render an icon using
IconComponent, e.g.👀 Screenshots
The visual differences here come largely from the differences between
globe-white.svgand the design systemlanguageicon.