From 38aaed9f7abaa7c5fe7750d9f9a61b60c3a3b0f3 Mon Sep 17 00:00:00 2001 From: DipperTheDan Date: Tue, 8 Oct 2024 09:35:09 +0100 Subject: [PATCH] fix(action-popover): ensure that opening using the up arrow focuses last element in the menu Currently we have a bug whereby when a user presses the up arrow key, the first focusable item in the menu is focused. This appears to be a bug as we have code that specifically handles this behaviour but is overridden elsewhere in the code. This fix ensures that the last focusable item is focused. fixes #6826 --- .../__internal__/action-popover-utils.tsx | 30 +++++ .../action-popover-item.component.tsx | 7 +- .../action-popover-menu.component.tsx | 62 +++-------- .../action-popover-test.stories.tsx | 96 +++++++++++++++- .../action-popover.component.tsx | 28 +++-- .../action-popover/action-popover.pw.tsx | 16 +++ .../action-popover/action-popover.stories.tsx | 2 +- .../action-popover/action-popover.test.tsx | 103 ++++++++++++++++-- .../action-popover/components.test-pw.tsx | 93 ++++++++++++++++ 9 files changed, 367 insertions(+), 70 deletions(-) create mode 100644 src/components/action-popover/__internal__/action-popover-utils.tsx diff --git a/src/components/action-popover/__internal__/action-popover-utils.tsx b/src/components/action-popover/__internal__/action-popover-utils.tsx new file mode 100644 index 0000000000..ed524cbd0d --- /dev/null +++ b/src/components/action-popover/__internal__/action-popover-utils.tsx @@ -0,0 +1,30 @@ +import React from "react"; +import { ActionPopoverItem } from "../action-popover-item/action-popover-item.component"; + +// Reusable type alias for item types +type ReactItem = React.ReactChild | React.ReactFragment | React.ReactPortal; + +export const getItems = ( + children: React.ReactNode | React.ReactNode[] +): ReactItem[] => + React.Children.toArray(children).filter( + (child) => React.isValidElement(child) && child.type === ActionPopoverItem + ); + +export const isItemDisabled = (item: ReactItem) => + React.isValidElement(item) && !!item.props?.disabled; + +export const findFirstFocusableItem = (items: ReactItem[]): number => + items.findIndex((_, index) => !isItemDisabled(items[index])); + +// FIX-ME: FE-6248 +// Once we no longer support Node 16, this function can be removed and `findLastIndex()` can be used in its place. +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLastIndex +export const findLastFocusableItem = (items: ReactItem[]): number => { + for (let i = items.length - 1; i >= 0; i--) { + if (!isItemDisabled(items[i])) { + return i; + } + } + return -1; +}; diff --git a/src/components/action-popover/action-popover-item/action-popover-item.component.tsx b/src/components/action-popover/action-popover-item/action-popover-item.component.tsx index c2dfc15464..05276f426c 100644 --- a/src/components/action-popover/action-popover-item/action-popover-item.component.tsx +++ b/src/components/action-popover/action-popover-item/action-popover-item.component.tsx @@ -216,10 +216,13 @@ export const ActionPopoverItem = ({ } }, [alignSubmenu, submenu]); - // focuses item on opening of actionPopover submenu + // Focuses item on opening of actionPopover submenu, but we want to do this once the Popover has finished opening + // We always want the focused item to be in the user's view for accessibility purposes, and without the initial unexpected scroll to top of page when used in a table. useEffect(() => { if (focusItem) { - ref.current?.focus({ preventScroll: true }); + setTimeout(() => { + ref.current?.focus(); + }, 0); } }, [focusItem]); diff --git a/src/components/action-popover/action-popover-menu/action-popover-menu.component.tsx b/src/components/action-popover/action-popover-menu/action-popover-menu.component.tsx index 36abe6021a..cf6a914483 100644 --- a/src/components/action-popover/action-popover-menu/action-popover-menu.component.tsx +++ b/src/components/action-popover/action-popover-menu/action-popover-menu.component.tsx @@ -1,10 +1,4 @@ -import React, { - useCallback, - useMemo, - useContext, - useState, - useEffect, -} from "react"; +import React, { useCallback, useMemo, useContext, useState } from "react"; import invariant from "invariant"; import { Menu } from "../action-popover.style"; @@ -16,6 +10,12 @@ import ActionPopoverDivider from "../action-popover-divider/action-popover-divid import ActionPopoverContext, { Alignment, } from "../__internal__/action-popover.context"; +import { + findFirstFocusableItem, + findLastFocusableItem, + getItems, + isItemDisabled, +} from "../__internal__/action-popover-utils"; export interface ActionPopoverMenuBaseProps { /** Children for the menu */ @@ -120,46 +120,16 @@ const ActionPopoverMenu = React.forwardRef< ` and \`${ActionPopoverDivider.displayName}\`.` ); - const items = useMemo(() => { - return React.Children.toArray(children).filter((child) => { - return React.isValidElement(child) && child.type === ActionPopoverItem; - }); - }, [children]); + const items = useMemo(() => getItems(children), [children]); - const isItemDisabled = useCallback( - (value: number) => { - const item = items[value]; - // The invariant will be triggered before this else path can be explored, hence the ignore else. - // istanbul ignore else - return React.isValidElement(item) && item.props.disabled; - }, + const checkItemDisabled = useCallback( + (value) => isItemDisabled(items[value]), [items] ); - const firstFocusableItem = items.findIndex( - (_, index) => !isItemDisabled(index) - ); - - // FIX-ME: FE-6248 - // Once we no longer support Node 16, this function can be removed and `findLastIndex()` can be used in it's place. - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLastIndex - function findLastFocusableItem() { - let lastFocusableItem = -1; - for (let i = items.length - 1; i >= 0; i--) { - if (!isItemDisabled(i)) { - lastFocusableItem = i; - break; - } - } - return lastFocusableItem; - } - - const lastFocusableItem = findLastFocusableItem(); + const firstFocusableItem = findFirstFocusableItem(items); - useEffect(() => { - if (isOpen && firstFocusableItem !== -1) - setFocusIndex(firstFocusableItem); - }, [isOpen, firstFocusableItem, setFocusIndex]); + const lastFocusableItem = findLastFocusableItem(items); const onKeyDown = useCallback( (e) => { @@ -173,7 +143,7 @@ const ActionPopoverMenu = React.forwardRef< e.preventDefault(); e.stopPropagation(); let indexValue = focusIndex + 1; - while (indexValue < items.length && isItemDisabled(indexValue)) { + while (indexValue < items.length && checkItemDisabled(indexValue)) { indexValue += 1; } if (indexValue >= items.length) { @@ -187,7 +157,7 @@ const ActionPopoverMenu = React.forwardRef< let indexValue = focusIndex - 1; while ( indexValue >= firstFocusableItem && - isItemDisabled(indexValue) + checkItemDisabled(indexValue) ) { indexValue -= 1; } @@ -216,7 +186,7 @@ const ActionPopoverMenu = React.forwardRef< items.forEach((item, index) => { if ( React.isValidElement(item) && - !isItemDisabled(index) && + !checkItemDisabled(index) && item.props.children.toLowerCase().startsWith(e.key.toLowerCase()) ) { // istanbul ignore else @@ -241,7 +211,7 @@ const ActionPopoverMenu = React.forwardRef< setOpen, focusIndex, items, - isItemDisabled, + checkItemDisabled, setFocusIndex, firstFocusableItem, lastFocusableItem, diff --git a/src/components/action-popover/action-popover-test.stories.tsx b/src/components/action-popover/action-popover-test.stories.tsx index a27f87a77e..24524d6f4c 100644 --- a/src/components/action-popover/action-popover-test.stories.tsx +++ b/src/components/action-popover/action-popover-test.stories.tsx @@ -17,10 +17,11 @@ import { FlatTableHeader, FlatTableCell, } from "../flat-table"; +import Box from "../box"; export default { title: "Action Popover/Test", - includeStories: ["Default"], + includeStories: ["Default", "LongMenuExample"], parameters: { info: { disable: true }, chromatic: { @@ -706,3 +707,96 @@ export const ActionPopoverPropsComponentAllDisabled = ( ); }; + +export const LongMenuExample = () => { + const submenu = ( + + {}}>Sub Menu 1 + {}}>Sub Menu 2 + {}}> + Sub Menu 3 + + + ); + const submenuWithIcons = ( + + {}}> + Sub Menu 1 + + {}}> + Sub Menu 2 + + {}}> + Sub Menu 3 + + + ); + return ( + + {}} onClose={() => {}}> + {}} + > + Business + + {}}> + Email Invoiceee + + {}} submenu={submenu}> + Print Invoice + + {}}> + Download PDF + + {}}> + Download CSV + + {}}> + Email Invoiceee + + {}} submenu={submenu}> + Print Invoice + + {}}> + Download PDF + + {}}> + Download CSV + + {}}> + Email Invoiceee + + {}} submenu={submenu}> + Print Invoice + + {}}> + Download PDF + + {}}> + Download CSV + + + {}}> + Delete + + + + {}}> + Download CSV + + + + {}} + > + Download CSV + + + + ); +}; diff --git a/src/components/action-popover/action-popover.component.tsx b/src/components/action-popover/action-popover.component.tsx index 203bec8fc2..0ecd2dec76 100644 --- a/src/components/action-popover/action-popover.component.tsx +++ b/src/components/action-popover/action-popover.component.tsx @@ -24,6 +24,11 @@ import ActionPopoverContext, { Alignment, } from "./__internal__/action-popover.context"; import useModalManager from "../../hooks/__internal__/useModalManager"; +import { + findFirstFocusableItem, + findLastFocusableItem, + getItems, +} from "./__internal__/action-popover-utils"; export interface RenderButtonProps { tabIndex: number; @@ -82,12 +87,6 @@ export const ActionPopover = ({ const buttonRef = useRef(null); const menu = useRef(null); - const itemCount = useMemo(() => { - return React.Children.toArray(children).filter((child) => { - return React.isValidElement(child) && child.type === ActionPopoverItem; - }).length; - }, [children]); - const hasProperChildren = useMemo(() => { const incorrectChild = React.Children.toArray(children).find( (child: React.ReactNode) => { @@ -105,6 +104,12 @@ export const ActionPopover = ({ return !incorrectChild; }, [children]); + const items = useMemo(() => getItems(children), [children]); + + const firstFocusableItem = findFirstFocusableItem(items); + + const lastFocusableItem = findLastFocusableItem(items); + invariant( hasProperChildren, `ActionPopover only accepts children of type \`${ActionPopoverItem.displayName}\`` + @@ -152,13 +157,14 @@ export const ActionPopover = ({ (e) => { e.stopPropagation(); const isOpening = !isOpen; + setFocusIndex(firstFocusableItem); setOpen(isOpening); if (!isOpening) { // Closing the menu should focus the MenuButton focusButton(); } }, - [isOpen, setOpen, focusButton] + [isOpen, firstFocusableItem, setOpen, focusButton] ); // Keyboard commands implemented as recommended by WAI-ARIA best practices @@ -169,16 +175,16 @@ export const ActionPopover = ({ if (Events.isSpaceKey(e) || Events.isDownKey(e) || Events.isEnterKey(e)) { e.preventDefault(); e.stopPropagation(); - setFocusIndex(0); + setFocusIndex(firstFocusableItem); setOpen(true); } else if (Events.isUpKey(e)) { e.preventDefault(); e.stopPropagation(); - setFocusIndex(itemCount - 1); + setFocusIndex(lastFocusableItem); setOpen(true); } }, - [itemCount, setOpen] + [firstFocusableItem, lastFocusableItem, setOpen] ); const handleEscapeKey = useCallback( @@ -200,7 +206,7 @@ export const ActionPopover = ({ useEffect(() => { const handler = ({ target }: MouseEvent) => { - // If the event didn't came from part of this component, close the menu. + // If the event didn't come from part of this component, close the menu. // There will be multiple document click listeners but we cant prevent propagation because it will interfere with // other instances on the same page diff --git a/src/components/action-popover/action-popover.pw.tsx b/src/components/action-popover/action-popover.pw.tsx index fdfac80ccb..0107a8093f 100644 --- a/src/components/action-popover/action-popover.pw.tsx +++ b/src/components/action-popover/action-popover.pw.tsx @@ -65,6 +65,7 @@ import { Submenu, SubmenuPositionedRight, ActionPopoverNestedInDialog, + LongMenuExample, } from "../../../src/components/action-popover/components.test-pw"; const keyToTrigger = ["Enter", " ", "End", "ArrowDown", "ArrowUp"] as const; @@ -103,6 +104,21 @@ test.describe("check functionality for ActionPopover component", () => { }); }); + test("should open and focus last element when up keyboard key is used to open menu", async ({ + mount, + page, + }) => { + await mount(); + await page.setViewportSize({ width: 600, height: 600 }); + + const actionPopoverButtonElement = actionPopoverButton(page).first(); + await actionPopoverButtonElement.press("ArrowUp"); + const focusedElement = await page.locator("*:focus"); + + await expect(focusedElement).toContainText("Download CSV"); + await expect(focusedElement).toBeVisible(); + }); + [keyToTrigger[0], keyToTrigger[1], keyToTrigger[3]].forEach((key) => { // TODO: Skipped due to flaky focus behaviour. To review in FE-6428 test.skip(`should open using ${key} keyboard key`, async ({ diff --git a/src/components/action-popover/action-popover.stories.tsx b/src/components/action-popover/action-popover.stories.tsx index ce6214d0a3..34ed29197f 100644 --- a/src/components/action-popover/action-popover.stories.tsx +++ b/src/components/action-popover/action-popover.stories.tsx @@ -86,7 +86,7 @@ export const Default: Story = () => { Download CSV - {}}> + {}}> Delete diff --git a/src/components/action-popover/action-popover.test.tsx b/src/components/action-popover/action-popover.test.tsx index 30b5ac038f..efc30e9b65 100644 --- a/src/components/action-popover/action-popover.test.tsx +++ b/src/components/action-popover/action-popover.test.tsx @@ -31,7 +31,7 @@ afterAll(() => { }); afterEach(() => { - jest.runAllTimers(); + jest.runOnlyPendingTimers(); }); testStyledSystemMarginRTL( @@ -450,6 +450,7 @@ test("clicking the menu button focuses the first focusable element", async () => ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item" })).toBeFocused(); }); @@ -524,6 +525,7 @@ test.each(["ArrowDown", "Space", "Enter", "ArrowUp"] as const)( screen.getByRole("button").focus(); const userEventKeyCode = key === "Space" ? " " : `{${key}}`; await user.keyboard(userEventKeyCode); + jest.runOnlyPendingTimers(); expect(screen.getByRole("list")).toBeVisible(); expect(onOpen).toHaveBeenCalledTimes(1); @@ -545,6 +547,7 @@ test.each(["ArrowDown", "Space", "Enter"] as const)( screen.getByRole("button").focus(); const userEventKeyCode = key === "Space" ? " " : `{${key}}`; await user.keyboard(userEventKeyCode); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 1" }) @@ -552,9 +555,7 @@ test.each(["ArrowDown", "Space", "Enter"] as const)( } ); -// test left in, but skipped, pending investigation/decision on https://github.com/Sage/carbon/issues/6826 -// eslint-disable-next-line jest/no-disabled-tests -test.skip("pressing ArrowUp key when focused on the menu button selects the last focusable item", async () => { +test("pressing ArrowUp key when focused on the menu button selects the last focusable item", async () => { const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); render( @@ -566,6 +567,7 @@ test.skip("pressing ArrowUp key when focused on the menu button selects the last screen.getByRole("button").focus(); await user.keyboard("{ArrowUp}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 2" })).toBeFocused(); }); @@ -589,12 +591,17 @@ test.each([ screen.getByRole("button").focus(); await user.keyboard("{Enter}"); + jest.runOnlyPendingTimers(); + + expect( + screen.getByRole("button", { name: "example item 1" }) + ).toBeFocused(); await user.keyboard(keycode); + jest.runOnlyPendingTimers(); expect(screen.queryByRole("list")).not.toBeInTheDocument(); expect(onClose).toHaveBeenCalledTimes(1); - // TODO - add test for correct element being focused if functionality is fixed (see FE-6424) } ); @@ -628,15 +635,19 @@ test("pressing the Down Arrow key when the menu is open focuses the next item an ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 2" })).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 3" })).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); }); @@ -652,15 +663,19 @@ test("pressing the Up Arrow key when the menu is open focuses the previous item ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); await user.keyboard("{ArrowUp}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 3" })).toBeFocused(); await user.keyboard("{ArrowUp}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 2" })).toBeFocused(); await user.keyboard("{ArrowUp}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); }); @@ -677,24 +692,34 @@ test("pressing the Home key when the menu is open focuses the first item, no mat ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 2" })).toBeFocused(); await user.keyboard("{Home}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 3" })).toBeFocused(); await user.keyboard("{Home}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 4" })).toBeFocused(); await user.keyboard("{Home}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); }); @@ -711,20 +736,34 @@ test("pressing the End key when the menu is open focuses the last item, no matte ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); await user.keyboard("{End}"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "example item 4" })).toBeFocused(); await user.keyboard("{ArrowUp}"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "example item 3" })).toBeFocused(); await user.keyboard("{End}"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "example item 4" })).toBeFocused(); await user.keyboard("{ArrowUp}"); + jest.runOnlyPendingTimers(); + await user.keyboard("{ArrowUp}"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "example item 2" })).toBeFocused(); await user.keyboard("{End}"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "example item 4" })).toBeFocused(); }); @@ -740,11 +779,13 @@ test("pressing Space when the menu is open does nothing", async () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect(screen.getByRole("list")).toBeVisible(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); await user.keyboard(" "); + jest.runOnlyPendingTimers(); expect(screen.getByRole("list")).toBeVisible(); expect(screen.getByRole("button", { name: "example item 1" })).toBeFocused(); @@ -764,21 +805,30 @@ test("pressing an alphabet character when the menu is open selects the next sele ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); // moves to first element starting with P await user.keyboard("p"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "Print Invoice" })).toBeFocused(); // moves to next element starting with D - noting that it skips the disabled "Disabled" item await user.keyboard("d"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "Download CSV" })).toBeFocused(); // moves to next element starting with D, it loops to the start await user.keyboard("d"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "Download PDF" })).toBeFocused(); // does nothing when there are no matches await user.keyboard("z"); + jest.runOnlyPendingTimers(); + expect(screen.getByRole("button", { name: "Download PDF" })).toBeFocused(); }); @@ -794,11 +844,13 @@ test("pressing a non-printable character key when the menu is open does nothing" ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect(screen.getByRole("list")).toBeVisible(); expect(screen.getByRole("button", { name: "first item" })).toBeFocused(); await user.keyboard("{F1}"); + jest.runOnlyPendingTimers(); expect(screen.getByRole("list")).toBeVisible(); expect(screen.getByRole("button", { name: "first item" })).toBeFocused(); @@ -892,7 +944,7 @@ describe("when an item has a submenu with default (left) alignment", () => { ); act(() => { - jest.runAllTimers(); + jest.runOnlyPendingTimers(); }); expect( @@ -969,7 +1021,7 @@ describe("when an item has a submenu with default (left) alignment", () => { screen.getByRole("button", { name: "example item with submenu" }) ); act(() => { - jest.runAllTimers(); + jest.runOnlyPendingTimers(); }); expect( @@ -978,7 +1030,7 @@ describe("when an item has a submenu with default (left) alignment", () => { await user.hover(screen.getByRole("button", { name: "example item 2" })); act(() => { - jest.runAllTimers(); + jest.runOnlyPendingTimers(); }); expect( @@ -1056,9 +1108,11 @@ describe("when an item has a submenu with default (left) alignment", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); screen.getByRole("button", { name: "example item with submenu" }).focus(); await user.keyboard("{ArrowLeft}"); + jest.runOnlyPendingTimers(); const firstItem = screen.getByRole("button", { name: "submenu item 1" }); expect(firstItem).toBeVisible(); @@ -1127,15 +1181,18 @@ describe("when an item has a submenu with default (left) alignment", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); screen.getByRole("button", { name: "example item with submenu" }).focus(); await user.keyboard("{ArrowLeft}"); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "submenu item 1" }) ).toBeVisible(); await user.keyboard("z"); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "submenu item 1" }) @@ -1166,9 +1223,11 @@ describe("when an item has a submenu with default (left) alignment", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); screen.getByRole("button", { name: "example item with submenu" }).focus(); await user.keyboard("{Enter}"); + jest.runOnlyPendingTimers(); const firstItem = screen.getByRole("button", { name: "submenu item 1" }); expect(firstItem).toBeVisible(); @@ -1227,14 +1286,20 @@ describe("when an item has a submenu with default (left) alignment", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); + screen.getByRole("button", { name: "example item with submenu" }).focus(); await user.keyboard("{ArrowLeft}"); + jest.runOnlyPendingTimers(); + expect( screen.getByRole("button", { name: "submenu item 1" }) ).toBeFocused(); await user.keyboard("{Escape}"); + jest.runOnlyPendingTimers(); + expect(screen.queryByRole("list")).not.toBeInTheDocument(); expect(screen.getByRole("button")).toBeFocused(); }); @@ -1271,7 +1336,7 @@ describe("when an item has a submenu with default (left) alignment", () => { ); act(() => { - jest.runAllTimers(); + jest.runOnlyPendingTimers(); }); expect( @@ -1449,9 +1514,11 @@ describe("when there isn't enough space on the screen to render a submenu on the ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); screen.getByRole("button", { name: "example item with submenu" }).focus(); await user.keyboard("{ArrowRight}"); + jest.runOnlyPendingTimers(); const firstItem = screen.getByRole("button", { name: "submenu item 1" }); expect(firstItem).toBeVisible(); @@ -1556,9 +1623,11 @@ describe("when the submenuPosition prop is set to 'right'", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); screen.getByRole("button", { name: "example item with submenu" }).focus(); await user.keyboard("{ArrowRight}"); + jest.runOnlyPendingTimers(); const firstItem = screen.getByRole("button", { name: "submenu item 1" }); expect(firstItem).toBeVisible(); @@ -1680,9 +1749,11 @@ describe("when the submenuPosition prop is set to 'right' and there isn't enough ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); screen.getByRole("button", { name: "example item with submenu" }).focus(); await user.keyboard("{ArrowLeft}"); + jest.runOnlyPendingTimers(); const firstItem = screen.getByRole("button", { name: "submenu item 1" }); expect(firstItem).toBeVisible(); @@ -1858,11 +1929,13 @@ describe("When ActionPopoverMenu contains multiple disabled items", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 1" }) ).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 4" }) @@ -1884,15 +1957,19 @@ describe("When ActionPopoverMenu contains multiple disabled items", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 1" }) ).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); + expect( screen.getByRole("button", { name: "example item 4" }) ).toBeFocused(); await user.keyboard("{ArrowUp}"); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 1" }) @@ -1914,19 +1991,25 @@ describe("When ActionPopoverMenu contains multiple disabled items", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 2" }) ).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); + expect( screen.getByRole("button", { name: "example item 4" }) ).toBeFocused(); await user.keyboard("{ArrowDown}"); + jest.runOnlyPendingTimers(); + expect( screen.getByRole("button", { name: "example item 5" }) ).toBeFocused(); await user.keyboard("{Home}"); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 2" }) @@ -1948,11 +2031,13 @@ describe("When ActionPopoverMenu contains multiple disabled items", () => { ); await user.click(screen.getByRole("button")); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 2" }) ).toBeFocused(); await user.keyboard("{End}"); + jest.runOnlyPendingTimers(); expect( screen.getByRole("button", { name: "example item 5" }) diff --git a/src/components/action-popover/components.test-pw.tsx b/src/components/action-popover/components.test-pw.tsx index 02811b6ae3..4a6af6541a 100644 --- a/src/components/action-popover/components.test-pw.tsx +++ b/src/components/action-popover/components.test-pw.tsx @@ -1268,3 +1268,96 @@ export const ActionPopoverNestedInDialog = () => { ); }; + +export const LongMenuExample = () => { + const submenu = ( + + {}}>Sub Menu 1 + {}}>Sub Menu 2 + {}}> + Sub Menu 3 + + + ); + const submenuWithIcons = ( + + {}}> + Sub Menu 1 + + {}}> + Sub Menu 2 + + {}}> + Sub Menu 3 + + + ); + return ( + + {}} onClose={() => {}}> + {}} + > + Business + + {}}> + Email Invoiceee + + {}} submenu={submenu}> + Print Invoice + + {}}> + Download PDF + + {}}> + Download CSV + + {}}> + Email Invoiceee + + {}} submenu={submenu}> + Print Invoice + + {}}> + Download PDF + + {}}> + Download CSV + + {}}> + Email Invoiceee + + {}} submenu={submenu}> + Print Invoice + + {}}> + Download PDF + + {}}> + Download CSV + + + {}}> + Delete + + + + {}}> + Download CSV + + + + {}} + > + Download CSV + + + + ); +};