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

Revisit a11y affordances of List View nodes #46585

Closed
getdave opened this issue Dec 15, 2022 · 11 comments
Closed

Revisit a11y affordances of List View nodes #46585

getdave opened this issue Dec 15, 2022 · 11 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility [Status] Blocked Used to indicate that a current effort isn't able to move forward [Type] Bug An existing feature does not function as intended [Type] Experimental Experimental feature or API.

Comments

@getdave
Copy link
Contributor

getdave commented Dec 15, 2022

Screen Shot 2022-12-15 at 14 48 25

It appears that the components within the <td> for nodes in the offcanvas experiment are aria-hidden. Does this make the node always hidden to screen readers?

I seem to remember that this may also be true for the main list view because otherwise screen readers will announce each node on navigating between them which is annoying.

@talldan Can you remember about this and whether we need to do anything to improve it?

@getdave getdave added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Block] Navigation Affects the Navigation Block [Type] Experimental Experimental feature or API. labels Dec 15, 2022
@talldan
Copy link
Contributor

talldan commented Dec 16, 2022

IIRC it was added to List View to prevent NVDA and JAWS from switching modes. I wasn't all that involved in the change, I think @alexstine and @andrewserong know more about it.

@alexstine
Copy link
Contributor

It was added to get around mode switching and list view select oddness. Might be time to remove it now though since list view has better markup with the application role. I may make a PR if I ever get enough time.

@getdave
Copy link
Contributor Author

getdave commented Jan 3, 2023

So to confirm we need a PR to remove the aria-hidden from the nodes?

@alexstine
Copy link
Contributor

@getdave Probably time to do that now. Also likely need to remove some other attributes from the tr tag. It will take some work. Lots of testing with different screen readers as well. We worked around this before by hacking the heck out of it, that is what you currently see. Although aria-hidden is applied to the link, aria-label on the tr is still allowing it to be read. I think that is how we did it.

@andrewserong might be best to unravel this since he implemented it. I think it is worth testing without the aria-hidden now that the list view itself is in better accessibility shape. We'll need to test block selection carefully. That is what brought on all these changes.

Thanks.

@andrewserong
Copy link
Contributor

andrewserong commented Jan 9, 2023

As far as I can remember, the aria-hidden we added was for the Button component that renders the a tag, to prevent the link from being announced twice, as well as helping ensure that NVDA didn't unintentionally switch modes while navigating. So I think that means the cell itself, which is a level up, shouldn't be hidden, and other elements that should be available to a screen reader could be added as a sibling to the block select button, rather than inside it?

As Alex mentions, though, things might have moved around a bit since we last added that in. My main recommendation is that if you're going to try removing it, be sure to test with a screen reader to make sure that it doesn't add back in more redundant announcements as elements are focused or navigated to.

@alexstine
Copy link
Contributor

The mode switching should be a non-issue now with the whole grid wrapped in a role="application". Others are still valid test cases though.

@talldan
Copy link
Contributor

talldan commented Jan 10, 2023

I gave List View (and the off canvas editor) a test using Voiceover today and noticed it's not working as expected.

The aria descriptions for items don't seem to be announced. Blocks are announced as 'Link, block name', which isn't quite as informative as it used to be. I'm not sure if it's related to this aria-hidden change. I didn't notice the same issue in Alex's video, so I'm guessing this is only a voiceover problem.

@getdave
Copy link
Contributor Author

getdave commented Jan 31, 2023

This Issue isn't specifically a problem of the Nav list view. It's a problem for all list view instances.

It needs to be resolved but it's going to require folks with NVDA to test and advise on which attributes need to change and how. Voiceover doesn't seem to be sufficient for testing unfortunately.

@scruffian
Copy link
Contributor

I think we should revisit this once Navigation: Remove OffCanvasEditor component is done.

@scruffian scruffian added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Mar 8, 2023
@alexstine
Copy link
Contributor

I am working on it slowly here: #48461

@scruffian
Copy link
Contributor

Since #48461 is closed I think we can also close this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility [Status] Blocked Used to indicate that a current effort isn't able to move forward [Type] Bug An existing feature does not function as intended [Type] Experimental Experimental feature or API.
Projects
None yet
Development

No branches or pull requests

5 participants