Skip to content

Implement language picker as design system accordion#7188

Merged
aduth merged 6 commits intomainfrom
aduth-language-picker
Oct 26, 2022
Merged

Implement language picker as design system accordion#7188
aduth merged 6 commits intomainfrom
aduth-language-picker

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Oct 21, 2022

🛠 Summary of changes

This pull request revises the language picker to leverage existing design system accordion behaviors, replacing an ad hoc implementation. This is similar to what had been implemented in the brochure site in GSA-TTS/identity-site#674 .

Benefits include:

  • Reduces size of JavaScript bundles in critical paths by eliminating custom JavaScript implementation
  • Ensures consistency of language picker with other accordion behaviors (including the "Official government website" banner)
  • Improves accessibility of the element by incorporating standard design system markup (e.g. the addition of aria-controls)
  • Brings some alignment to the brochure site language picker implementation (many styles were copied directly from the code in Refactor language picker as accordion GSA-TTS/identity-site#674) while retaining some of the IdP-specific requirements, absent a larger refactor toward implementing the Identifier component in the IdP.
  • Removes the second-to-last item remaining in the legacy app/javascript/app folder toward eventually eliminating the folder

👀 Screenshots

While the intent was to keep the appearance of the language picker largely the same, I tried to strike a balance with aligning to the brochure site language picker and simplifying the styling overall, which resulted in some minor changes.

Open to any and all suggestions!

Mobile:

State Before After
Collapsed mobile-before-collapsed mobile-after-collapsed
Expanded mobile-before-expanded mobile-after-expanded

Desktop:

State Before After
Collapsed desktop-before-collapsed desktop-after-collapsed
Expanded desktop-before-expanded desktop-after-expanded

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM!

@aduth aduth force-pushed the aduth-language-picker branch from a9eabd6 to c0357fd Compare October 21, 2022 19:54
@aduth aduth marked this pull request as draft October 21, 2022 19:54
@aduth aduth marked this pull request as ready for review October 24, 2022 15:11
@aduth aduth force-pushed the aduth-language-picker branch from dfac960 to 2bf1780 Compare October 26, 2022 13:50
@aduth
Copy link
Contributor Author

aduth commented Oct 26, 2022

Made a minor focus improvement in 2bf1780 to restore the offset which exists in the current production implementation. (It could probably be further improved, mostly concerned with not introducing a regression at this point)

Production Before After
Screen Shot 2022-10-26 at 9 47 56 AM Screen Shot 2022-10-26 at 9 49 12 AM Screen Shot 2022-10-26 at 9 49 33 AM

@aduth aduth merged commit 622f9c5 into main Oct 26, 2022
@aduth aduth deleted the aduth-language-picker branch October 26, 2022 15:02
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.

3 participants