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 in-page link without target #112

Open
wants to merge 2 commits into
base: edge-civictechto
Choose a base branch
from

Conversation

YumiChen
Copy link
Member

@YumiChen YumiChen commented Jan 21, 2025

Fix #79

For twitterConnectBtn and fbConnectBtn are not for page navigation( there are event listeners are bound to them instead), remove a tags and move menuitem role item to list items instead

Note. this UI is actually not in accessibility tree for its grandparent has display: none style. We can see if we want to review whether to include these types of issues in the board

@thomassth
Copy link

I think if they're supposed to be buttons, <button> would be more suitable?

@YumiChen
Copy link
Member Author

YumiChen commented Jan 22, 2025

Good point. Was referencing the W3C WAI-ARIA menubar example:
https://www.w3.org/WAI/ARIA/apg/patterns/menubar/examples/menubar-editor/

While both button and li should be accessible, I'll update to use button to align with the ids. Thanks for the suggestion!

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.

[a11y] Issue 21: In-page link without target (Closed Page)
2 participants