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

Remove unneeded tab, tablist and aria-selected roles from navigation #5075

Merged
merged 1 commit into from Jan 19, 2024

Conversation

JuliaKirschenheuter
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter commented Jan 16, 2024

☑️ Resolves

🖼️ Screenshots

🏚️ Before | 🏡 After

No visual changes. Result: no errors through automatically a11y tests.

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@susnux susnux added bug Something isn't working accessibility Making sure we design for the widest range of people possible, including those who have disabilities feature: settings Related to the settings component labels Jan 16, 2024
@ShGKme
Copy link
Contributor

ShGKme commented Jan 18, 2024

  • Button + manual focus - doesn't move focus (both general and screen reader)
  • Link + prevent + manual focus - doesn't move focus (both general and screen reader)
  • Link with native behavior - focus is moved, but with a very weird behavior.
    It is moved though other elements, e.g. skip links of table header. And I cannot understand why...

For example, on this video, after click the screen reader focus is successfully moved on the header. However, it made many steps and went through other elements, like 5th user's action menu button (yellow square).

focus-bug.mp4

I tried to disable the focus trap, remove debounce unfocus, play with the link and nothing helped.

If you don't have any ideas here, I'd propose to disable default link behavior for now with .prevent and fix it later.

@JuliaKirschenheuter
Copy link
Contributor Author

JuliaKirschenheuter commented Jan 18, 2024

disable default link behavior for now with .prevent

Following behavior will take place: link will be semantic link but will work as a button. That means focus will stay on links and strange focus behavior will be prohibited.

Let's do that! -> Right/perfect focus isn't so important as odd screen reader behavior

@susnux
Copy link
Contributor

susnux commented Jan 18, 2024

  • It is moved though other elements

Maybe because it is handled like navigation -> reset to tabstop 0 and then move to ID?

@ShGKme
Copy link
Contributor

ShGKme commented Jan 18, 2024

Maybe because it is handled like navigation -> reset to tabstop 0 and then move to ID?

I tried similar navigation on MDN and it worked fine...

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-haspopup

image

…ation

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
@JuliaKirschenheuter JuliaKirschenheuter merged commit 1ad3480 into master Jan 19, 2024
15 checks passed
@JuliaKirschenheuter JuliaKirschenheuter deleted the fix/42599-remove-unneeded-attrs branch January 19, 2024 10:53
@Pytal Pytal mentioned this pull request Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: settings Related to the settings component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants