Skip to content

Commit

Permalink
fix(popover-container): ensure that tab sequence is not lost when con…
Browse files Browse the repository at this point in the history
…tainer has radio buttons

This fix ensures that if the last focusable element in a popover container is a radio button, that
the tab sequence is not affected once focus is outside of the container

fixes #7067
  • Loading branch information
DipperTheDan committed Dec 5, 2024
1 parent bd252b7 commit a843fe8
Show file tree
Hide file tree
Showing 5 changed files with 215 additions and 1 deletion.
29 changes: 29 additions & 0 deletions src/components/popover-container/components.test-pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Pill from "../pill";
import Badge from "../badge";
import Box from "../box";
import { Select, Option } from "../select";
import RadioButton, { RadioButtonGroup } from "../radio-button";
import PopoverContainer, {
PopoverContainerProps,
} from "./popover-container.component";
Expand Down Expand Up @@ -434,3 +435,31 @@ export const PopoverContainerFocusOrder = (
</Box>
);
};

export const WithRadioButtons = () => {
const [open1, setOpen1] = useState(false);
return (
<Box padding="25px" display="inline-flex">
<PopoverContainer
position="left"
onOpen={() => setOpen1(true)}
onClose={() => setOpen1(false)}
open={open1}
renderOpenComponent={({ ref, onClick }) => (
<Button aria-label="open" ref={ref} onClick={onClick}>
With Radio children
</Button>
)}
p={0}
>
<Box display="flex" justifyContent="space-between" p={2}>
<RadioButtonGroup name="bar">
<RadioButton value="1" label="radio 1" />
<RadioButton value="2" label="radio 2" />
</RadioButtonGroup>
</Box>
</PopoverContainer>
<Button>foo</Button>
</Box>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Typography from "../typography";
import Search from "../search";
import IconButton from "../icon-button";
import Icon from "../icon";
import RadioButton, { RadioButtonGroup } from "../radio-button";

export default {
title: "Popover Container/Test",
Expand All @@ -22,6 +23,7 @@ export default {
"InsideMenu",
"InsideMenuWithOpenButton",
"WithFullWidthButton",
"WithRadioButtons",
],
parameters: {
info: { disable: true },
Expand Down Expand Up @@ -246,3 +248,32 @@ export const WithFullWidthButton = () => {
</PopoverContainer>
);
};

export const WithRadioButtons = () => {
const [open1, setOpen1] = useState(false);

return (
<Box padding="25px" display="inline-flex">
<PopoverContainer
position="left"
onOpen={() => setOpen1(true)}
onClose={() => setOpen1(false)}
open={open1}
renderOpenComponent={({ ref, onClick }) => (
<Button aria-label="Notifications" ref={ref} onClick={onClick}>
With Radio children
</Button>
)}
p={0}
>
<Box display="flex" justifyContent="space-between" p={2}>
<RadioButtonGroup name="bar">
<RadioButton value="1" label="radio 1" />
<RadioButton value="2" label="radio 2" />
</RadioButtonGroup>
</Box>
</PopoverContainer>
<Button>foo</Button>
</Box>
);
};
51 changes: 50 additions & 1 deletion src/components/popover-container/popover-container.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import useFocusPortalContent from "../../hooks/__internal__/useFocusPortalConten
import tagComponent, {
TagProps,
} from "../../__internal__/utils/helpers/tags/tags";
import { defaultFocusableSelectors } from "../../__internal__/focus-trap/focus-trap-utils";

export interface RenderOpenProps {
tabIndex: number;
Expand Down Expand Up @@ -272,6 +273,38 @@ export const PopoverContainer = ({
closePopover,
);

const onFocusNextElement = useCallback((ev) => {
const allFocusableElements: HTMLElement[] = Array.from(
document.querySelectorAll(defaultFocusableSelectors) ||
/* istanbul ignore next */ [],
);
const filteredElements = allFocusableElements.filter(
(el) => el === openButtonRef.current || Number(el.tabIndex) !== -1,
);

const openButtonRefIndex = filteredElements.indexOf(
openButtonRef.current as HTMLElement,
);

filteredElements[openButtonRefIndex + 1].focus();
closePopover(ev);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const handleFocusGuard = (
direction: "prev" | "next",
ev: React.FocusEvent<HTMLElement>,
) => {
if (direction === "next" && onFocusNextElement) {
// Focus the next focusable element outside of the popover
onFocusNextElement(ev);
return;
}

// istanbul ignore else
if (direction === "prev") openButtonRef.current?.focus();
};

const renderOpenComponentProps = {
tabIndex: 0,
"aria-expanded": isOpen,
Expand Down Expand Up @@ -335,7 +368,23 @@ export const PopoverContainer = ({
</FocusTrap>
</ModalContext.Provider>
) : (
popover()
<>
<div
data-element="tab-guard-top"
// eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex
tabIndex={0}
aria-hidden
onFocus={(ev) => handleFocusGuard("prev", ev)}
/>
{popover()}
<div
data-element="tab-guard-bottom"
// eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex
tabIndex={0}
aria-hidden
onFocus={(ev) => handleFocusGuard("next", ev)}
/>
</>
);

return (
Expand Down
20 changes: 20 additions & 0 deletions src/components/popover-container/popover-container.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
WithRenderCloseButtonComponent,
PopoverContainerComponentCoverButton,
PopoverContainerFocusOrder,
WithRadioButtons,
} from "../popover-container/components.test-pw";
import Portrait from "../portrait";

Expand Down Expand Up @@ -624,6 +625,25 @@ test.describe("Check props of Popover Container component", () => {
await expect(third).toBeFocused();
});

test("should focus the next focusable element outside of the container once finished keyboard navigating through the container's content", async ({
mount,
page,
}) => {
await mount(<WithRadioButtons />);

const openButton = page.getByRole("button", { name: "open" });
const container = popoverContainerContent(page);
const additionalButton = page.getByRole("button", { name: "foo" });

await openButton.click();
await page.keyboard.press("Tab"); // focus on first radio button
await page.keyboard.press("Tab"); // focus on close icon
await page.keyboard.press("Tab"); // focus outside of container and on to additional button

await expect(container).not.toBeVisible();
await expect(additionalButton).toBeFocused();
});

test.describe("Accessibility tests", () => {
test("should check accessibility for Default example", async ({
mount,
Expand Down
85 changes: 85 additions & 0 deletions src/components/popover-container/popover-container.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { testStyledSystemPadding } from "../../__spec_helper__/__internal__/test
import PopoverContainer from "./popover-container.component";
import { Select, Option } from "../select";
import useMediaQuery from "../../hooks/useMediaQuery";
import Button from "../button";
import RadioButton, { RadioButtonGroup } from "../radio-button";

jest.mock("../../hooks/useMediaQuery");

Expand Down Expand Up @@ -531,6 +533,89 @@ describe("closing the popup", () => {
});
});

test("when content is navigated via keyboard, the next focusable item should be focused and popup closed", async () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
render(
<>
<PopoverContainer
position="left"
renderOpenComponent={({ ref, onClick }) => (
<Button aria-label="open button" ref={ref} onClick={onClick}>
Open
</Button>
)}
>
<RadioButtonGroup name="bar">
<RadioButton value="1" label="radio 1" />
<RadioButton value="2" label="radio 2" />
</RadioButtonGroup>
</PopoverContainer>
<Button>Example Button</Button>
</>,
);

const openButton = screen.getByRole("button", { name: "open button" });
await user.click(openButton);
await user.tab(); // tab to close icon
await user.tab(); // tab to RadioButtonGroup
await user.tab(); // tab to Example Button (outside of popup)

const popup = await screen.findByRole("dialog");
await waitFor(() => expect(popup).not.toBeVisible());

const exampleButton = screen.getByRole("button", { name: "Example Button" });
expect(exampleButton).toHaveFocus();
});

test("when the popover is opened and shift tab key is pressed, the open button should be focused and popup closed", async () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
render(
<>
<PopoverContainer
position="left"
renderOpenComponent={({ ref, onClick }) => (
<Button aria-label="open button" ref={ref} onClick={onClick}>
Open
</Button>
)}
>
<RadioButtonGroup name="bar">
<RadioButton value="1" label="radio 1" />
<RadioButton value="2" label="radio 2" />
</RadioButtonGroup>
</PopoverContainer>
<Button>Example Button</Button>
</>,
);

const openButton = screen.getByRole("button", { name: "open button" });
await user.click(openButton);
await user.tab(); // tab to close icon
await user.tab(); // tab to content
await user.tab({ shift: true }); // shift tab back to close icon
await user.tab({ shift: true }); // shift tab back to the opening trigger element

const popup = await screen.findByRole("dialog");
await waitFor(() => expect(popup).not.toBeVisible());
expect(openButton).toHaveFocus();
});

test("if only the open trigger is the only focusable element on screen, when the popover is opened and tab key is used to navigate content, it should navigate back to the opening trigger", async () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
render(
<>
<PopoverContainer title="My popup" open />
</>,
);

const openButton = screen.getByRole("button", { name: "My popup" });
await user.click(openButton);
await user.tab(); // tab to close icon
await user.tab(); // tab back out of content to the opening trigger element

expect(openButton).toHaveFocus();
});

testStyledSystemPadding(
(props) => (
<PopoverContainer open title="My popup" {...props}>
Expand Down

0 comments on commit a843fe8

Please sign in to comment.