diff --git a/CHANGELOG.md b/CHANGELOG.md
index 11ca4d0fc7..3324f9dacc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,17 @@
+### [143.2.4](https://github.com/Sage/carbon/compare/v143.2.3...v143.2.4) (2024-10-18)
+
+
+### Bug Fixes
+
+* **action-popover:** ensure that opening using the up arrow focuses last element in the menu ([38aaed9](https://github.com/Sage/carbon/commit/38aaed9f7abaa7c5fe7750d9f9a61b60c3a3b0f3)), closes [#6826](https://github.com/Sage/carbon/issues/6826)
+
+### [143.2.3](https://github.com/Sage/carbon/compare/v143.2.2...v143.2.3) (2024-10-18)
+
+
+### Bug Fixes
+
+* downgrade @tanstack/react-virtual to version 3.10.1 ([b1cd42f](https://github.com/Sage/carbon/commit/b1cd42ffc258b9fda8db76009876128cd3b7c2bb))
+
### [143.2.2](https://github.com/Sage/carbon/compare/v143.2.1...v143.2.2) (2024-10-16)
diff --git a/package-lock.json b/package-lock.json
index 63b1057083..f038c6adc3 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "carbon-react",
- "version": "143.2.2",
+ "version": "143.2.4",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "carbon-react",
- "version": "143.2.2",
+ "version": "143.2.4",
"hasInstallScript": true,
"license": "Apache-2.0",
"dependencies": {
@@ -14,7 +14,7 @@
"@floating-ui/react-dom": "~1.3.0",
"@octokit/rest": "^18.12.0",
"@styled-system/prop-types": "^5.1.5",
- "@tanstack/react-virtual": "^3.6.0",
+ "@tanstack/react-virtual": "3.10.1",
"@types/styled-system": "^5.1.22",
"chalk": "^4.1.2",
"ci-info": "^3.8.0",
@@ -132,6 +132,7 @@
"jest": "^29.5.0",
"jest-canvas-mock": "^2.5.2",
"jest-environment-jsdom": "^29.5.0",
+ "jest-fail-on-console": "^3.3.1",
"jest-fetch-mock": "^3.0.3",
"jest-styled-components": "^6.3.4",
"jsdom": "^21.1.0",
@@ -6928,11 +6929,12 @@
}
},
"node_modules/@tanstack/react-virtual": {
- "version": "3.10.8",
- "resolved": "https://registry.npmjs.org/@tanstack/react-virtual/-/react-virtual-3.10.8.tgz",
- "integrity": "sha512-VbzbVGSsZlQktyLrP5nxE+vE1ZR+U0NFAWPbJLoG2+DKPwd2D7dVICTVIIaYlJqX1ZCEnYDbaOpmMwbsyhBoIA==",
+ "version": "3.10.1",
+ "resolved": "https://registry.npmjs.org/@tanstack/react-virtual/-/react-virtual-3.10.1.tgz",
+ "integrity": "sha512-h5kNeE+yQwspjl9E3sJ3UYQu/MuspNOBT5cVdc+NA0uU9B1XSkxbzp86teV3arMDVcQ4ESExqs4JyIirYAMcuA==",
+ "license": "MIT",
"dependencies": {
- "@tanstack/virtual-core": "3.10.8"
+ "@tanstack/virtual-core": "3.10.1"
},
"funding": {
"type": "github",
@@ -6944,9 +6946,10 @@
}
},
"node_modules/@tanstack/virtual-core": {
- "version": "3.10.8",
- "resolved": "https://registry.npmjs.org/@tanstack/virtual-core/-/virtual-core-3.10.8.tgz",
- "integrity": "sha512-PBu00mtt95jbKFi6Llk9aik8bnR3tR/oQP1o3TSi+iG//+Q2RTIzCEgKkHG8BB86kxMNW6O8wku+Lmi+QFR6jA==",
+ "version": "3.10.1",
+ "resolved": "https://registry.npmjs.org/@tanstack/virtual-core/-/virtual-core-3.10.1.tgz",
+ "integrity": "sha512-JDi3wU1HIxuxx8BgD7Ix8IXlelCKdTJIh9c0qBs+QXHdix3mjMbkXI3wOq0TuCx1w1RGgzZue34QrM/NPdp/sw==",
+ "license": "MIT",
"funding": {
"type": "github",
"url": "https://github.com/sponsors/tannerlinsley"
@@ -16124,6 +16127,13 @@
"node": "^14.15.0 || ^16.10.0 || >=18.0.0"
}
},
+ "node_modules/jest-fail-on-console": {
+ "version": "3.3.1",
+ "resolved": "https://registry.npmjs.org/jest-fail-on-console/-/jest-fail-on-console-3.3.1.tgz",
+ "integrity": "sha512-dmq/dmh5OBgJlD1MJdpznzwFQP8S7msf3ghTGWQLGhagWwHKzGtqXza76nuJUKOK7BdwqcTK6CCE49Xxv4ckUQ==",
+ "dev": true,
+ "license": "MIT"
+ },
"node_modules/jest-fetch-mock": {
"version": "3.0.3",
"resolved": "https://registry.npmjs.org/jest-fetch-mock/-/jest-fetch-mock-3.0.3.tgz",
diff --git a/package.json b/package.json
index 9191c5410c..5814012819 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
{
"name": "carbon-react",
- "version": "143.2.2",
+ "version": "143.2.4",
"description": "A library of reusable React components for easily building user interfaces.",
"files": [
"lib",
@@ -153,6 +153,7 @@
"jest": "^29.5.0",
"jest-canvas-mock": "^2.5.2",
"jest-environment-jsdom": "^29.5.0",
+ "jest-fail-on-console": "^3.3.1",
"jest-fetch-mock": "^3.0.3",
"jest-styled-components": "^6.3.4",
"jsdom": "^21.1.0",
@@ -185,7 +186,7 @@
"@floating-ui/react-dom": "~1.3.0",
"@octokit/rest": "^18.12.0",
"@styled-system/prop-types": "^5.1.5",
- "@tanstack/react-virtual": "^3.10.8",
+ "@tanstack/react-virtual": "3.10.1",
"@types/styled-system": "^5.1.22",
"chalk": "^4.1.2",
"ci-info": "^3.8.0",
diff --git a/scripts/generate_metadata/generate_metadata.mjs b/scripts/generate_metadata/generate_metadata.mjs
index afd66c07a2..bba9a07e4b 100644
--- a/scripts/generate_metadata/generate_metadata.mjs
+++ b/scripts/generate_metadata/generate_metadata.mjs
@@ -1,4 +1,3 @@
-/* eslint-disable no-console */
import fs from "fs";
import fetch from "node-fetch";
import semver from "semver";
@@ -47,7 +46,7 @@ export const writeFile = (jsonString) => {
if (err) {
throw err;
} else {
- console.log("Successfully created metadata.json file.");
+ global.console.log("Successfully created metadata.json file.");
}
});
};
@@ -58,8 +57,8 @@ export const generateMetadata = async () => {
try {
versions = await fetchVersions();
} catch (err) {
- console.error(err);
- process.exit(1);
+ global.console.error(err);
+ return;
}
const formattedVersions = formatVersions(versions);
diff --git a/scripts/generate_metadata/generate_metadata.spec.js b/scripts/generate_metadata/generate_metadata.spec.js
deleted file mode 100644
index 00d21c3d32..0000000000
--- a/scripts/generate_metadata/generate_metadata.spec.js
+++ /dev/null
@@ -1,118 +0,0 @@
-import { generateMetadata, writeFile } from "./generate_metadata";
-
-const fs = require("fs");
-
-jest.mock("fs");
-
-const mockNpmVersions = {
- versions: {
- "98.0.0": {},
- "99.0.0": {},
- "100.1.1": {},
- "100.2.1": {},
- "101.0.0": {},
- "102.0.0": {},
- },
-};
-
-const mockMetadata = {
- versions: {
- "v102.0.0": "https://carbon.sage.com/v/102.0.0/index.html",
- "v101.0.0": "https://carbon.sage.com/v/101.0.0/index.html",
- "v100.2.1": "https://carbon.sage.com/v/100.2.1/index.html",
- },
-};
-
-describe("generateMetadata script", () => {
- let consoleErrorMock;
-
- beforeAll(() => {
- jest.spyOn(global.console, "log").mockImplementation(() => {});
- consoleErrorMock = jest
- .spyOn(global.console, "error")
- .mockImplementation(() => {});
- });
-
- beforeEach(() => {
- fetch.resetMocks();
- fetch.mockResponse(JSON.stringify(mockNpmVersions));
- fs.mkdirSync = jest.fn((path, options, callback) => {
- callback();
- });
- fs.writeFileSync = jest.fn((path, json, callback) => {
- callback();
- });
- });
-
- afterAll(() => {
- global.console.log.mockReset();
- global.console.error.mockReset();
- });
-
- it("should create a metadata.json file in a metadata directory", async () => {
- await generateMetadata();
-
- expect(fs.mkdirSync).toHaveBeenCalledWith(
- "metadata",
- {},
- expect.any(Function)
- );
-
- expect(fs.writeFileSync).toHaveBeenCalledWith(
- "metadata/metadata.json",
- JSON.stringify(mockMetadata),
- expect.any(Function)
- );
- });
-
- describe("when there is an error creating the metadata directory", () => {
- it("should throw an error", () => {
- fs.mkdirSync = jest.fn((path, options, callback) => {
- callback(new Error("An error occurred."));
- });
-
- expect(() => {
- writeFile();
- }).toThrowError("An error occurred.");
- });
- });
-
- describe("when there is an error creating the metadata.json file", () => {
- it("should throw an error", () => {
- fs.writeFileSync = jest.fn((path, json, callback) => {
- callback(new Error("An error occurred."));
- });
-
- expect(() => {
- writeFile();
- }).toThrowError("An error occurred.");
- });
- });
-
- describe("when there is an error fetching the carbon data from npm", () => {
- const mockExit = jest
- .spyOn(process, "exit")
- .mockImplementation((number) => {
- throw new Error(`process.exit: ${number}`);
- });
-
- afterAll(() => {
- mockExit.mockRestore();
- });
-
- it("should throw an error with exit code 1", async () => {
- fetch.mockResponse(JSON.stringify(mockNpmVersions), {
- status: 500,
- ok: false,
- });
-
- await expect(async () => {
- await generateMetadata();
- }).rejects.toThrowError("process.exit: 1");
- expect(consoleErrorMock).toHaveBeenCalledWith(
- new Error("Failed to fetch from npm with HTTP error code 500")
- );
- expect(mockExit).toHaveBeenCalledWith(1);
- });
- });
-});
diff --git a/scripts/generate_metadata/generate_metadata.test.js b/scripts/generate_metadata/generate_metadata.test.js
new file mode 100644
index 0000000000..55570fc5b6
--- /dev/null
+++ b/scripts/generate_metadata/generate_metadata.test.js
@@ -0,0 +1,102 @@
+import fs from "fs";
+import fetch from "jest-fetch-mock";
+import { generateMetadata, writeFile } from "./generate_metadata";
+
+jest.mock("fs");
+const mockedFs = jest.mocked(fs);
+
+const mockNpmVersions = {
+ versions: {
+ "98.0.0": {},
+ "99.0.0": {},
+ "100.1.1": {},
+ "100.2.1": {},
+ "101.0.0": {},
+ "102.0.0": {},
+ },
+};
+
+const mockMetadata = {
+ versions: {
+ "v102.0.0": "https://carbon.sage.com/v/102.0.0/index.html",
+ "v101.0.0": "https://carbon.sage.com/v/101.0.0/index.html",
+ "v100.2.1": "https://carbon.sage.com/v/100.2.1/index.html",
+ },
+};
+
+beforeEach(() => {
+ jest.spyOn(global.console, "log").mockImplementation(() => {});
+ jest.spyOn(global.console, "error").mockImplementation(() => {});
+
+ fetch.mockResponse(JSON.stringify(mockNpmVersions));
+
+ mockedFs.mkdirSync = jest.fn((path, options, callback) => {
+ callback();
+ });
+ mockedFs.writeFileSync = jest.fn((path, json, callback) => {
+ callback();
+ });
+});
+
+afterEach(() => {
+ jest.resetAllMocks();
+ fetch.resetMocks();
+});
+
+test("creates a metadata.json file in a metadata directory", async () => {
+ await generateMetadata();
+
+ expect(mockedFs.mkdirSync).toHaveBeenCalledWith(
+ "metadata",
+ {},
+ expect.any(Function)
+ );
+
+ expect(mockedFs.writeFileSync).toHaveBeenCalledWith(
+ "metadata/metadata.json",
+ JSON.stringify(mockMetadata),
+ expect.any(Function)
+ );
+});
+
+test("throws an error, when unable to create the metadata directory", () => {
+ mockedFs.mkdirSync = jest.fn((path, options, callback) => {
+ callback(new Error("An error occurred."));
+ });
+
+ expect(() => writeFile()).toThrow("An error occurred.");
+});
+
+test("throws an error, when unable to create the metadata.json file", () => {
+ mockedFs.writeFileSync = jest.fn((path, json, callback) => {
+ callback(new Error("An error occurred."));
+ });
+
+ expect(() => writeFile()).toThrow("An error occurred.");
+});
+
+describe("when unable to fetch carbon data from npm", () => {
+ it("logs error with http status code", async () => {
+ fetch.mockResponse(JSON.stringify(mockNpmVersions), {
+ status: 500,
+ ok: false,
+ });
+
+ await generateMetadata();
+
+ expect(global.console.error).toHaveBeenCalledWith(
+ new Error("Failed to fetch from npm with HTTP error code 500")
+ );
+ });
+
+ it("does not attempt to write metadata.json file", async () => {
+ fetch.mockResponse(JSON.stringify(mockNpmVersions), {
+ status: 500,
+ ok: false,
+ });
+
+ await generateMetadata();
+
+ expect(mockedFs.writeFileSync).not.toHaveBeenCalled();
+ });
+});
diff --git a/src/__spec_helper__/__internal__/index.ts b/src/__spec_helper__/__internal__/index.ts
index 03a08d45f1..8d58d4f4a2 100644
--- a/src/__spec_helper__/__internal__/index.ts
+++ b/src/__spec_helper__/__internal__/index.ts
@@ -1,10 +1,17 @@
import { configure } from "@testing-library/react";
import { enableFetchMocks } from "jest-fetch-mock";
+import failOnConsole from "jest-fail-on-console";
+
import { setupMatchMediaMock } from "../mock-match-media";
import setupResizeObserverMock from "../mock-resize-observer";
import setupScrollToMock from "../mock-element-scrollto";
+
import "@testing-library/jest-dom";
+failOnConsole({
+ shouldFailOnError: true,
+ shouldFailOnWarn: false,
+});
enableFetchMocks();
setupResizeObserverMock();
setupMatchMediaMock();
diff --git a/src/__spec_helper__/__internal__/select-test-utils.ts b/src/__spec_helper__/__internal__/select-test-utils.ts
index cabeabe834..541fda2a85 100644
--- a/src/__spec_helper__/__internal__/select-test-utils.ts
+++ b/src/__spec_helper__/__internal__/select-test-utils.ts
@@ -25,7 +25,7 @@ export function simulateSelectTextboxEvent(
});
resizeObserver.resize();
});
- if (eventType === "focus") jest.runOnlyPendingTimers();
+ if (eventType === "focus") act(() => jest.runOnlyPendingTimers());
container.update();
}
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
+
+
+
+ );
+};
diff --git a/src/components/numeral-date/numeral-date.test.tsx b/src/components/numeral-date/numeral-date.test.tsx
index cf07febb23..93fef7e476 100644
--- a/src/components/numeral-date/numeral-date.test.tsx
+++ b/src/components/numeral-date/numeral-date.test.tsx
@@ -1563,7 +1563,12 @@ test("should submit the form when enter key is pressed", async () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
const onSubmit = jest.fn();
render(
-