Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use picture element to avoid unnecessary image load #11182

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

aduth
Copy link
Member

@aduth aduth commented Aug 30, 2024

🛠 Summary of changes

Updates footer markup to use <picture> element to avoid loading an unnecessary image.

The GSA logo rendered the footer is either light or dark depending on the viewport size. In the previous implementation, both images would be downloaded, despite the fact that only one would ever be visible.

📜 Testing Plan

  1. Go to http://localhost:3000
  2. Verify no visual regression of the "GSA" logo in the footer, at both large and small viewport size
  3. Open browser developer tools Network tab
  4. Reload the page
  5. Verify that only one variation of the "square-gsa" logo is loaded

👀 Screenshots

Before After
image image

changelog: Internal, Performance, Use picture element to avoid unnecessary image load
Comment on lines 4 to 5
<%= tag.source(media: '(min-width: 1024px)', srcset: asset_url('sp-logos/square-gsa.svg')) %>
<%= image_tag(asset_url('sp-logos/square-gsa-dark.svg'), size: 20, alt: '') %>
Copy link
Member Author

Choose a reason for hiding this comment

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

There's a little bit of trickery here, where we never expect the "fallback" to be loaded here. But the fallback is strictly required by the spec. The only reason the fallback would be triggered is for mobile, where we'd want to show the mobile image, hence the inversion between the two images here.

@aduth
Copy link
Member Author

aduth commented Aug 30, 2024

Separately, I'd previously considered using an SVG mask for this image to vary the color depending on viewport size, but it's not just the difference of blue vs. white, since the actual GSA text itself is part of the image. Technically we might be able to make that invisible and let the background show through.

@aduth aduth merged commit 652f19e into main Sep 3, 2024
2 checks passed
@aduth aduth deleted the aduth-picture-el-footer branch September 3, 2024 21:10
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