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

Add aria-expanded to Button component reflecting isToggled #473

Closed
aduth opened this issue Apr 20, 2017 · 5 comments
Closed

Add aria-expanded to Button component reflecting isToggled #473

aduth opened this issue Apr 20, 2017 · 5 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@aduth
Copy link
Member

aduth commented Apr 20, 2017

Tip from @afercia at #392 (comment) :

[...] using an aria-pressed attribute set to true for the active mode and false for the inactive one. This would make them being announced as selected Toggle button and Toggle button, indicating that only one can be active at a time.

In #449 (cc @youknowriad), we introduced a new isToggled prop to the Button component to indicate the toggled state of the sidebar. Given above, it's probably appropriate to apply the aria-pressed attribute when this prop is true or false (attribute reflecting prop, omitted when undefined).

Related resource: https://www.w3.org/TR/wai-aria/states_and_properties#aria-pressed

Edit:

Per discussion below, aria-expanded (and aria-controls) most accurately describe this behavior.

@aduth aduth added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Task Issues or PRs that have been broken down into an individual action to take labels Apr 20, 2017
@afercia
Copy link
Contributor

afercia commented Apr 20, 2017

Hm not exactly :) aria-pressed should be used when there's a group of buttons and only one can be "active" at a time. See for example in the Customizer the "device preview" buttons at the bottom of the sidebar.

Instead, when a button expands or collapses a sidebar (or any toggleable section), the most appropriate ARIA attribute to use is aria-expanded (false/true) to indicate something has expanded/collapsed in the page.

@aduth
Copy link
Member Author

aduth commented Apr 20, 2017

Interesting. The spec reads as though it applies to a single button with on/off states:

Toggle buttons require a full press-and-release cycle to change their value. Activating it once changes the value to true, and activating it another time changes the value back to false.

https://www.w3.org/TR/wai-aria/states_and_properties#aria-pressed

I agree aria-expanded seems more applicable though. Appears it should also apply aria-controls since the button controls the behavior of the sidebar?

If the element with the aria-expanded attribute controls the expansion of another grouping container that is not 'owned by' the element, the author SHOULD reference the container by using the aria-controls attribute.

https://www.w3.org/TR/wai-aria/states_and_properties#aria-expanded

I'll update the issue accordingly.

@aduth aduth changed the title Add aria-pressed to Button component reflecting isToggled Add aria-expanded to Button component reflecting isToggled Apr 20, 2017
@afercia
Copy link
Contributor

afercia commented Apr 20, 2017

Yeah, but on a single button doesn't help so much. The practical effect is screen readers (VoiceOver for example) will announce the button as toggle button instead of just button when the aria-pressed attribute is present.

aria-expanded is already used a lot in WordPress core, and it's the right thing to use when something expands/collapses. Note that the spec allows aria-expanded to be used also on the expanding element but that doesn't help so much users to understand what's going on. We use it only on the control that expands/collapses something, as that is the UI control currently focused and announced.

About aria-controls, it would be great if it only worked 😬 See http://www.heydonworks.com/article/aria-controls-is-poop (sorry for the post title!)

@afercia
Copy link
Contributor

afercia commented Aug 1, 2017

@aduth I was thinking to close this and set aria-expanded or aria-pressed on a case by case basis. Some buttons need aria-expanded, for example the Settings sidebar toggle. Other buttons need aria-pressed, for example the Calendar AM/PM.

@aduth
Copy link
Member Author

aduth commented Aug 1, 2017

That sounds fine to me @afercia 👍

@aduth aduth closed this as completed Aug 1, 2017
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). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

2 participants