From 26e142a2fb62493942694c2a527230a1dece2ea5 Mon Sep 17 00:00:00 2001 From: nuria1110 Date: Wed, 13 Nov 2024 14:54:06 +0000 Subject: [PATCH] fix(multi-action-button, split-button): ensure screen reader commands can navigate menu popup Fixes issue where screen reader users using VoiceOver could not navigate into the menu buttons using commands VO+Space to open the menu then VO+right to navigate. The first option will now gain focus on open using any interaction method, including onClick, keyboard navigation and screen reader controls. fix #7054 --- .../multi-action-button.component.tsx | 3 +- .../multi-action-button.pw.tsx | 63 +++++++------ .../multi-action-button.stories.tsx | 7 ++ .../multi-action-button.test.tsx | 8 +- .../split-button/split-button.component.tsx | 3 +- .../split-button/split-button.pw.tsx | 38 ++++---- .../split-button/split-button.stories.tsx | 7 ++ .../split-button/split-button.style.ts | 2 +- .../split-button/split-button.test.tsx | 91 +------------------ .../useChildButtons/useChildButtons.tsx | 21 ++--- .../useMenuKeyboardNavigation.test.tsx | 64 +++++-------- .../useMenuKeyboardNavigation.ts | 64 +++++++------ 12 files changed, 151 insertions(+), 220 deletions(-) diff --git a/src/components/multi-action-button/multi-action-button.component.tsx b/src/components/multi-action-button/multi-action-button.component.tsx index e73390e26a..ea9321a4df 100644 --- a/src/components/multi-action-button/multi-action-button.component.tsx +++ b/src/components/multi-action-button/multi-action-button.component.tsx @@ -58,8 +58,6 @@ export const MultiActionButton = ({ const handleClick = ( ev: React.MouseEvent, ) => { - // ensure button is focused when clicked (Safari) - buttonRef.current?.focus(); showButtons(); handleInsideClick(); if (onClick) { @@ -81,6 +79,7 @@ export const MultiActionButton = ({ const renderAdditionalButtons = () => ( { ( [ - ["left", 200], - ["right", 153], + ["left", 0], + ["right", -46], ] as [MultiActionButtonProps["position"], number][] ).forEach(([position, value]) => { test(`should render with menu position to the ${position}`, async ({ @@ -121,7 +121,7 @@ test.describe("Prop tests", () => { await actionButton.click(); const listContainer = getDataElementByValue(page, "additional-buttons"); await expect(listContainer).toHaveCSS("position", "absolute"); - await assertCssValueIsApproximately(listContainer, "top", 45); + await assertCssValueIsApproximately(listContainer, "top", 46); await assertCssValueIsApproximately(listContainer, "left", value); }); }); @@ -193,7 +193,7 @@ test.describe("Prop tests", () => { }); test.describe("Functional tests", () => { - test(`should verify that clicking the main button opens the additional buttons`, async ({ + test(`should verify that clicking the main button opens the additional buttons and focuses the first button`, async ({ mount, page, }) => { @@ -203,9 +203,10 @@ test.describe("Functional tests", () => { .first() .locator("button") .click(); - await expect(getDataElementByValue(page, "additional-buttons")).toHaveCount( - 1, - ); + const additionalButtons = getDataElementByValue(page, "additional-buttons"); + + await expect(additionalButtons).toBeVisible(); + await expect(additionalButtons.getByRole("button").first()).toBeFocused(); }); [...keysToTrigger].forEach((key) => { @@ -220,11 +221,13 @@ test.describe("Functional tests", () => { ); await actionButton.focus(); await page.keyboard.press(key); - await expect( - getDataElementByValue(page, "additional-buttons") - .getByRole("button") - .first(), - ).toBeFocused(); + const additionalButtons = getDataElementByValue( + page, + "additional-buttons", + ); + + await expect(additionalButtons).toBeVisible(); + await expect(additionalButtons.getByRole("button").first()).toBeFocused(); }); }); @@ -271,7 +274,7 @@ test.describe("Functional tests", () => { await expect(listButton1).toBeFocused(); }); - test(`should verify pressing shift + tab moves focus to previous child buttons and to the MultiActionButton if first child button is focused`, async ({ + test(`should verify pressing Shift+Tab moves focus to previous child button, then the main button and closes the list`, async ({ mount, page, }) => { @@ -295,6 +298,8 @@ test.describe("Functional tests", () => { await expect( getComponent(page, "multi-action-button").locator("button"), ).toBeFocused(); + await expect(listButton1).not.toBeVisible(); + await expect(listButton2).not.toBeVisible(); }); test(`should verify pressing ArrowDown key does not loop focus to top`, async ({ @@ -316,7 +321,7 @@ test.describe("Functional tests", () => { await expect(listButton3).toBeFocused(); }); - test(`should verify pressing tab moves focus to next child buttons and to second MultiActionButton if last child button is focused`, async ({ + test(`should verify pressing Tab moves focus to next child button, then closes the list and focuses the next element on the page`, async ({ mount, page, }) => { @@ -336,7 +341,6 @@ test.describe("Functional tests", () => { .getByRole("button") .nth(2); - await page.keyboard.press("Tab"); await expect(listButton1).toBeFocused(); await page.keyboard.press("Tab"); await expect(listButton2).toBeFocused(); @@ -346,6 +350,9 @@ test.describe("Functional tests", () => { await expect( getComponent(page, "multi-action-button").nth(1).locator("button"), ).toBeFocused(); + await expect(listButton1).not.toBeVisible(); + await expect(listButton2).not.toBeVisible(); + await expect(listButton3).not.toBeVisible(); }); test(`should verify that pressing metaKey + ArrowUp moves focus to first child button`, async ({ @@ -434,7 +441,7 @@ test.describe("Functional tests", () => { const listButton3 = getDataElementByValue(page, "additional-buttons") .getByRole("button") .nth(2); - await page.keyboard.press("Tab"); + await expect(listButton1).toBeFocused(); await page.keyboard.down("Meta"); await page.keyboard.press("ArrowDown"); @@ -458,7 +465,7 @@ test.describe("Functional tests", () => { const listButton3 = getDataElementByValue(page, "additional-buttons") .getByRole("button") .nth(2); - await page.keyboard.press("Tab"); + await expect(listButton1).toBeFocused(); await page.keyboard.down("Control"); await page.keyboard.press("ArrowDown"); @@ -482,7 +489,7 @@ test.describe("Functional tests", () => { const listButton3 = getDataElementByValue(page, "additional-buttons") .getByRole("button") .nth(2); - await page.keyboard.press("Tab"); + await expect(listButton1).toBeFocused(); await page.keyboard.press("End"); await expect(listButton3).toBeFocused(); @@ -553,15 +560,15 @@ test.describe("Functional tests with child buttons wrapped in a custom component await expect(listButton1).toBeFocused(); }); - test(`should verify pressing shift + tab moves focus to previous child buttons and to the MultiActionButton if first child button is focused`, async ({ + test(`should verify pressing Shift+Tab moves focus to previous child button, then the main button and closes the list`, async ({ mount, page, }) => { await mount(); const actionButton = getComponent(page, "multi-action-button") - .locator("button") - .first(); + .first() + .locator("button"); await actionButton.click(); const listButton1 = getDataElementByValue(page, "additional-buttons") .getByRole("button") @@ -577,6 +584,8 @@ test.describe("Functional tests with child buttons wrapped in a custom component await expect( getComponent(page, "multi-action-button").locator("button"), ).toBeFocused(); + await expect(listButton1).not.toBeVisible(); + await expect(listButton2).not.toBeVisible(); }); test(`should verify pressing ArrowDown key does not loop focus to top`, async ({ @@ -598,7 +607,7 @@ test.describe("Functional tests with child buttons wrapped in a custom component await expect(listButton3).toBeFocused(); }); - test(`should verify pressing tab moves focus to next child buttons and to second MultiActionButton if last child button is focused`, async ({ + test(`should verify pressing Tab moves focus to next child button, then closes the list and focuses the next element on the page`, async ({ mount, page, }) => { @@ -618,7 +627,6 @@ test.describe("Functional tests with child buttons wrapped in a custom component .getByRole("button") .nth(2); - await page.keyboard.press("Tab"); await expect(listButton1).toBeFocused(); await page.keyboard.press("Tab"); await expect(listButton2).toBeFocused(); @@ -628,6 +636,9 @@ test.describe("Functional tests with child buttons wrapped in a custom component await expect( getComponent(page, "multi-action-button").nth(1).locator("button"), ).toBeFocused(); + await expect(listButton1).not.toBeVisible(); + await expect(listButton2).not.toBeVisible(); + await expect(listButton3).not.toBeVisible(); }); test(`should verify that pressing metaKey + ArrowUp moves focus to first child button`, async ({ @@ -716,7 +727,7 @@ test.describe("Functional tests with child buttons wrapped in a custom component const listButton3 = getDataElementByValue(page, "additional-buttons") .getByRole("button") .nth(2); - await page.keyboard.press("Tab"); + await expect(listButton1).toBeFocused(); await page.keyboard.down("Meta"); await page.keyboard.press("ArrowDown"); @@ -740,7 +751,7 @@ test.describe("Functional tests with child buttons wrapped in a custom component const listButton3 = getDataElementByValue(page, "additional-buttons") .getByRole("button") .nth(2); - await page.keyboard.press("Tab"); + await expect(listButton1).toBeFocused(); await page.keyboard.down("Control"); await page.keyboard.press("ArrowDown"); @@ -764,7 +775,7 @@ test.describe("Functional tests with child buttons wrapped in a custom component const listButton3 = getDataElementByValue(page, "additional-buttons") .getByRole("button") .nth(2); - await page.keyboard.press("Tab"); + await expect(listButton1).toBeFocused(); await page.keyboard.press("End"); await expect(listButton3).toBeFocused(); diff --git a/src/components/multi-action-button/multi-action-button.stories.tsx b/src/components/multi-action-button/multi-action-button.stories.tsx index dfc2302565..4447e2359c 100644 --- a/src/components/multi-action-button/multi-action-button.stories.tsx +++ b/src/components/multi-action-button/multi-action-button.stories.tsx @@ -19,6 +19,13 @@ const meta: Meta = { argTypes: { ...styledSystemProps, }, + decorators: [ + (Story) => ( + + + + ), + ], }; export default meta; diff --git a/src/components/multi-action-button/multi-action-button.test.tsx b/src/components/multi-action-button/multi-action-button.test.tsx index 0174c66758..38c2b22982 100644 --- a/src/components/multi-action-button/multi-action-button.test.tsx +++ b/src/components/multi-action-button/multi-action-button.test.tsx @@ -75,17 +75,21 @@ test("should call 'onClick' when the main button is clicked", async () => { expect(onClick).toHaveBeenCalledTimes(1); }); -test("should open additional buttons when the main button is clicked", async () => { +test("should open additional buttons when the main button is clicked and focus on the first child", async () => { const user = userEvent.setup(); render( + , ); await user.click(screen.getByRole("button", { name: "Main Button" })); - expect(await screen.findByRole("button", { name: "First" })).toBeVisible(); + const button = screen.getByRole("button", { name: "First" }); + + expect(button).toBeVisible(); + expect(button).toHaveFocus(); }); test("should close additional buttons when a child button is clicked", async () => { diff --git a/src/components/split-button/split-button.component.tsx b/src/components/split-button/split-button.component.tsx index b62915bb63..ba7828c67d 100644 --- a/src/components/split-button/split-button.component.tsx +++ b/src/components/split-button/split-button.component.tsx @@ -111,8 +111,6 @@ export const SplitButton = ({ }; const handleToggleClick = () => { - // ensure button is focused when clicked (Safari) - toggleButton.current?.focus(); showButtons(); }; @@ -178,6 +176,7 @@ export const SplitButton = ({ return ( { ( [ - ["left", 200], - ["right", 242], + ["left", 0], + ["right", 42], ] as [SplitButtonProps["position"], number][] ).forEach(([position, value]) => { test(`should render with menu position to the ${position}`, async ({ @@ -277,7 +277,7 @@ test.describe("Prop tests", () => { await getDataElementByValue(page, "dropdown").click(); const listContainer = additionalButtonsContainer(page); await expect(listContainer).toHaveCSS("position", "absolute"); - await assertCssValueIsApproximately(listContainer, "top", 45); + await assertCssValueIsApproximately(listContainer, "top", 46); await assertCssValueIsApproximately(listContainer, "left", value); }); }); @@ -398,18 +398,21 @@ test.describe("Functional tests", () => { await expect(additionalButtonsContainer(page)).not.toBeVisible(); }); - test(`should verify pressing Tab moves focus to next child button and to second SplitButton at end of list`, async ({ + test(`should verify pressing Tab moves focus to next child button, then closes the list and focuses the next element on the page`, async ({ mount, page, }) => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); + await expect(additionalButton(page, 0)).toBeFocused(); await page.keyboard.press("Tab"); await expect(additionalButton(page, 1)).toBeFocused(); await page.keyboard.press("Tab"); + await expect(additionalButton(page, 2)).toBeFocused(); await page.keyboard.press("Tab"); + + await expect(additionalButton(page, 0)).not.toBeVisible(); await expect(mainButton(page).nth(1)).toBeFocused(); }); @@ -420,7 +423,6 @@ test.describe("Functional tests", () => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); await page.keyboard.press("ArrowDown"); await expect(additionalButton(page, 1)).toBeFocused(); await page.keyboard.press("ArrowDown"); @@ -429,7 +431,7 @@ test.describe("Functional tests", () => { await expect(additionalButton(page, 2)).toBeFocused(); }); - test(`should verify pressing Shift+Tab moves focus to previous child button and to main button at start of list`, async ({ + test(`should verify pressing Shift+Tab moves focus to previous child button, then closes the list and focuses the toggle button`, async ({ mount, page, }) => { @@ -440,6 +442,8 @@ test.describe("Functional tests", () => { await page.keyboard.press("Shift+Tab"); await expect(additionalButton(page, 0)).toBeFocused(); await page.keyboard.press("Shift+Tab"); + + await expect(additionalButton(page, 0)).not.toBeVisible(); await expect(splitToggleButton(page).first()).toBeFocused(); }); @@ -504,7 +508,6 @@ test.describe("Functional tests", () => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); await page.keyboard.down("Meta"); await page.keyboard.press("ArrowDown"); await expect(additionalButton(page, 2)).toBeFocused(); @@ -517,7 +520,6 @@ test.describe("Functional tests", () => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); await page.keyboard.down("Control"); await page.keyboard.press("ArrowDown"); await expect(additionalButton(page, 2)).toBeFocused(); @@ -530,7 +532,6 @@ test.describe("Functional tests", () => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); await page.keyboard.press("End"); await expect(additionalButton(page, 2)).toBeFocused(); }); @@ -584,18 +585,21 @@ test.describe("Functional tests in a custom component", () => { await expect(additionalButton(page, 2)).toBeVisible(); }); - test(`should verify pressing Tab moves focus to next child button and to second SplitButton at end of list`, async ({ + test(`should verify pressing Tab moves focus to next child button, then closes the list and focuses the next element on the page`, async ({ mount, page, }) => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); + await expect(additionalButton(page, 0)).toBeFocused(); await page.keyboard.press("Tab"); await expect(additionalButton(page, 1)).toBeFocused(); await page.keyboard.press("Tab"); + await expect(additionalButton(page, 2)).toBeFocused(); await page.keyboard.press("Tab"); + + await expect(additionalButton(page, 0)).not.toBeVisible(); await expect(mainButton(page).nth(1)).toBeFocused(); }); @@ -606,7 +610,6 @@ test.describe("Functional tests in a custom component", () => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); await page.waitForTimeout(1000); await page.keyboard.press("ArrowDown"); await page.keyboard.press("ArrowDown"); @@ -615,7 +618,7 @@ test.describe("Functional tests in a custom component", () => { await expect(additionalButton(page, 2)).toBeFocused(); }); - test(`should verify pressing shift and tab moves focus to previous child button and to main button at start of list`, async ({ + test(`should verify pressing Shift+Tab moves focus to previous child button, then closes the list and focuses the toggle button`, async ({ mount, page, }) => { @@ -626,7 +629,9 @@ test.describe("Functional tests in a custom component", () => { await page.keyboard.press("Shift+Tab"); await expect(additionalButton(page, 0)).toBeFocused(); await page.keyboard.press("Shift+Tab"); - await expect(splitToggleButton(page).first()).toBeFocused(); + + await expect(additionalButton(page, 0)).not.toBeVisible(); + await expect(splitToggleButton(page).nth(0)).toBeFocused(); }); test(`should verify pressing ArrowUp moves focus to previous child button and does not loop when first child is focused`, async ({ @@ -690,7 +695,6 @@ test.describe("Functional tests in a custom component", () => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); await page.keyboard.down("Meta"); await page.keyboard.press("ArrowDown"); await expect(additionalButton(page, 2)).toBeFocused(); @@ -703,7 +707,6 @@ test.describe("Functional tests in a custom component", () => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); await page.keyboard.down("Control"); await page.keyboard.press("ArrowDown"); await expect(additionalButton(page, 2)).toBeFocused(); @@ -716,7 +719,6 @@ test.describe("Functional tests in a custom component", () => { await mount(); await splitToggleButton(page).nth(0).click(); - await additionalButton(page, 0).focus(); await page.keyboard.press("End"); await expect(additionalButton(page, 2)).toBeFocused(); }); diff --git a/src/components/split-button/split-button.stories.tsx b/src/components/split-button/split-button.stories.tsx index df62dd10ac..c1c993c4ab 100644 --- a/src/components/split-button/split-button.stories.tsx +++ b/src/components/split-button/split-button.stories.tsx @@ -18,6 +18,13 @@ const meta: Meta = { argTypes: { ...styledSystemProps, }, + decorators: [ + (Story) => ( + + + + ), + ], }; export default meta; diff --git a/src/components/split-button/split-button.style.ts b/src/components/split-button/split-button.style.ts index 504061934e..e169d3dbf0 100644 --- a/src/components/split-button/split-button.style.ts +++ b/src/components/split-button/split-button.style.ts @@ -8,7 +8,7 @@ const StyledSplitButton = styled.div` display: inline-block; position: relative; - ${StyledButton}:first-of-type { + & > ${StyledButton}:first-of-type { border-top-right-radius: var(--borderRadius000); border-bottom-right-radius: var(--borderRadius000); } diff --git a/src/components/split-button/split-button.test.tsx b/src/components/split-button/split-button.test.tsx index 34efc49118..1553edd964 100644 --- a/src/components/split-button/split-button.test.tsx +++ b/src/components/split-button/split-button.test.tsx @@ -338,7 +338,7 @@ test("should render non-Carbon Button children", async () => { render({spanElement}); await user.click(screen.getByRole("button", { name: "Show more" })); - expect(await screen.findByText("span-element")).toBeInTheDocument(); + expect(await screen.findByText("span-element")).toBeVisible(); }); test("should render with the correct styles when 'buttonType' prop is 'primary' and not disabled", () => { @@ -442,6 +442,7 @@ test("should render the child buttons when a 'Enter' keydown event detected and }); expect(childButton).toBeVisible(); + expect(childButton).toHaveFocus(); }); test("should render the child buttons when a ' ' (space) keydown event detected and toggle button is focused", async () => { @@ -459,44 +460,7 @@ test("should render the child buttons when a ' ' (space) keydown event detected }); expect(childButton).toBeVisible(); -}); - -test("should not hide the additional buttons when already open and 'Enter' key pressed with toggle focused", async () => { - const user = userEvent.setup(); - render( - - - , - ); - - const toggle = screen.getByRole("button", { name: "Show more" }); - await user.click(toggle); - const childButton = await screen.findByRole("button", { - name: "Single Button", - }); - - expect(childButton).toBeVisible(); - await user.keyboard("{Enter}"); - expect(childButton).toBeVisible(); -}); - -test("should not hide the additional buttons when already open and ' ' (space) key pressed with toggle focused", async () => { - const user = userEvent.setup(); - render( - - - , - ); - - const toggle = screen.getByRole("button", { name: "Show more" }); - await user.click(toggle); - const childButton = await screen.findByRole("button", { - name: "Single Button", - }); - - expect(childButton).toBeVisible(); - await user.keyboard(" "); - expect(childButton).toBeVisible(); + expect(childButton).toHaveFocus(); }); test("should render the child buttons when a 'ArrowDown' keydown event detected and toggle button is focused", async () => { @@ -529,7 +493,7 @@ test("should render the child buttons when a 'ArrowUp' keydown event detected an expect(toggle).toHaveFocus(); await user.keyboard("{arrowup}"); const child = await screen.findByRole("button", { name: "Single Button" }); - + expect(child).toBeVisible(); expect(child).toHaveFocus(); }); @@ -679,49 +643,6 @@ test("should hide the additional buttons when a 'Escape' keydown event detected expect(screen.queryByRole("list")).not.toBeInTheDocument(); }); -test("should render with the correct styles when 'align' prop is 'right'", async () => { - const user = userEvent.setup(); - render( - - - , - ); - - const toggle = screen.getByRole("button", { name: "Show more" }); - toggle.focus(); - await user.keyboard("{arrowDown}"); - const childButton = await screen.findByRole("button", { - name: "Single Button", - }); - - expect(childButton).toHaveStyle({ - justifyContent: "right", - textAlign: "right", - }); -}); - -test("should render with the correct styles when 'align' prop is 'left'", async () => { - const user = userEvent.setup(); - - render( - - - , - ); - - const toggle = screen.getByRole("button", { name: "Show more" }); - toggle.focus(); - await user.keyboard("{arrowDown}"); - const childButton = await screen.findByRole("button", { - name: "Single Button", - }); - - expect(childButton).toHaveStyle({ - justifyContent: "left", - textAlign: "left", - }); -}); - test("should call the relevant 'onClick' callback and hide the additional buttons when a child button is clicked", async () => { const user = userEvent.setup(); const onClickMock = jest.fn(); @@ -883,7 +804,7 @@ test("should support navigating to the first child button via home key", async ( expect(button1).toHaveFocus(); }); -test("should support navigating the additional buttons via tab key and hide the list when pressed on last button", async () => { +test("should support navigating the additional buttons via tab key", async () => { const user = userEvent.setup(); render( @@ -912,8 +833,6 @@ test("should support navigating the additional buttons via tab key and hide the await user.tab(); expect(button3).toHaveFocus(); await user.tab(); - - expect(screen.queryByRole("list")).not.toBeInTheDocument(); }); test("should support navigating the additional buttons via shift+tab key, hide the list when pressed on first button and refocus toggle", async () => { diff --git a/src/hooks/__internal__/useChildButtons/useChildButtons.tsx b/src/hooks/__internal__/useChildButtons/useChildButtons.tsx index fdf5898d03..46d03019d8 100644 --- a/src/hooks/__internal__/useChildButtons/useChildButtons.tsx +++ b/src/hooks/__internal__/useChildButtons/useChildButtons.tsx @@ -1,4 +1,4 @@ -import { useState, useRef, useCallback } from "react"; +import { useState, useRef, useCallback, useEffect } from "react"; import Events from "../../../__internal__/utils/helpers/events"; import useMenuKeyboardNavigation from "../useMenuKeyboardNavigation"; @@ -35,6 +35,13 @@ const useChildButtons = ( [], ); + // focus first child button when opened + useEffect(() => { + if (showAdditionalButtons) { + getButtonChildren()?.[0]?.focus(); + } + }, [showAdditionalButtons, getButtonChildren]); + const handleToggleButtonKeyDown = ( ev: React.KeyboardEvent, ) => { @@ -42,19 +49,11 @@ const useChildButtons = ( Events.isEnterKey(ev) || Events.isSpaceKey(ev) || Events.isDownKey(ev) || - Events.isUpKey(ev) || - (showAdditionalButtons && Events.isTabKey(ev)); + Events.isUpKey(ev); if (isToggleKey) { ev.preventDefault(); - - if (!showAdditionalButtons) { - showButtons(); - } - - setTimeout(() => { - getButtonChildren()?.[0]?.focus(); - }); + showButtons(); } }; diff --git a/src/hooks/__internal__/useMenuKeyboardNavigation/useMenuKeyboardNavigation.test.tsx b/src/hooks/__internal__/useMenuKeyboardNavigation/useMenuKeyboardNavigation.test.tsx index 29cc04cfe0..8d8357ec0f 100644 --- a/src/hooks/__internal__/useMenuKeyboardNavigation/useMenuKeyboardNavigation.test.tsx +++ b/src/hooks/__internal__/useMenuKeyboardNavigation/useMenuKeyboardNavigation.test.tsx @@ -127,45 +127,6 @@ describe("useMenuKeyboardNavigation", () => { expect(screen.getByText(`${childButtonID}-0`)).toHaveFocus(); }); - it("pressing Tab key focuses the next child in the list, closes the list when the last one is reached and focuses the next element in the DOM", () => { - const hideCb = jest.fn(); - render(); - - fireEvent.keyDown(screen.getByTestId(containerID), { key: "Tab" }); - expect(screen.getByText(`${childButtonID}-0`)).toHaveFocus(); - fireEvent.keyDown(screen.getByTestId(containerID), { key: "Tab" }); - expect(screen.getByText(`${childButtonID}-1`)).toHaveFocus(); - fireEvent.keyDown(screen.getByTestId(containerID), { key: "Tab" }); - expect(screen.getByText(`${childButtonID}-2`)).toHaveFocus(); - fireEvent.keyDown(screen.getByTestId(containerID), { key: "Tab" }); - jest.runAllTimers(); - expect(hideCb).toHaveBeenCalled(); - expect(screen.getByTestId(nextDOMElementID)).toHaveFocus(); - }); - - it("pressing Shift + Tab key focuses the previous child in the list, closes the list when the first one is reached and focuses the main button", () => { - const hideCb = jest.fn(); - render(); - screen.getByText(`${childButtonID}-2`).focus(); - expect(screen.getByText(`${childButtonID}-2`)).toHaveFocus(); - fireEvent.keyDown(screen.getByTestId(containerID), { - key: "Tab", - shiftKey: true, - }); - expect(screen.getByText(`${childButtonID}-1`)).toHaveFocus(); - fireEvent.keyDown(screen.getByTestId(containerID), { - key: "Tab", - shiftKey: true, - }); - expect(screen.getByText(`${childButtonID}-0`)).toHaveFocus(); - fireEvent.keyDown(screen.getByTestId(containerID), { - key: "Tab", - shiftKey: true, - }); - expect(hideCb).toHaveBeenCalled(); - expect(screen.getByTestId(mainButtonID)).toHaveFocus(); - }); - it("pressing Escape key calls the hide callback and focuses the main button", () => { const hideCb = jest.fn(); render(); @@ -196,6 +157,17 @@ describe("useMenuKeyboardNavigation", () => { expect(keyDownEvent.defaultPrevented).toBe(false); }); + it("pressing Tab key does not trigger prevent default", () => { + const hideCb = jest.fn(); + render(); + const container = screen.getByTestId(containerID); + const keyDownEvent = createEvent.keyDown(container, { key: "Tab" }); + + fireEvent(container, keyDownEvent); + + expect(keyDownEvent.defaultPrevented).toBe(false); + }); + it("pressing ArrowDown key does trigger prevent default", () => { const hideCb = jest.fn(); render(); @@ -239,4 +211,18 @@ describe("useMenuKeyboardNavigation", () => { expect(keyDownEvent.defaultPrevented).toBe(true); }); + + it("calls hide callback when no child button in the menu is focused", () => { + const hideCb = jest.fn(); + render(); + + fireEvent.focus(screen.getByRole("button", { name: "Main Button" })); + + fireEvent.focus(screen.getByText(`${childButtonID}-0`)); + + fireEvent.focus( + screen.getByRole("button", { name: "Next Element in DOM" }), + ); + expect(hideCb).toHaveBeenCalled(); + }); }); diff --git a/src/hooks/__internal__/useMenuKeyboardNavigation/useMenuKeyboardNavigation.ts b/src/hooks/__internal__/useMenuKeyboardNavigation/useMenuKeyboardNavigation.ts index 18b9d07e0f..05679c9fe8 100644 --- a/src/hooks/__internal__/useMenuKeyboardNavigation/useMenuKeyboardNavigation.ts +++ b/src/hooks/__internal__/useMenuKeyboardNavigation/useMenuKeyboardNavigation.ts @@ -1,6 +1,5 @@ -import { useCallback } from "react"; +import { useCallback, useEffect } from "react"; import Events from "../../../__internal__/utils/helpers/events"; -import { defaultFocusableSelectors } from "../../../__internal__/focus-trap/focus-trap-utils"; import useModalManager from "../useModalManager"; export default ( @@ -34,7 +33,9 @@ export default ( const handleKeyDown = useCallback( (ev) => { - if (!(Events.isEnterKey(ev) || Events.isSpaceKey(ev))) { + if ( + !(Events.isEnterKey(ev) || Events.isSpaceKey(ev) || Events.isTabKey(ev)) + ) { ev.preventDefault(); } @@ -75,41 +76,38 @@ export default ( nextIndex = currentIndex + 1; } - const tabPressed = Events.isTabKey(ev); - const tabShiftPressed = tabPressed && Events.isShiftKey(ev); - - if (tabShiftPressed) { - if (currentIndex === 0) { - refocusMainControl(); - } else { - nextIndex = currentIndex - 1; - } - } else if (tabPressed) { - if (currentIndex === (childrenLength as number) - 1) { - const elements = Array.from( - document.querySelectorAll( - defaultFocusableSelectors, - ) as NodeListOf, - ).filter((el) => Number(el.tabIndex) !== -1); - - const indexOf = elements.indexOf( - mainControlRef.current as HTMLButtonElement, - ); - - elements[indexOf + 1]?.focus(); - // timeout enforces that the "hide" method will be run after browser focuses on the next element - setTimeout(hide, 0); - } else { - nextIndex = currentIndex + 1; - } - } - if (nextIndex > -1) { buttonChildren?.[nextIndex]?.focus(); } }, - [hide, refocusMainControl, mainControlRef, getButtonChildren], + [getButtonChildren], ); + // check if a child button is focused, if not hide the menu + const checkFocus = useCallback(() => { + const buttonChildren = getButtonChildren(); + + if (!buttonChildren) { + return; + } + + const buttonChildrenFocused = Array.from(buttonChildren).some( + (button) => button === document.activeElement, + ); + + if (!buttonChildrenFocused) { + hide(); + } + }, [getButtonChildren, hide]); + + useEffect(() => { + document.addEventListener("focusin", checkFocus); + window.addEventListener("blur", hide); + return () => { + document.removeEventListener("focusin", checkFocus); + window.removeEventListener("blur", hide); + }; + }, [checkFocus, hide]); + return handleKeyDown; };