Skip to content

Commit

Permalink
fix(LEMS-2225): add conditional aria-describedby logic (#2283)
Browse files Browse the repository at this point in the history 
* adding aria described by changes

* adding changeset.

* update logic and story

* remove aria label condition for aria described by

* update units tests

* update custom aria described by story

* lint fix

* update changeset

* updatet test to be more consistent

* update test to include additional check

* update test to split out assertions

---------

Co-authored-by: Cat Johnson <[email protected]>
  • Loading branch information
@anakaren-rojas @catandthemachines
anakaren-rojas and catandthemachines authored Aug 1, 2024
1 parent 513e85c commit 166ae1d
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/plenty-rockets-breathe.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-popover": patch
---

Adding logic for aria-describedby
61 changes: 58 additions & 3 deletions __docs__/wonder-blocks-popover/popover.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import Button from "@khanacademy/wonder-blocks-button";
import {View} from "@khanacademy/wonder-blocks-core";
import {Strut} from "@khanacademy/wonder-blocks-layout";
import {color, spacing} from "@khanacademy/wonder-blocks-tokens";
import {LabelLarge} from "@khanacademy/wonder-blocks-typography";
import {HeadingMedium, LabelLarge} from "@khanacademy/wonder-blocks-typography";
import type {Placement} from "@khanacademy/wonder-blocks-tooltip";

import {Popover, PopoverContent} from "@khanacademy/wonder-blocks-popover";
Expand Down Expand Up @@ -87,6 +87,16 @@ const styles = StyleSheet.create({
flexDirection: "row",
gap: spacing.medium_16,
},
srOnly: {
border: 0,
clip: "rect(0,0,0,0)",
height: 1,
margin: -1,
overflow: "hidden",
padding: 0,
position: "absolute",
width: 1,
},
});

type StoryComponentType = StoryObj<typeof Popover>;
Expand Down Expand Up @@ -726,10 +736,10 @@ export const PopoverAlignment: StoryComponentType = {
};

/**
* With custom aria-label - overrides the default aria-describedby and aria-labelledby
* With custom aria-label - overrides the default aria-labelledby
*/

export const CustomAriaLabel: StoryComponentType = {
export const WithCustomAriaLabel: StoryComponentType = {
args: {
children: <Button>Open popover</Button>,
content: ContentMappings.withTextOnly,
Expand All @@ -742,3 +752,48 @@ export const CustomAriaLabel: StoryComponentType = {
"aria-label": "Popover with custom aria label",
} as PopoverArgs,
};

/**
* With custom aria-describedby - overrides the default aria-describedby
*/
export const WithCustomAriaDescribedBy = ({
placement,
}: {
placement: Placement;
}) => {
const [opened, setOpened] = React.useState(false);

return (
<View style={styles.example}>
<Popover
aria-describedby="custom-popover-description"
placement={placement}
opened={opened}
onClose={() => setOpened(false)}
content={
<>
<HeadingMedium
id="custom-popover-description"
style={styles.srOnly}
>
Hidden text that would describe the popover content
</HeadingMedium>
<PopoverContent
title="Title"
content="Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip commodo."
closeButtonVisible
/>
</>
}
>
<Button
onClick={() => {
setOpened(true);
}}
>
{`Open popover`}
</Button>
</Popover>
</View>
);
};
107 changes: 92 additions & 15 deletions packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,17 +583,21 @@ describe("Popover", () => {
).toBeInTheDocument();
});

it("should announce a popover correctly by reading the aria label", async () => {
it("should announce a popover correctly by reading the aria-label attribute", async () => {
// Arrange
render(
<Popover
id="test-popover"
onClose={jest.fn()}
aria-label="Popover Aria Label"
content={
<PopoverContentCore>
<button data-close-button onClick={close}>
Close Popover
</button>
<PopoverContentCore closeButtonVisible={true}>
<h1 id="test-popover-title">
This is a popover title
</h1>
<p id="test-popover-content">
This is a popover description
</p>
</PopoverContentCore>
}
>
Expand All @@ -602,19 +606,92 @@ describe("Popover", () => {
);

// Act
// Open the popover
const openButton = await screen.findByRole("button", {
name: "Open default popover",
});

await userEvent.click(openButton);
const popover = await screen.findByRole("dialog");
await userEvent.click(
await screen.findByRole("button", {
name: "Open default popover",
}),
);

// Assert
expect(
await screen.findByRole("dialog", {
name: "Popover Aria Label",
}),
).toBeInTheDocument();
});

it("should announce a popover correctly by reading the title contents", async () => {
// Arrange
render(
<Popover
id="test-popover"
onClose={jest.fn()}
aria-describedby="describing-popover-id"
content={
<PopoverContentCore closeButtonVisible={true}>
<h1 id="test-popover-title">
This is a popover title
</h1>
<p id="describing-popover-id">
This is a popover description
</p>
</PopoverContentCore>
}
>
<Button>Open default popover</Button>
</Popover>,
);

expect(popover).toHaveAttribute("aria-label", "Popover Aria Label");
expect(popover).not.toHaveAttribute("aria-labelledby");
expect(popover).not.toHaveAttribute("aria-describedby");
// Act
await userEvent.click(
await screen.findByRole("button", {
name: "Open default popover",
}),
);

//Assert
expect(
await screen.findByRole("dialog", {
name: "This is a popover title",
}),
).toBeInTheDocument();
});

it("should announce a popover correctly by reading the describing contents", async () => {
// Arrange
render(
<Popover
id="test-popover"
onClose={jest.fn()}
aria-describedby="describing-popover-id"
content={
<PopoverContentCore closeButtonVisible={true}>
<h1 id="test-popover-title">
This is a popover title
</h1>
<p id="describing-popover-id">
This is a popover description
</p>
</PopoverContentCore>
}
>
<Button>Open default popover</Button>
</Popover>,
);

// Act
await userEvent.click(
await screen.findByRole("button", {
name: "Open default popover",
}),
);

//Assert
expect(
await screen.findByRole("dialog", {
description: "This is a popover description",
}),
).toBeInTheDocument();
});

it("should correctly describe the popover content core's aria label", async () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/wonder-blocks-popover/src/components/popover.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,12 @@ export default class Popover extends React.Component<Props, State> {
showTail,
portal,
"aria-label": ariaLabel,
"aria-describedby": ariaDescribedBy,
} = this.props;
const {anchorElement} = this.state;

const ariaDescribedBy = ariaLabel ? undefined : `${uniqueId}-content`;
const describedBy = ariaDescribedBy || `${uniqueId}-content`;

const ariaLabelledBy = ariaLabel ? undefined : `${uniqueId}-title`;

const popperContent = (
Expand All @@ -290,7 +292,7 @@ export default class Popover extends React.Component<Props, State> {
<PopoverDialog
{...props}
aria-label={ariaLabel}
aria-describedby={ariaDescribedBy}
aria-describedby={describedBy}
aria-labelledby={ariaLabelledBy}
id={uniqueId}
onUpdate={(placement) => this.setState({placement})}
Expand Down

0 comments on commit 166ae1d

Please sign in to comment.