Make Accordion fully accessible #2113
Conversation
🦋 Changeset detectedLatest commit: 8f11dae The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2113 +/- ##
=======================================
Coverage 97.06% 97.07%
=======================================
Files 243 243
Lines 28203 28238 +35
Branches 2462 2382 -80
=======================================
+ Hits 27376 27411 +35
Misses 827 827
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
Size Change: +12 B (0%) Total Size: 92.7 kB
ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-wnweeyosuo.chromatic.com/ Chromatic results:
|
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (6bbc4d6) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2113"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2113 |
There was a problem hiding this comment.
praise: This is sooo well detailed 👏
| - The accordion can be animated using the `animated` prop. However, | ||
| PLEASE set this according to the users' "prefers reduced motion" | ||
| setting (this can be in the application or in their operating system). |
There was a problem hiding this comment.
praise: Thanks for specifying this here!
| // Not testing the actual value of the aria-labelledby attribute | ||
| // because it comes from the unique ID generator. | ||
| expect(section1ContentPanel).toHaveAttribute("aria-labelledby"); |
There was a problem hiding this comment.
suggestion: I think you can still check the aria-labelledby value if you pass an id to the AccordionSection instance.
e.g.
<AccordionSection id="test"...>
then the panel would probably have something like: aria-labeledby=test-accordion-section-header
There was a problem hiding this comment.
Oh right! Thank you. I do think I have to update the headerId to make sure it uses a passed in ID to get that to work because I think it's only using a unique ID right now.
| @@ -75,6 +75,8 @@ type Props = AriaProps & { | |||
| style?: StyleType; | |||
| }; | |||
|
|
|||
| const LANDMARK_PROLIFERATION_THRESHOLD = 6; | |||
There was a problem hiding this comment.
praise: Thanks for adding this check 💯.
Summary:
This PR includes
regionas advised by w3aria-labelledbyas advised by w3Issue: https://khanacademy.atlassian.net/browse/WB-1585
Test plan:
yarn jest packages/wonder-blocks-accordion/src/components/__tests__/accordion.test.tsxGo to the storybook page and check the accessibility tool to confirm
that
regionandaria-labelledbyare set appropriately.Turn on Voiceover and confirm that the new region(s) are available in the landmarks menu.
trimmed.mov