-
Notifications
You must be signed in to change notification settings - Fork 1
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
Accessibility fixes: keyboard navigation to city, and checkbox labels #351
Accessibility fixes: keyboard navigation to city, and checkbox labels #351
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
a3941cd
to
25c9f95
Compare
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🟡 | Statements | 65.24% (+0.51% 🔼) |
760/1165 |
🔴 | Branches | 49.71% (+0.14% 🔼) |
174/350 |
🟡 | Functions | 62.5% (+0.43% 🔼) |
165/264 |
🟡 | Lines | 64.37% (+0.53% 🔼) |
701/1089 |
Test suite run success
364 tests passing in 28 suites.
Report generated by 🧪jest coverage report action from c639448
Ok, that's looking healthy now. Note I have used |
This comment was marked as outdated.
This comment was marked as outdated.
Thank you so much for your work on this and updating the PR, Chris! This is hugely appreciated. I am merging and deploying the changes now. Please note, that we are also switching to a new production sever, so the updated UI will be available at http://136.156.139.13 |
I noted that it isn't possible to navigate to single city view using keyboard (Enter key), which excludes all those who are unable to use mouse.
This PR addresses that, while also correcting another accessibility issue in which checkboxes were not associated with their label.
I have also tested these changes using NVDA screen reader, and they work as expected.
resolves #350