Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(action-popover): add aria-labelledby and aria-describedby props #7115

Merged
merged 2 commits into from
Dec 11, 2024
Merged

feat(action-popover): add aria-labelledby and aria-describedby props #7115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6267cb5
feat(action-popover): add aria-labelledby and aria-describedby props
nuria1110 Dec 5, 2024
4966712
Merge branch 'master' into FE-7015
nuria1110 Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...onents/action-popover/action-popover-menu-button/action-popover-menu-button.component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { IconType } from "../../icon";
export type ActionPopoverMenuButtonAria = {
"aria-haspopup": string;
"aria-label": string;
nineteen88 marked this conversation as resolved.
Show resolved Hide resolved
"aria-labelledby"?: string;
"aria-describedby"?: string;
"aria-controls": string;
"aria-expanded": string;
};
Expand Down Expand Up @@ -41,6 +43,7 @@ export const ActionPopoverMenuButton = ({
iconPosition,
size,
children,
ariaAttributes,
...props
}: ActionPopoverMenuButtonProps) => (
<MenuButtonOverrideWrapper>
Expand All @@ -49,6 +52,7 @@ export const ActionPopoverMenuButton = ({
iconType={iconType}
iconPosition={iconPosition}
size={size}
{...ariaAttributes}
{...props}
>
{children}
Expand Down
52 changes: 51 additions & 1 deletion src/components/action-popover/action-popover-test.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
ActionPopoverItem,
ActionPopoverMenu,
ActionPopoverProps,
ActionPopoverMenuButton,
} from ".";
import {
FlatTable,
Expand All @@ -21,7 +22,7 @@ import Box from "../box";

export default {
title: "Action Popover/Test",
includeStories: ["Default", "LongMenuExample"],
includeStories: ["Default", "LongMenuExample", "WithAriaAttributes"],
parameters: {
info: { disable: true },
chromatic: {
Expand Down Expand Up @@ -800,3 +801,52 @@ export const LongMenuExample = () => {
</Box>
);
};

export const WithAriaAttributes = () => {
return (
<>
<span id="test-label-id">
Instead of "actions", this should be the label
</span>
<br />
<span id="test-description-id">This should be the description</span>

<ActionPopover
aria-labelledby="test-label-id"
aria-describedby="test-description-id"
>
<ActionPopoverItem>example item</ActionPopoverItem>
</ActionPopover>

<span id="foo-label-id">Instead of "Foo", this should be the label </span>
<br />
<span id="foo-description-id">
This should be the description for Foo
</span>

<ActionPopover
aria-labelledby="foo-label-id"
aria-describedby="foo-description-id"
renderButton={({
tabIndex,
"data-element": dataElement,
ariaAttributes,
}) => (
<ActionPopoverMenuButton
buttonType="tertiary"
iconType="dropdown"
iconPosition="after"
size="small"
tabIndex={tabIndex}
data-element={dataElement}
ariaAttributes={ariaAttributes}
>
Foo
</ActionPopoverMenuButton>
)}
>
<ActionPopoverItem onClick={() => {}}>foo</ActionPopoverItem>
</ActionPopover>
</>
);
};
12 changes: 12 additions & 0 deletions src/components/action-popover/action-popover.component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export interface RenderButtonProps {
ariaAttributes: {
"aria-haspopup": string;
"aria-label": string;
nineteen88 marked this conversation as resolved.
Show resolved Hide resolved
"aria-labelledby"?: string;
"aria-describedby"?: string;
"aria-controls": string;
"aria-expanded": string;
};
Expand All @@ -62,6 +64,10 @@ export interface ActionPopoverProps extends MarginProps {
rightAlignMenu?: boolean;
/** Prop to specify an aria-label for the component */
"aria-label"?: string;
/** Prop to specify an aria-labelledby for the component */
"aria-labelledby"?: string;
/** Prop to specify an aria-describedby for the component */
"aria-describedby"?: string;
}

const onOpenDefault = () => {};
Expand All @@ -78,6 +84,8 @@ export const ActionPopover = ({
horizontalAlignment = "left",
submenuPosition = "left",
"aria-label": ariaLabel,
"aria-labelledby": ariaLabelledBy,
"aria-describedby": ariaDescribedBy,
...rest
}: ActionPopoverProps) => {
const l = useLocale();
Expand Down Expand Up @@ -233,6 +241,8 @@ export const ActionPopover = ({
ariaAttributes: {
"aria-haspopup": "true",
"aria-label": ariaLabel || l.actionPopover.ariaLabel(),
nineteen88 marked this conversation as resolved.
Show resolved Hide resolved
"aria-labelledby": ariaLabelledBy,
"aria-describedby": ariaDescribedBy,
"aria-controls": menuID,
"aria-expanded": `${isOpen}`,
},
Expand All @@ -244,6 +254,8 @@ export const ActionPopover = ({
role="button"
aria-haspopup="true"
aria-label={ariaLabel || l.actionPopover.ariaLabel()}
aria-labelledby={ariaLabelledBy}
aria-describedby={ariaDescribedBy}
aria-controls={menuID}
aria-expanded={isOpen}
tabIndex={isOpen ? -1 : 0}
Expand Down
11 changes: 10 additions & 1 deletion src/components/action-popover/action-popover.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,16 @@ Menu items can also be displayed without icons.
### With custom menu button

It is possible to use the `renderButton` prop to override the default button used to trigger the opening and closing of
ActionPopoverMenu. Expand the code example below for how to use this.
ActionPopoverMenu.

The `renderButton` prop is a function that allows consumers to pass `tabIndex`, `data-element` and the provided aria attribute props to
`ActionPopoverMenuButton` or a custom button component.

A default `aria-label` will be provided to the button, but this can be overridden by passing the `ariaLabel` prop to the
`ActionPopover` or making use of the `actionPopover.ariaLabel` translation key. Please ensure to set this to `undefined` if your
button contains visible text, as this will prevent screen readers from reading the visible label.

See the example below for an example of how to use the `renderButton` prop:

<Canvas of={ActionPopoverStories.CustomMenuButton} />

Expand Down
26 changes: 26 additions & 0 deletions src/components/action-popover/action-popover.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ export const CustomMenuButton: Story = () => {
tabIndex={tabIndex}
data-element={dataElement}
ariaAttributes={ariaAttributes}
aria-label={undefined}
>
More
</ActionPopoverMenuButton>
Expand All @@ -232,6 +233,31 @@ export const CustomMenuButton: Story = () => {
Delete
</ActionPopoverItem>
</ActionPopover>
<ActionPopover
renderButton={({
tabIndex,
"data-element": dataElement,
ariaAttributes,
}) => (
<ActionPopoverMenuButton
buttonType="tertiary"
iconType="dropdown"
iconPosition="after"
size="small"
tabIndex={tabIndex}
data-element={dataElement}
ariaAttributes={ariaAttributes}
/>
)}
>
<ActionPopoverItem icon="email" onClick={() => {}}>
Email Invoice
</ActionPopoverItem>
<ActionPopoverDivider />
<ActionPopoverItem onClick={() => {}} icon="delete">
Delete
</ActionPopoverItem>
</ActionPopover>
<ActionPopover
renderButton={({ "data-element": dataElement }) => (
<Link onClick={() => {}} data-element={dataElement}>
Expand Down
84 changes: 83 additions & 1 deletion src/components/action-popover/action-popover.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,32 @@ test("uses the aria-label prop if provided", () => {
expect(screen.getByRole("button")).toHaveAccessibleName("test aria label");
});

test("renders with the provided aria-labelledby prop", () => {
render(
<>
<span id="test-label-id">test label</span>
<ActionPopover aria-labelledby="test-label-id">
<ActionPopoverItem>example item</ActionPopoverItem>
</ActionPopover>
</>,
);
expect(screen.getByRole("button")).toHaveAccessibleName("test label");
});

test("renders with the provided aria-describedby prop", () => {
render(
<>
<span id="test-description-id">test description</span>
<ActionPopover aria-describedby="test-description-id">
<ActionPopoverItem>example item</ActionPopoverItem>
</ActionPopover>
</>,
);
expect(screen.getByRole("button")).toHaveAccessibleDescription(
"test description",
);
});

test("renders with the menu closed by default", () => {
render(
<ActionPopover>
Expand Down Expand Up @@ -1825,7 +1851,7 @@ test("an error is thrown, with appropriate error message, if an invalid element
globalConsoleSpy.mockRestore();
});

test("an error is thrown, with appropriate error message, if a submenu has incorrecr children", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: good tidying up

test("an error is thrown, with appropriate error message, if a submenu has incorrect children", async () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

const globalConsoleSpy = jest
Expand Down Expand Up @@ -1911,6 +1937,62 @@ describe("when the renderButton prop is passed", () => {

expect(menuButton).toHaveAttribute("tabindex", "-1");
});

it("renders the menu button with the provided aria-label", () => {
render(
<ActionPopover
renderButton={(props) => (
<ActionPopoverMenuButton
buttonType="tertiary"
iconType="dropdown"
iconPosition="after"
size="small"
aria-label="test label"
{...props}
>
Foo
</ActionPopoverMenuButton>
)}
>
<ActionPopoverItem onClick={jest.fn()}>foo</ActionPopoverItem>
</ActionPopover>,
);

const menuButton = screen.getByRole("button");
expect(menuButton).toBeVisible();
expect(menuButton).toHaveAccessibleName("test label");
});

it("renders the menu button with the provided aria-labelledby and aria-describedby", () => {
render(
<>
<span id="test-label-id">test label</span>
<span id="test-description-id">test description</span>
<ActionPopover
aria-labelledby="test-label-id"
aria-describedby="test-description-id"
renderButton={(props) => (
<ActionPopoverMenuButton
buttonType="tertiary"
iconType="dropdown"
iconPosition="after"
size="small"
{...props}
>
Foo
</ActionPopoverMenuButton>
)}
>
<ActionPopoverItem onClick={() => {}}>foo</ActionPopoverItem>
</ActionPopover>
</>,
);

const menuButton = screen.getByRole("button");
expect(menuButton).toBeVisible();
expect(menuButton).toHaveAccessibleName("test label");
expect(menuButton).toHaveAccessibleDescription("test description");
});
});

describe("When ActionPopoverMenu contains multiple disabled items", () => {
Expand Down
Loading