Skip to content

Commit

Permalink
Merge pull request #7115 from Sage/FE-7015
Browse files Browse the repository at this point in the history
feat(action-popover): add aria-labelledby and aria-describedby props
  • Loading branch information
nuria1110 authored Dec 11, 2024
2 parents 9193ac2 + 4966712 commit 34f367f
Show file tree
Hide file tree
Showing 6 changed files with 186 additions and 3 deletions.
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;
"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
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
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;
"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(),
"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
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
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
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 () => {
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

0 comments on commit 34f367f

Please sign in to comment.