diff --git a/.changeset/dirty-dingos-battle.md b/.changeset/dirty-dingos-battle.md new file mode 100644 index 000000000..c82f45e0a --- /dev/null +++ b/.changeset/dirty-dingos-battle.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-popover": patch +--- + +Pass the correct ID to the title and content so screen readers can announce popovers correctly diff --git a/__docs__/wonder-blocks-popover/popover-content-core.stories.tsx b/__docs__/wonder-blocks-popover/popover-content-core.stories.tsx index 9383d2934..0a81e6b27 100644 --- a/__docs__/wonder-blocks-popover/popover-content-core.stories.tsx +++ b/__docs__/wonder-blocks-popover/popover-content-core.stories.tsx @@ -34,15 +34,6 @@ export default { version={packageConfig.version} /> ), - docs: { - description: { - component: null, - }, - source: { - // See https://github.com/storybookjs/storybook/issues/12596 - excludeDecorators: true, - }, - }, }, decorators: [ (Story: any): React.ReactElement> => ( @@ -94,14 +85,17 @@ export const WithIcon: StoryComponentType = { <> - This is an article - With the content + + This is an article + + With the content ), closeButtonVisible: true, style: styles.popoverWithIcon, }, + render: (args) => , }; // NOTE: Adding a wrapper to cast the component so Storybook doesn't complain. @@ -116,6 +110,7 @@ export const WithDetailCell: StoryComponentType = { children: , style: styles.popoverWithCell, }, + render: (args) => , }; WithDetailCell.parameters = { @@ -179,6 +174,7 @@ export const Dark: StoryComponentType = { color: "darkBlue", style: styles.customPopover, }, + render: (args) => , }; Dark.parameters = { diff --git a/__docs__/wonder-blocks-popover/popover-content.stories.tsx b/__docs__/wonder-blocks-popover/popover-content.stories.tsx index 359176717..bdb91db65 100644 --- a/__docs__/wonder-blocks-popover/popover-content.stories.tsx +++ b/__docs__/wonder-blocks-popover/popover-content.stories.tsx @@ -59,6 +59,7 @@ export const Default: StoryComponentType = { content: "The default version only includes text.", closeButtonVisible: true, }, + render: (args) => , }; Default.storyName = "Default (text)"; @@ -89,6 +90,7 @@ export const Emphasized: StoryComponentType = { ), }, + render: (args) => , }; Emphasized.parameters = { @@ -106,8 +108,9 @@ export const WithIcon: StoryComponentType = { args: { title: "Popover with Icon", content: "Popovers can include images on the left.", - icon: Wonder Blocks logo, + icon: Wonder Blocks logo, }, + render: (args) => , }; WithIcon.parameters = { @@ -125,7 +128,7 @@ export const WithIllustration: StoryComponentType = { "As you can see, this popover includes a full-bleed illustration.", image: ( An illustration of a person skating on a pencil , }; WithIllustration.parameters = { diff --git a/__docs__/wonder-blocks-popover/popover.accessibility.stories.mdx b/__docs__/wonder-blocks-popover/popover.accessibility.stories.mdx index 6d50052bb..94e714496 100644 --- a/__docs__/wonder-blocks-popover/popover.accessibility.stories.mdx +++ b/__docs__/wonder-blocks-popover/popover.accessibility.stories.mdx @@ -32,9 +32,12 @@ The Popover component should follow these guidelines: **Popover Dialog:** The popover component will populate the +[aria-labelledby](https://www.w3.org/TR/wai-aria/#aria-labelledby) and [aria-describedby](https://www.w3.org/TR/wai-aria-1.1/#aria-describedby) -attribute automatically, unless the user sets an `id` prop inside the Popover -instance. Internally, it will be set on the trigger element. +attributes automatically. Internally, it will be set in the `dialog` element and +the `PopoverContent` component to reference the `title` and `content` +elements/props. By doing this, the popover will be announced by screen readers +as a dialog with a title and content. ## Keyboard Interaction diff --git a/__docs__/wonder-blocks-popover/popover.argtypes.tsx b/__docs__/wonder-blocks-popover/popover.argtypes.tsx index 10ba93b68..69cbec8f5 100644 --- a/__docs__/wonder-blocks-popover/popover.argtypes.tsx +++ b/__docs__/wonder-blocks-popover/popover.argtypes.tsx @@ -14,42 +14,103 @@ import { } from "./popover-content-core.stories"; // NOTE: Casting to any to avoid type errors. -const DefaultWrapper = Default as React.ElementType; -const EmphasizedWrapper = Emphasized as React.ElementType; -const WithIconWrapper = WithIcon as React.ElementType; -const WithIllustrationWrapper = WithIllustration as React.ElementType; -const CoreWithIconWrapper = CoreWithIcon as React.ElementType; -const CoreWithDetailCellWrapper = CoreWithDetailCell as React.ElementType; -const CoreDarkWrapper = CoreDark as React.ElementType; +const DefaultWrapper = Default as any; +const EmphasizedWrapper = Emphasized as any; +const WithIconWrapper = WithIcon as any; +const WithIllustrationWrapper = WithIllustration as any; +const CoreWithIconWrapper = CoreWithIcon as any; +const CoreWithDetailCellWrapper = CoreWithDetailCell as any; +const CoreDarkWrapper = CoreDark as any; -export const ContentMappings: { - [key: string]: React.ReactNode; -} = { - withTextOnly: , - withEmphasis: , - withIcon: , - withIllustration: , - coreWithIcon: , - coreWithCell: , - coreDark: , +// NOTE: We have to use the `render` method to fix a bug in Storybook where +// reusable stories don't render properly with CSF v3. +// See https://github.com/storybookjs/storybook/issues/15954#issuecomment-1835905271 +export const ContentMappings = { + withTextOnly: DefaultWrapper.render({...Default.args}), + withEmphasis: EmphasizedWrapper.render({...Emphasized.args}), + withIcon: WithIconWrapper.render({...WithIcon.args}), + withIllustration: WithIllustrationWrapper.render({ + ...WithIllustration.args, + }), + coreWithIcon: CoreWithIconWrapper.render({...CoreWithIcon.args}), + coreWithCell: CoreWithDetailCellWrapper.render({ + ...CoreWithDetailCell.args, + }), + coreDark: CoreDarkWrapper.render({...CoreDark.args}), }; export default { children: { + description: + `The element that triggers the popover. This element will be ` + + `used to position the popover. It can be either a Node or a ` + + `function using the children-as-function pattern to pass an open ` + + `function for use anywhere within children. The latter provides ` + + `a lot of flexibility in terms of what actions may trigger the ` + + `\`Popover\` to launch the popover dialog.`, control: { type: null, }, }, placement: { + description: + `Where the popover should try to appear in relation to the ` + + `trigger element.,`, control: { type: "select", options: ["top", "bottom", "right", "left"], }, }, content: { + description: + `The content of the popover. You can either use ` + + `[PopoverContent](../?path=/docs/popover-popovercontent--docs) ` + + `with one of the pre-defined variants, or include your own ` + + `custom content using ` + + `[PopoverContentCore](../?path=/docs/popover-popovercontentcore--docs) ` + + `directly.`, control: {type: "select"}, defaultValue: ContentMappings.withTextOnly, options: Object.keys(ContentMappings) as Array, mapping: ContentMappings, }, + id: { + description: + `The unique identifier to give to the popover. Provide ` + + `this in cases where you want to override the default accessibility ` + + `solution. This identifier will be applied to the popover title and ` + + `content.\n\n` + + `This is also used as a prefix to the IDs of the popover's elements. ` + + `For example, if you pass \`"my-popover"\` as the ID, the popover ` + + `title will have the ID \`"my-popover-title"\` and the popover ` + + `content will have the ID \`"my-popover-content"\`.`, + }, + initialFocusId: { + description: + `The selector for the element that will be focused when the ` + + `popover content shows. When not set, the first focusable ` + + `element within the popover content will be used.`, + }, + opened: { + description: + `When true, the popover content is shown.\n\n` + + `Using this prop makes the component behave as a controlled ` + + `component. The parent is responsible for managing the ` + + `opening/closing of the popover when using this prop.`, + control: { + type: "boolean", + }, + }, + onClose: { + description: `Called when the popover content is closed.`, + }, + testId: { + description: `Test ID used for e2e testing.`, + }, + showTail: { + description: `Whether to show the popover tail or not.`, + control: { + type: "boolean", + }, + }, }; diff --git a/__docs__/wonder-blocks-popover/popover.stories.tsx b/__docs__/wonder-blocks-popover/popover.stories.tsx index 4257ea9fd..43b05c594 100644 --- a/__docs__/wonder-blocks-popover/popover.stories.tsx +++ b/__docs__/wonder-blocks-popover/popover.stories.tsx @@ -13,8 +13,31 @@ import {Popover, PopoverContent} from "@khanacademy/wonder-blocks-popover"; import packageConfig from "../../packages/wonder-blocks-popover/package.json"; import ComponentInfo from "../../.storybook/components/component-info"; -import PopoverArgtypes from "./popover.argtypes"; +import PopoverArgtypes, {ContentMappings} from "./popover.argtypes"; +/** + * Popovers provide additional information that is related to a particular + * element and/or content. They can include text, links, icons and + * illustrations. The main difference with `Tooltip` is that they must be + * dismissed by clicking an element. + * + * This component uses the `PopoverPopper` component to position the + * `PopoverContentCore` component according to the children it is wrapping. + * + * ### Usage + * + * ```jsx + * import {Popover, PopoverContent} from "@khanacademy/wonder-blocks-popover"; + * + * {}} + * content={ + * + * }> + * + * + * ``` + */ export default { title: "Popover/Popover", component: Popover as unknown as React.ComponentType, @@ -26,15 +49,6 @@ export default { version={packageConfig.version} /> ), - docs: { - description: { - component: null, - }, - source: { - // See https://github.com/storybookjs/storybook/issues/12596 - excludeDecorators: true, - }, - }, // TODO(WB-1170): Reassess this after investigating more about Chromatic // flakyness. chromatic: { @@ -43,9 +57,7 @@ export default { }, decorators: [ (Story): React.ReactElement> => ( - - - + {Story()} ), ], } as Meta; @@ -78,14 +90,7 @@ type PopoverArgs = Partial; export const Default: StoryComponentType = { args: { children: , - content: ( - - ), - + content: ContentMappings.withTextOnly, placement: "top", dismissEnabled: true, id: "", @@ -338,6 +343,23 @@ WithInitialFocusId.parameters = { }, }; +/** + * Popovers can have custom layouts. This is done by using the + * `PopoverContentCore` component. + * + * _NOTE:_ If you choose to use this component, you'll have to set the + * `aria-labelledby` and `aria-describedby` attributes manually. Make sure to + * pass the `id` prop to the `Popover` component and use it as the value for + * 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 = { + args: { + children: , + content: ContentMappings.coreWithIcon, + id: "custom-popover", + } as PopoverArgs, +}; /** * Alignment example */ diff --git a/packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx b/packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx index f9f979ed8..a2be8682a 100644 --- a/packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx +++ b/packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx @@ -287,4 +287,73 @@ describe("Popover", () => { // Assert expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); + + describe("a11y", () => { + it("should announce a popover correctly by reading the title contents", async () => { + // Arrange + render( + + } + > + + , + ); + + // Act + // Open the popover + userEvent.click( + screen.getByRole("button", {name: "Open default popover"}), + ); + + // Assert + expect( + screen.getByRole("dialog", { + name: "The title is read by the screen reader", + }), + ).toBeInTheDocument(); + }); + + it("should announce a custom popover correctly by reading the title contents", async () => { + // Arrange + render( + +

+ This is a custom popover title +

+

+ The custom popover description +

+ + } + > + + , + ); + + // Act + // Open the popover + userEvent.click( + screen.getByRole("button", {name: "Open default popover"}), + ); + + // Assert + expect( + screen.getByRole("dialog", { + name: "This is a custom popover title", + }), + ).toBeInTheDocument(); + }); + }); }); diff --git a/packages/wonder-blocks-popover/src/components/popover-content-core.tsx b/packages/wonder-blocks-popover/src/components/popover-content-core.tsx index effed1cd6..90642be65 100644 --- a/packages/wonder-blocks-popover/src/components/popover-content-core.tsx +++ b/packages/wonder-blocks-popover/src/components/popover-content-core.tsx @@ -132,9 +132,10 @@ const styles = StyleSheet.create({ * elements */ closeButton: { + margin: 0, position: "absolute", - right: Spacing.xSmall_8, - top: Spacing.xSmall_8, + right: Spacing.xxxSmall_4, + top: Spacing.xxxSmall_4, // Allows the button to be above the title and/or custom content zIndex: 1, }, diff --git a/packages/wonder-blocks-popover/src/components/popover-content.tsx b/packages/wonder-blocks-popover/src/components/popover-content.tsx index 98973bbfd..6807c43da 100644 --- a/packages/wonder-blocks-popover/src/components/popover-content.tsx +++ b/packages/wonder-blocks-popover/src/components/popover-content.tsx @@ -47,6 +47,12 @@ type CommonProps = AriaProps & { * Test ID used for e2e testing. */ testId?: string; + /** + * Unique ID for the popover. This is used as a prefix to the IDs of the + * popover's elements. + * @ignore + */ + uniqueId?: string; }; type Props = @@ -209,6 +215,7 @@ export default class PopoverContent extends React.Component { style, title, testId, + uniqueId, } = this.props; return ( @@ -232,10 +239,15 @@ export default class PopoverContent extends React.Component { {this.maybeRenderIcon()} - + {title} - {content} + + {content} + diff --git a/packages/wonder-blocks-popover/src/components/popover-dialog.tsx b/packages/wonder-blocks-popover/src/components/popover-dialog.tsx index d54229f2b..cd3244950 100644 --- a/packages/wonder-blocks-popover/src/components/popover-dialog.tsx +++ b/packages/wonder-blocks-popover/src/components/popover-dialog.tsx @@ -77,6 +77,7 @@ export default class PopoverDialog extends React.Component { style, showTail, "aria-describedby": ariaDescribedby, + "aria-labelledby": ariaLabelledBy, } = this.props; const contentProps = children.props as any; @@ -90,6 +91,7 @@ export default class PopoverDialog extends React.Component { { } }; - renderContent(): PopoverContents { + renderContent(uniqueId: string): PopoverContents { const {content} = this.props; const popoverContents: PopoverContents = @@ -214,7 +210,12 @@ export default class Popover extends React.Component { : content; // @ts-expect-error: TS2769 - No overload matches this call. - return React.cloneElement(popoverContents, {ref: this.contentRef}); + return React.cloneElement(popoverContents, { + ref: this.contentRef, + // internal prop: only injected by Popover + // This allows us to announce the popover content when it is opened. + uniqueId, + }); } renderPopper(uniqueId: string): React.ReactNode { @@ -233,12 +234,13 @@ export default class Popover extends React.Component { {(props: PopperElementProps) => ( this.setState({placement})} showTail={showTail} > - {this.renderContent()} + {this.renderContent(uniqueId)} )}