WB-1636: Announce popover contents correctly to screen readers #2134
Conversation
🦋 Changeset detectedLatest commit: d17c268 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 |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
|
Size Change: +44 B (0%) Total Size: 91 kB
ℹ️ View Unchanged
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (43fcb47) 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="PR2134"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2134 |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2134 +/- ##
=======================================
Coverage 97.05% 97.05%
=======================================
Files 241 241
Lines 28038 28055 +17
Branches 2464 2417 -47
=======================================
+ Hits 27212 27229 +17
Misses 826 826
Continue to review full report in Codecov by Sentry.
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-qmxqgpdpxr.chromatic.com/ Chromatic results:
|
| </View> | ||
| </> | ||
| ), | ||
| closeButtonVisible: true, | ||
| style: styles.popoverWithIcon, | ||
| }, | ||
| render: (args) => <PopoverContentCore {...args} />, |
There was a problem hiding this comment.
note: These render wrappers are included to solve a known SB issue with reusable stories + CSF 3 (I added a comment in another part to point to this ticket): See storybookjs/storybook#15954 (comment)
| @@ -106,8 +108,9 @@ export const WithIcon: StoryComponentType = { | |||
| args: { | |||
| title: "Popover with Icon", | |||
| content: "Popovers can include images on the left.", | |||
| icon: <img src="/logo.svg" width="100%" alt="Wonder Blocks logo" />, | |||
| icon: <img src="./logo.svg" width="100%" alt="Wonder Blocks logo" />, | |||
There was a problem hiding this comment.
note: Adding relative paths to fix some issues in the production version of SB (these images are broken in there).
| }; | ||
|
|
||
| export default { | ||
| children: { | ||
| description: |
There was a problem hiding this comment.
note: Added descriptions to improve the Popover documentation in SB.
| * these attributes. Also, make sure to assign the `${id}-title` prop to the | ||
| * `title` element and `${id}-content` prop to the `content` element. | ||
| */ | ||
| export const CustomPopoverContent: StoryComponentType = { |
There was a problem hiding this comment.
note: I noticed that we didn't include a story with custom popovers, so I added it here. This also helps to manually test custom popovers with screen readers.
Summary:
The Popover dialog element is not assigning correctly the
aria-describedbyvalue, thus causing Screen Readers not being able to read the popover contents
correctly.
This PR fixes the issue by assigning the
aria-describedbyvalue to the popovercontent, and also associates the popover title with the popover dialog via the
aria-labelledbyattribute. By implementing this, the popover dialog will beread correctly by Screen Readers.
Also fixed some issues with the Storybook docs by adding correct descriptions
and fixing some of the Arg types to the Popover component.
Issue: WB-1636
Test plan:
This is better to be tested with Safari + VoiceOver.
title should be read as soon as the popover dialog is opened.
BEFORE
AFTER
