Skip to content

Implement language picker expander as icon#10098

Merged
aduth merged 1 commit intomainfrom
aduth-language-picker-expander-icon
Feb 20, 2024
Merged

Implement language picker expander as icon#10098
aduth merged 1 commit intomainfrom
aduth-language-picker-expander-icon

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Feb 15, 2024

🛠 Summary of changes

Updates footer language picker expander icon to use IconComponent.

This is a planned follow-on to #10065.

Why?

  • Eliminates at least one network request (image) from the common layout
  • Simplifies and reduces the size of the application stylesheet
  • Consolidates on a common icon glyph for this sort of expander, also used in the "Here's how you know" banner (single "expand_more" icon vs. six variations of angle-arrow-*
    • Enables us to delete those icons from the design system
  • Avoids flickering that can occur initially when expanding, due to network request for separate image (rotates a single image instead)
  • Nifty animation! (disabled when preferred, for improved accessibility)

👀 Screenshots

expander.mov

changelog: Internal, Performance, Reduce number of image assets loaded in common layout
<span id="language-picker-description-<%= unique_id %>" class="language-picker__label-text">
<%= t('i18n.language') %>
</span>
<%= render IconComponent.new(icon: :expand_more, size: 3, class: 'language-picker__expander') %>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The specific use of the expand_more was intentional to enable reuse / caching with the same icon used in the banner for "Here's how you know".

Copy link
Contributor

Choose a reason for hiding this comment

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

if we wanted we could make add some sort of regression spec that makes sure those two link the same image? to encode the goal in our specs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an interesting thought, though I think it'd be tricky to implement in practice, since both load the image indirectly through CSS. I've been curious to use a bit more of the Chrome DevTools protocol like we did with browser emulation and that might be an option here (e.g. monitoring Network], but even with that could be hard to pinpoint what loads what.

<span id="language-picker-description-<%= unique_id %>" class="language-picker__label-text">
<%= t('i18n.language') %>
</span>
<%= render IconComponent.new(icon: :expand_more, size: 3, class: 'language-picker__expander') %>
Copy link
Contributor

Choose a reason for hiding this comment

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

if we wanted we could make add some sort of regression spec that makes sure those two link the same image? to encode the goal in our specs

@aduth aduth merged commit f2d1559 into main Feb 20, 2024
@aduth aduth deleted the aduth-language-picker-expander-icon branch February 20, 2024 13:38
aduth added a commit that referenced this pull request Feb 20, 2024
aduth added a commit that referenced this pull request Feb 20, 2024
* Revert "Implement language picker expander as icon (#10098)"

This reverts commit f2d1559.

* Revert "Reimplement icon component as SVG image mask (#10065)"

This reverts commit 4926cdd.
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