Skip to content

Remove nonstandard and redundant icons for government banner#10275

Merged
aduth merged 2 commits intomainfrom
aduth-rm-lock
Mar 21, 2024
Merged

Remove nonstandard and redundant icons for government banner#10275
aduth merged 2 commits intomainfrom
aduth-rm-lock

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Mar 21, 2024

🛠 Summary of changes

Removes icon assets to allow design system standard to be used instead:

Visually, they are identical, but the custom lock icon we implemented previously was slightly lighter, which notably stood out more from the surrounding content (see screenshots below).

📜 Testing Plan

  1. Visit http://localhost:3000
  2. Click "Here's how you know"
  3. Observe no regressions in appearance of icons in banner content

👀 Screenshots

Before After
Screenshot 2024-03-21 at 11 27 02 AM Screenshot 2024-03-21 at 11 26 52 AM

changelog: Bug Fixes, Layout, Use design system asset for government banner lock icon
@aduth aduth marked this pull request as draft March 21, 2024 15:36
@aduth
Copy link
Contributor Author

aduth commented Mar 21, 2024

Converting to draft since I'm noticing several more images we can remove in favor of design system defaults, even within the banner itself.

Edit: Implemented in f44d2fd and updated pull request content to reflect.

@aduth aduth changed the title Remove nonstandard lock icon for government banner Remove nonstandard and redundant icons for government banner Mar 21, 2024
@aduth aduth marked this pull request as ready for review March 21, 2024 15:42
@zachmargolis
Copy link
Contributor

The color on the lock icon changed slightly, is it supposed to match the text it's in? Or be a specific color?

@aduth
Copy link
Contributor Author

aduth commented Mar 21, 2024

The color on the lock icon changed slightly, is it supposed to match the text it's in? Or be a specific color?

Technically it's supposed to match the text around it, if we were using the documented inline SVG styles. We don't follow this guidance and instead reference the image externally, as an optimization for collapsed content.

But the actual color change was mentioned in the pull request and even though it's still static and doesn't strictly follow the surrounding content, I think in practice the updated color is more aligned to the surrounding content.

the custom lock icon we implemented previously was slightly lighter, which notably stood out more from the surrounding content

@aduth
Copy link
Contributor Author

aduth commented Mar 21, 2024

Technically it's supposed to match the text around it, if we were using the documented inline SVG styles. We don't follow this guidance and instead reference the image externally, as an optimization for collapsed content.

For what it's worth, I would like to move toward reconciliation here, but it's something I'd propose to USWDS and we can take advantage of if/when it comes down to us.

@aduth aduth merged commit d9839df into main Mar 21, 2024
@aduth aduth deleted the aduth-rm-lock branch March 21, 2024 17:58
@aduth
Copy link
Contributor Author

aduth commented Mar 21, 2024

Technically it's supposed to match the text around it, if we were using the documented inline SVG styles. We don't follow this guidance and instead reference the image externally, as an optimization for collapsed content.

For what it's worth, I would like to move toward reconciliation here, but it's something I'd propose to USWDS and we can take advantage of if/when it comes down to us.

See upstream: uswds/uswds#5829

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants