Skip to content

Render language picker as collapsed by default in markup#7236

Merged
aduth merged 3 commits intomainfrom
aduth-language-picker-collapsed
Oct 27, 2022
Merged

Render language picker as collapsed by default in markup#7236
aduth merged 3 commits intomainfrom
aduth-language-picker-collapsed

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Oct 27, 2022

🛠 Summary of changes

As a follow-on improvement (minor regression fix) to #7188, this updates the language picker component to render as collapsed by default in the initial markup.

While previously the language picker would become collapsed by default, it wouldn't occur until JavaScript had initialized, which could result in some brief flickering of the language picker while the page is loading.

It could be argued that the collapsing should occur in JavaScript in order to support no-JavaScript browsers, however (a) this is not how it behaved prior to #7188 and the changes here seek to restore the original behavior, and (b) due to the styling of the language picker, it's possible for the options to overlap page content in such a way that may worsen the experience for no-JavaScript users. Future work could explore improvements to the no-JavaScript language picker experience.

This also resolves a styling issue where the arrow caret would rely on the presence of aria-expanded attribute to display the correct icon, which was not assigned previously in markup and would display incorrectly both for no-JavaScript users and until initialization occurred.

📜 Testing Plan

  • Ensure no regressions in the behavior of the language picker

👀 Screenshots

Before After
Screen Shot 2022-10-27 at 11 13 14 AM Screen Shot 2022-10-27 at 11 13 05 AM

aduth added 3 commits October 27, 2022 11:07
changelog: Bug Fixes, Page Layout, Avoid flickering language picker appearance on page load
@zachmargolis
Copy link
Contributor

Should we pick this onto RC 222 before deploying?

@aduth
Copy link
Contributor Author

aduth commented Oct 27, 2022

Should we pick this onto RC 222 before deploying?

Eh, we can, but it's not a huge issue IMO if it's more trouble to cherry-pick. In practice it's a bit of brief flickering on page load.

@aduth aduth merged commit 1b71195 into main Oct 27, 2022
@aduth aduth deleted the aduth-language-picker-collapsed branch October 27, 2022 15:32
zachmargolis pushed a commit that referenced this pull request Oct 27, 2022
* Collapse language picker by default

changelog: Bug Fixes, Page Layout, Avoid flickering language picker appearance on page load

* Add spec for language picker accessible relation

* Add spec for language options

(cherry picked from commit 1b71195)
@zachmargolis
Copy link
Contributor

zachmargolis commented Oct 27, 2022

Should we pick this onto RC 222 before deploying?

Eh, we can, but it's not a huge issue IMO if it's more trouble to cherry-pick. In practice it's a bit of brief flickering on page load.

Done, see fe15105 (thanks to cherry-pick -x for keeping the original SHA as a reference)

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