Add keyboard shortcut and aria attribute to menu button #2342
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After restore menu toggle to the logo, these things might need to change.
Use resize event listener to set button role
If only set button role in page load, when users resize the browser window from desktop layout to mobile layout, will cause the toggle function to fail.
Set aria-expanded to logo container
After change the button role to logo container, the button in logo container can be removed. And the aria-expanded should set to logo container.
Add keyboard shortcut to logo button
According to ARIA Authoring Practices Guide, the button should add support for Enter and Space key to activate the button. Because that's the native HTML button's behavior, user may expect this function when focus on logo button.
Use addEventListener instead of onClick function
The onClick function will cause the go-to home page link in desktop layout not work. I think it might be the preventDefault method to cause this problem. So I just use the addEventListener method.
Do you follow the guidelines?