Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

feat(Accordion): add a11y behavior for kb navigation and screen reader support#1322

Merged
silviuaavram merged 46 commits intomasterfrom
feat-accordion-behavior
May 22, 2019
Merged

feat(Accordion): add a11y behavior for kb navigation and screen reader support#1322
silviuaavram merged 46 commits intomasterfrom
feat-accordion-behavior

Conversation

@silviuaavram
Copy link
Collaborator

@silviuaavram silviuaavram commented May 10, 2019

Work that aims to implement the Accordion design pattern from aria. https://www.w3.org/TR/2017/NOTE-wai-aria-practices-1.1-20171214/#accordion and example https://www.w3.org/TR/2017/NOTE-wai-aria-practices-1.1-20171214/examples/accordion/accordion.html

Changes:

  • title has new structure. It has a focusable Box with a button role between root and Layout.
  • changed element types to a description list from HTML5. Used dl, dh and dd for accordion, title and content.
  • added behaviors for accordion, title and content.
  • added default IDs for title and content. Did it from Accordion, could not figure out a better way through behaviors.
  • implemented expanded Accordion type. With it truthy, it will always keep a panel open.
  • added keyboard actions support, on Arrow up/down, home/end and Enter/Space. Navigation is circular, so updated the ContainerFocusHandler to support this.
    -added unit tests for Accordion, Title, Content and behaviors.

Work in progress:

  • Ctrl + PageUp/PageDown from Content to jump focus to previous/next Title. - Chrome uses this for tab navigation. I don't think we should override it, and this shortcut is only useful for big panels with many focusable elements in the Content.

<Box
onFocus={this.handleFocus}
onClick={this.handleClick}
className={AccordionTitle.slotClassNames.button}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not button anymore, let's change the name of the slotClassNames

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more question, do we really need the box around the Layout? The Layout itself is a div, can't we just use the Layout component here?

@mnajdova
Copy link
Contributor

mnajdova commented May 17, 2019

We can use marginInlineStart: 0, in the accordionContentStyles.ts root elements, in order to avoid the browser's added spacing before the accordion content.

@silviuaavram silviuaavram merged commit 35375e3 into master May 22, 2019
@delete-merged-branch delete-merged-branch bot deleted the feat-accordion-behavior branch May 22, 2019 11:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants