From bd252b701b991b3092effeffb15dfb2067637948 Mon Sep 17 00:00:00 2001 From: DipperTheDan Date: Thu, 5 Dec 2024 13:45:04 +0000 Subject: [PATCH 1/2] chore(focus-trap): address spelling mistake in code comment --- src/__internal__/focus-trap/focus-trap.component.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__internal__/focus-trap/focus-trap.component.tsx b/src/__internal__/focus-trap/focus-trap.component.tsx index 751d2e7998..8432ba6b42 100644 --- a/src/__internal__/focus-trap/focus-trap.component.tsx +++ b/src/__internal__/focus-trap/focus-trap.component.tsx @@ -279,7 +279,7 @@ const FocusTrap = ({ onFocus: updateCurrentFocusedElement, }); - // passes focusProps, sets tabIndex and onBlur if no tabIndex has been expicitly set on child + // passes focusProps, sets tabIndex and onBlur if no tabIndex has been explicitly set on child const clonedChildren = React.Children.map(children, (child) => { const focusableChild = child as React.ReactElement; return React.cloneElement( From a843fe8be5df9ad728f6ee235c3e2c848808711c Mon Sep 17 00:00:00 2001 From: DipperTheDan Date: Thu, 5 Dec 2024 13:48:04 +0000 Subject: [PATCH 2/2] fix(popover-container): ensure that tab sequence is not lost when container 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 --- .../popover-container/components.test-pw.tsx | 29 +++++++ .../popover-container-test.stories.tsx | 31 +++++++ .../popover-container.component.tsx | 51 ++++++++++- .../popover-container.pw.tsx | 20 +++++ .../popover-container.test.tsx | 85 +++++++++++++++++++ 5 files changed, 215 insertions(+), 1 deletion(-) diff --git a/src/components/popover-container/components.test-pw.tsx b/src/components/popover-container/components.test-pw.tsx index 4c624a366f..646437080d 100644 --- a/src/components/popover-container/components.test-pw.tsx +++ b/src/components/popover-container/components.test-pw.tsx @@ -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"; @@ -434,3 +435,31 @@ export const PopoverContainerFocusOrder = ( ); }; + +export const WithRadioButtons = () => { + const [open1, setOpen1] = useState(false); + return ( + + setOpen1(true)} + onClose={() => setOpen1(false)} + open={open1} + renderOpenComponent={({ ref, onClick }) => ( + + )} + p={0} + > + + + + + + + + + + ); +}; diff --git a/src/components/popover-container/popover-container-test.stories.tsx b/src/components/popover-container/popover-container-test.stories.tsx index 2286ca54f6..338e758cea 100644 --- a/src/components/popover-container/popover-container-test.stories.tsx +++ b/src/components/popover-container/popover-container-test.stories.tsx @@ -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", @@ -22,6 +23,7 @@ export default { "InsideMenu", "InsideMenuWithOpenButton", "WithFullWidthButton", + "WithRadioButtons", ], parameters: { info: { disable: true }, @@ -246,3 +248,32 @@ export const WithFullWidthButton = () => { ); }; + +export const WithRadioButtons = () => { + const [open1, setOpen1] = useState(false); + + return ( + + setOpen1(true)} + onClose={() => setOpen1(false)} + open={open1} + renderOpenComponent={({ ref, onClick }) => ( + + )} + p={0} + > + + + + + + + + + + ); +}; diff --git a/src/components/popover-container/popover-container.component.tsx b/src/components/popover-container/popover-container.component.tsx index 5889c3b4ae..5d707157de 100644 --- a/src/components/popover-container/popover-container.component.tsx +++ b/src/components/popover-container/popover-container.component.tsx @@ -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; @@ -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, + ) => { + 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, @@ -335,7 +368,23 @@ export const PopoverContainer = ({ ) : ( - popover() + <> +
handleFocusGuard("prev", ev)} + /> + {popover()} +
handleFocusGuard("next", ev)} + /> + ); return ( diff --git a/src/components/popover-container/popover-container.pw.tsx b/src/components/popover-container/popover-container.pw.tsx index 1b939d41fb..1021f0e5ce 100644 --- a/src/components/popover-container/popover-container.pw.tsx +++ b/src/components/popover-container/popover-container.pw.tsx @@ -34,6 +34,7 @@ import { WithRenderCloseButtonComponent, PopoverContainerComponentCoverButton, PopoverContainerFocusOrder, + WithRadioButtons, } from "../popover-container/components.test-pw"; import Portrait from "../portrait"; @@ -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(); + + 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, diff --git a/src/components/popover-container/popover-container.test.tsx b/src/components/popover-container/popover-container.test.tsx index e5efb70905..60b8eff0db 100644 --- a/src/components/popover-container/popover-container.test.tsx +++ b/src/components/popover-container/popover-container.test.tsx @@ -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"); @@ -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( + <> + ( + + )} + > + + + + + + + , + ); + + 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( + <> + ( + + )} + > + + + + + + + , + ); + + 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( + <> + + , + ); + + 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) => (