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

Fix a11y in header button #1833

Merged
merged 12 commits into from
Mar 23, 2025

Conversation

ShubhamOulkar
Copy link
Contributor

@ShubhamOulkar ShubhamOulkar commented Mar 12, 2025

Task

Closes #1830

Tabbing navigation shown on landing page
theme-lang-fix

New lang picker design

image

Note:- This PR specifically addresses keyboard navigation for the language picker and toggle theme button. Other navigation menus and submenus should also be fully accessible via keyboard but are not covered in this PR. However, the language picker on mobile devices currently has duplicate id attributes, which need to be removed, and CSS design should be update accordingly.

@ShubhamOulkar ShubhamOulkar requested review from a team as code owners March 12, 2025 04:49
Copy link

netlify bot commented Mar 12, 2025

Deploy Preview for expressjscom-preview ready!

Name Link
🔨 Latest commit 0f06fd1
🔍 Latest deploy log https://app.netlify.com/sites/expressjscom-preview/deploys/67e05379f3ce8a0008ae70e9
😎 Deploy Preview https://deploy-preview-1833--expressjscom-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ShubhamOulkar ShubhamOulkar marked this pull request as draft March 12, 2025 04:51
@ShubhamOulkar
Copy link
Contributor Author

ShubhamOulkar commented Mar 12, 2025

Should we consider adding country flags to the language picker?

@ShubhamOulkar ShubhamOulkar marked this pull request as ready for review March 13, 2025 09:40
@bjohansebas
Copy link
Member

Should we consider adding country flags to the language picker?

Hmm, I don’t know, if we add the flags as emojis, they might look different depending on the system. If we add them as SVGs, we’d have quite a few images. Isn’t just the name enough?

@ShubhamOulkar
Copy link
Contributor Author

ShubhamOulkar commented Mar 16, 2025

Isn’t just the name enough?

It is more than enough.

Should we add a title attribute in English to each language link? This is just a random thought that came to my mind while testing. All language names are translated into their respective scripts, and now I can't tell which one is Chinese, Korean, or Japanese. Also it improves accessibility for all users.

We don't need language-picker-mobile.html; use the same picker on mobile devices with some additional styling. Also, we can remove JS and CSS code.

@bjohansebas
Copy link
Member

It's a good idea to move the styles to classes. The ID should never have been used for styling, but it's necessary for the JavaScript, so it should be kept.

Make that last change, and we can merge this.

@bjohansebas bjohansebas merged commit 461386f into expressjs:gh-pages Mar 23, 2025
7 checks passed
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.

Change theme and language picker buttons are not accessible by keyboard
2 participants