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

Expandable UI sections accessibility #469

Closed
afercia opened this issue Apr 20, 2017 · 14 comments
Closed

Expandable UI sections accessibility #469

afercia opened this issue Apr 20, 2017 · 14 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Priority] High Used to indicate top priority items that need quick attention

Comments

@afercia
Copy link
Contributor

afercia commented Apr 20, 2017

This is about any UI section that expands to show something, for example, the Post Settings:

admin ui default

For accessibility, ideally the expandable sections should be placed in the markup immediately after the control that expands them, or they should be implemented in a way that the sections become the only "reachable" content after the control that expands them.

There are different techniques to implement this, depending on the placement of the expandable section in the markup. Worth exploring a bit the best option. For now, I'd suggest just a couple things:

  • programmatically moving focus from the button to the panel is a no go, as the information provided by the aria-expanded attribute on the button would be missed (yes, the buttons should use aria-expanded)
  • using aria-controls to reference a panel which is far from the button in the markup wouldn't solve the issue, because aria-controls is poop (sorry, it's a quote!)
  • once the panel is open, consider to treat it as sort of "modal" as Adrian Roselli suggested for the content blocks (ping @aardrian !); this way, the panel would be the only available content for keyboard and screen reader users, until they complete the task within the panel. This can be implemented with different techniques, to evaluate with care (constraining tabbing, hiding anything else with aria-hidden, etc.)
@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Apr 20, 2017
@jasmussen
Copy link
Contributor

Thank you again. Very actionable — the best kind of -'able!

Do defaults make a difference for accessibility here? For example if we show the sidebar open by default, and show all boxes inside expanded by default, would that be better or worse?

@aardrian
Copy link

Is the panel really analogous to a modal? As in, is it necessary for the user to complete a task (or abort that task) before being allowed to leave the panel? Is there ever a case where information necessary to complete that task is available elsewhere on the screen?

@afercia
Copy link
Contributor Author

afercia commented Apr 20, 2017

The sidebar is visible by default, by the way.

Ah ok, I was assuming it was closed by default, as in the screenshot posted on Slack. Then yes, defaults make a difference: if it's open by default we can't treat it as a modal.
By the way, the sidebar/panels placement still matters. When tabbing, users will find a button that says Post Settings button expanded. Then, the sidebar should immediately follow.
That's a bit more complicated 😬
Wondering if moving the Publish button after the sidebar would make things easier.
What about two toolbars?

  • on the top: controls related to content editing: switch mode, undo/redo, insert
  • then the Post Settings button with the sidebar immediately following the button
  • on the bottom toolbar: controls related to the post state: Saved, Preview, Publish

This way, users could choose to navigate trough all the sidebar controls/options or collapse it and go straight to Saved, Preview, Publish.

@aardrian
Copy link

Wondering if moving the Publish button after the sidebar would make things easier.

Any reason not to have two Publish buttons? One before your sidebar (full of disclosure widgets) and one at the end of the sidebar.

This presumes that the goal of the UI is to reduce friction to hitting that Publish button, of course.

@afercia
Copy link
Contributor Author

afercia commented Apr 20, 2017

Hm... the Publish button on the top would be between the sidebar and the controls that open the sidebar... not sure that would be so ideal 😞

@afercia
Copy link
Contributor Author

afercia commented Apr 30, 2017

In the case of the "Post Settings" button, the first step for better a11y would be adding an aria-expanded attribute. If set when using the component, it would be as simple as changing
<Button onClick={ toggleSidebar } isToggled={ isSidebarOpened }>
to
<Button onClick={ toggleSidebar } isToggled={ isSidebarOpened } aria-expanded={ isSidebarOpened }>
unless controlling this from the component is considered preferable (but not all the <Button> will need aria-expanded). /cc @aduth

@afercia
Copy link
Contributor Author

afercia commented Apr 30, 2017

See #473

@mtias
Copy link
Member

mtias commented Aug 31, 2017

@afercia did your recent work on the tabs and panels address this?

@afercia
Copy link
Contributor Author

afercia commented Aug 31, 2017

@mtias Even if the issue title uses the term "panel" this doesn't apply only to the sidebar "panels" but to:

any UI section that expands to show something

I'd consider it as a "tracking issue" because any developer working on such UI patterns should be aware of what to do. See for example #2204

@mtias
Copy link
Member

mtias commented Aug 31, 2017

Can we edit the title to make it more clear? We should find a different way to keep these considerations, maybe an accessibility doc could group them.

@afercia afercia changed the title Expandable panels accessibility Expandable UI sections accessibility Aug 31, 2017
@afercia
Copy link
Contributor Author

afercia commented Aug 31, 2017

We should find a different way to keep these considerations, maybe an accessibility doc could group them.

Totally agree, would be great to have someone volunteering for this 🙂

aduth added a commit that referenced this issue Oct 27, 2017
Recommended combobox aria-controls is non-configurable and may recommend a poorer experience.

See: #469
See: http://www.heydonworks.com/article/aria-controls-is-poop
@jeffpaul jeffpaul added this to the Merge Proposal milestone Feb 8, 2018
@afercia afercia added the [Type] Developer Documentation Documentation for developers label Mar 28, 2018
@karmatosed karmatosed modified the milestones: Merge Proposal, Merge Proposal: Accessibility Apr 12, 2018
@grahamarmfield
Copy link

Happened to come across this when testing something else... And I have some views.

  • The expanded panels now seem to be adjacent to the buttons that open them - which is good.
  • Despite what Heydon Pickering says in his article that @afercia references, I think the aria-controls attribute should be added to the button that opens the panel. This means that the panels will need to have some kind of id set.
  • I'm not in favour of making the panels modals as I may want to have more than one open at a time. For example: thinking about background and foreground colours.

@karmatosed karmatosed removed the [Type] Developer Documentation Documentation for developers label Apr 13, 2018
@karmatosed
Copy link
Member

I"m removing the documentation tab so this gets focused on for a code solution.

@karmatosed karmatosed added the [Priority] High Used to indicate top priority items that need quick attention label Apr 13, 2018
@afercia
Copy link
Contributor Author

afercia commented Jun 20, 2018

Many things have changed since this issue was opened. Gutenberg has taken a different direction, making most of the expandable "panels" behave like sort of modals. In these cases there's the need to manage focus programmatically and implement also other things, like constraining tabbing within a panel. See #6987 for this.

We've discussed this issue during today's accessibility bug scrub and decided to close it. Specific pending cases should be addressed in specific issues.

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). [Priority] High Used to indicate top priority items that need quick attention
Projects
None yet
Development

No branches or pull requests

7 participants