Skip to content

Render icon for mobile navigation close button#10056

Merged
aduth merged 2 commits intomainfrom
aduth-mobile-nav-close-icon
Feb 8, 2024
Merged

Render icon for mobile navigation close button#10056
aduth merged 2 commits intomainfrom
aduth-mobile-nav-close-icon

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Feb 8, 2024

🛠 Summary of changes

Updates rendering of the mobile navigation menu close button icon to use IconComponent instead of a custom SVG.

Why?

  • This image comes from the Login.gov Design System, but is not used within the design system itself, so I'm aiming to remove it from there
  • Avoids redundancy and reduces risk for desync in the styling of "Close" icons
  • May marginally improve performance since the single sprite.svg SVG sprite image is assumed to be cached by the time this image is displayed

📜 Testing Plan

  1. Go to http://localhost:3000 on a mobile device (or emulated browser viewport)
  2. Sign in
  3. From the account dashboard, click "Menu" in the top-right
  4. Observe that the menu's close button is visible, shown as a blue "X" icon

👀 Screenshots

Visual differences are expected to be very minimal.

Before After
localhost_3000_account(iPhone XR) (1) localhost_3000_account(iPhone XR)

aduth added 2 commits February 8, 2024 08:58
changelog: Internal, Code Quality, Remove redundant image for mobile navigation menu button
@aduth
Copy link
Contributor Author

aduth commented Feb 8, 2024

  • May marginally improve performance since the single sprite.svg SVG sprite image is assumed to be cached by the time this image is displayed

Thinking out loud a bit: This is probably true for this icon's usage, since it appears on the account page where we're using other icons. But I'm not sure the guidance holds true for replacing any ad-hoc icon with the sprite, since it may introduce the sprite download to a page where it doesn't already exist. For example, we have icons we could replace on the sign-in page (in the "Here's how you know" banner and language picker), but the sprite isn't currently being loaded on those pages, and it's about 22kb in size.

Some ideas:

  • There might be opportunities to reduce the size of the sprite itself, e.g. updating outdated SVGO, or configure sprite generation to trim unnecessary data, or post-process the generated sprite with optimizing tool
  • Bypass the sprite and render the original SVGs, optionally as inline elements

@aduth aduth merged commit 1047126 into main Feb 8, 2024
@aduth aduth deleted the aduth-mobile-nav-close-icon branch February 8, 2024 15:10
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.

2 participants