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

Use aria-hidden="true" for arrow icons in sidebar #1387

Closed
afercia opened this issue Jun 23, 2017 · 12 comments
Closed

Use aria-hidden="true" for arrow icons in sidebar #1387

afercia opened this issue Jun 23, 2017 · 12 comments
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@afercia
Copy link
Contributor

afercia commented Jun 23, 2017

Firefox and NVDA suffer from a bug where the value change of an aria-expanded attribute doesn't get announced when the element it is applied to gets "repainted". This was first noticed for CSS generated icons but seems it applies also to the SVG icons in the sidebar meta boxes because they're updated on the fly when a meta box opens:

Technical details on:
nvaccess/nvda#5247
Trying to summarize the feedback from @jcsteh :

When the CSS pseudo-element content is changed, the accessible (jargon for "set of properties in the browser accessibility tree") for the affected element ... gets completely regenerated, which includes any children ... As a result, NVDA is told that focus landed on a new element which just happens to occupy exactly the same space ... the fact that the behaviour is different between the two cases (with and without the pseudo-element content change) is a Firefox bug. The fact that we don't speak anything at all (even a focus change) is an NVDA bug.

When a meta box is closed, it uses a dashicons-arrow-right

screen shot 2017-06-23 at 10 45 38

that is then updated on the fly to a dashicons-arrow-down when the meta box opens:

screen shot 2017-06-23 at 10 46 02

This triggers the bug and the aria-expanded state change is not announced. We've fixed this in WordPress core in a few places, simply wrapping the icon in a span with an aria-hidden="true" attribute. I'd propose to use the same fix in Gutenberg, on a case by case basis where icons are updated on the fly and there's some ARIA state change screen readers should announce.

@afercia
Copy link
Contributor Author

afercia commented May 31, 2018

I'm going to re-open this (it was reverted in #1537) as it still happens with Firefox + NVDA (nothing is announced) and JAWS re-announces the whole thing including the component label, while it should just announce the state change "expanded" or "collapsed".

@samikeijonen
Copy link
Contributor

Does it help if we just rotate the SVG with CSS? Not actually change it at all.

@afercia
Copy link
Contributor Author

afercia commented Jun 21, 2018

No it wouldn't help because the browser repaints the element anyways.

@tofumatt tofumatt changed the title Completely hide sidebar meta boxes SVG icons from assistive technologies Use aria-hidden="true" for arrow icons in sidebar Oct 4, 2018
@tofumatt
Copy link
Member

tofumatt commented Oct 4, 2018

I'm reducing the scope of this issue to the scenario highlighted above: the sidebar icons should be hidden from assistive technology using aria-hidden="true". If there are other instances where this should happen: please file separate issues so we can address them there (or provide an exhaustive list of which elements to hide) 🙂

@tofumatt tofumatt self-assigned this Oct 4, 2018
@afercia
Copy link
Contributor Author

afercia commented Oct 5, 2018

Note: the icons have been changed in f9e82d0 I guess it doesn't make a difference and the Firefox+NVDA issue is still there (becaus there's still a re-rendering). Testing it again would be nice.

@mtias mtias added the Needs Testing Needs further testing to be confirmed. label Oct 7, 2018
@mtias mtias added this to the WordPress 5.0 milestone Oct 7, 2018
@designsimply
Copy link
Member

I tested with WordPress 4.9.8 and Gutenberg 4.1.0-rc.2 using Firefox 62.0.3 on macOS 10.13.6 and found that the Settings > Document > Featured Image > svg icon to toggle the panel open/closed does have the aria-hidden="true" attribute applied.

screen shot 2018-10-24 at 4 42 01 pm

@afercia I also tried testing with VoiceOver on my Mac to see what would get read aloud when tabbing through the document settings component panels, and I found that it did said "text" for the panel heading and the icon was ignored. (2m16s) It's quite possible I did it wrong though! To the point of this specific issue being about icons, I believe that the aria-hidden="true" for sidebar icon panel toggles is being ignored which is expected at this time. Is this correct and is this test good enough or should I look for extra help in testing this? 😊

@designsimply designsimply added [Status] Needs More Info Follow-up required in order to be actionable. and removed Needs Testing Needs further testing to be confirmed. labels Oct 24, 2018
@afercia
Copy link
Contributor Author

afercia commented Oct 25, 2018

@designsimply thanks for testing! However, the proposal here is to add a span element with aria-hidden=true around the SVG icon. The bug to solve is Firefox + NVDA don't announce the aria-expanded value expanded / collapsed because of the repainting triggered by the icon change.

@jcsteh
Copy link

jcsteh commented Oct 25, 2018 via email

@afercia
Copy link
Contributor Author

afercia commented Oct 25, 2018

@jcsteh thanks for chiming in and for the update!

@catehstn
Copy link

catehstn commented Nov 1, 2018

@designsimply can you please take a look and see if this has addressed this issue, please? Thank you!

@afercia
Copy link
Contributor Author

afercia commented Nov 1, 2018

@catehstn @designsimply I'm not fully convinced testing against a software that is in alpha state is the best path forward. I'd recommend to wait the NVDA fix gets actually released. I see it's in the 2018.4 milestone which, at the time of writing this, is scheduled for November 22. Thank you.

@timwright12
Copy link
Contributor

@afercia @tofumatt This one was pretty straightforward, so I just went ahead and patched it.

@mtias mtias added [Status] In Progress Tracking issues with work in progress and removed [Status] Needs More Info Follow-up required in order to be actionable. labels Nov 13, 2018
@designsimply designsimply removed the [Status] In Progress Tracking issues with work in progress label Nov 23, 2018
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).
Projects
None yet
Development

No branches or pull requests

8 participants