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

impr(test config): allow keyboard navigation (@Truiteseche) #5528

Conversation

Truiteseche
Copy link
Contributor

Description

Improve keyboard navigation accessibility on the test-config section, improve semantic.

Maybe the fact that we were not able to focus textButtons with the keyboard was on purpose but I think it's more relevant to allow it, especially since in mobile layout it is focusable. I also sometimes accidentally log out because the log out button is just before the text field in the tabindex hierarchy, so it would resolve that small but annoying issue.

impr(test-config section): impossible to navigate with the keyboard in the test-config section (@Truiteseche)

Related issues (doesn't fix it at all):
#4857

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label Jun 25, 2024
@Miodec
Copy link
Member

Miodec commented Jun 27, 2024

Focused elements should use the outline to indicate focus. I know it would require a lot more changes than this (you would need to remove variable padding on the button to avoid outline misalignment) - do you want me to do this or do you wanna give it a try?

@Miodec Miodec added the waiting for update Pull requests or issues that require changes/comments before continuing label Jun 27, 2024
@Truiteseche
Copy link
Contributor Author

Truiteseche commented Jun 28, 2024

Thank you for your feedback! Indeed I kicked the outline in favor of the same effect as on hover, because the outline was glitched, but yeah it would be better with the outline. I'd like to give it a try even though I don't fully understand what you mean removing variable padding.

@Miodec
Copy link
Member

Miodec commented Jul 1, 2024

Thank you for your feedback! Indeed I kicked the outline in favor of the same effect as on hover, because the outline was glitched, but yeah it would be better with the outline. I'd like to give it a try even though I don't fully understand what you mean removing variable padding.

By variable padding i mean this
image
image

@Truiteseche
Copy link
Contributor Author

Truiteseche commented Jul 3, 2024

Thank you very much! In my recent commit, I put the outline back in place, as you advised me I removed the variable padding (in favor of the variable margin) to avoid the outline misalignment, and I removed overflow: hidden on some containers to avoid the outline display problems On my side, it's ready to merge.

@Miodec Miodec changed the title impr(test-config section): impossible to navigate with the keyboard in the test-config section (@Truiteseche) impr(test config): allow keyboard navigation (@Truiteseche) Jul 4, 2024
@Miodec
Copy link
Member

Miodec commented Jul 4, 2024

Nicely done, thanks.

@Miodec Miodec merged commit cf855bd into monkeytypegame:master Jul 4, 2024
7 checks passed
@Truiteseche Truiteseche deleted the impr/test-config-accessibility-keyboard-navigation branch July 4, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend User interface or web stuff waiting for update Pull requests or issues that require changes/comments before continuing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants