-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Made the language and documentation version switcher accessible by keyboard. #2347
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
base: main
Are you sure you want to change the base?
Conversation
7431bbc to
74b62df
Compare
SaptakS
left a comment
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.
I haven't tested the styling, but left some comments on the markup. Let me know if you have any thoughts.
74b62df to
61c8772
Compare
|
Lovely suggestions thank you! I've tried to incorporate each of them 👍 |
djangoproject/scss/_style.scss
Outdated
| display: inline-block; | ||
| } | ||
| &:hover li.other, | ||
| &:focus-within li.other { |
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.
I think instead of using focus-within and removing .open usage in SCSS, it might be better to leave it the other way (i.e. use &.open li.other to actually show the other languages, instead of &:focus-within li.other).
My reasoning for this is, with &.open, the language options are shown on button press, whereas with focus-within, it shows as soon as the button is focused. I think the button press UX is better in keyboard, than automatically showing on focus, because with automatic showing with focus-within user will have to tab through all the languages and all the documentation version to reach the "top of the page" button. But with button press, the language and documentation versions only show if enter pressed, and otherwise can be easily tabbed through.
Thoughts?
This is how the interaction looks to me with .open
https://github.com/user-attachments/assets/7b616c81-2bf0-4fa9-8af8-4415816c20f2
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.
I think this is a nicer keyboard UX as well 👍 pushed a tweak
61c8772 to
c5cdfb1
Compare
Fixes #1955
Note that I removed the titles as I don't believe they are worth it