fix(multi-action-button, split-button): ensure child button are acces…

…sible Fixes issue where when clicking on the toggle button to open the menu and then pressing Tab, focus would move away from the button onto the next focusable element on the page, while maintaining the menu open. This would also occur when using Control+Option+Space with voiceOver to open the menu. When the menu is opened using these methods, Tab should now move focus onto first menu child. fix #7005
Sage · Oct 21, 2024 · 9a454fd · 9a454fd
1 parent 421c4a6
commit 9a454fd
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 46 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
@@ -58,6 +58,8 @@ export const MultiActionButton = ({
   const handleClick = (
     ev: React.MouseEvent<HTMLButtonElement | HTMLAnchorElement>
   ) => {
+    // ensure button is focused when clicked (Safari)
+    buttonRef.current?.focus();
     showButtons();
     handleInsideClick();
     if (onClick) {

diff --git a/src/components/multi-action-button/multi-action-button.pw.tsx b/src/components/multi-action-button/multi-action-button.pw.tsx
@@ -15,7 +15,6 @@ import {
 import {
   assertCssValueIsApproximately,
   checkAccessibility,
-  continuePressingTAB,
 } from "../../../playwright/support/helper";
 import { SIZE, CHARACTERS } from "../../../playwright/support/constants";
 import {
@@ -265,7 +264,6 @@ test.describe("Functional tests", () => {
     const listButton1 = getDataElementByValue(page, "additional-buttons")
       .getByRole("button")
       .nth(0);
-    await continuePressingTAB(page, 2);
     await page.keyboard.press("ArrowUp");
     await expect(listButton1).toBeFocused();
     await page.keyboard.press("ArrowUp");
@@ -290,8 +288,8 @@ test.describe("Functional tests", () => {
     const listButton2 = getDataElementByValue(page, "additional-buttons")
       .getByRole("button")
       .nth(1);
-    await continuePressingTAB(page, 2);
-    await expect(listButton2).toBeFocused();
+
+    await listButton2.focus();
     await page.keyboard.press("Shift+Tab");
     await expect(listButton1).toBeFocused();
     await page.keyboard.press("Shift+Tab");
@@ -313,8 +311,8 @@ test.describe("Functional tests", () => {
     const listButton3 = getDataElementByValue(page, "additional-buttons")
       .getByRole("button")
       .nth(2);
-    await continuePressingTAB(page, 3);
-    await expect(listButton3).toBeFocused();
+
+    await listButton3.focus();
     await page.keyboard.press("ArrowDown");
     await expect(listButton3).toBeFocused();
   });
@@ -325,7 +323,10 @@ test.describe("Functional tests", () => {
   }) => {
     await mount(<MultiActionTwoButtons />);
 
-    await page.keyboard.press("Tab");
+    const actionButton = getComponent(page, "multi-action-button")
+      .first()
+      .locator("button");
+    await actionButton.click();
     const listButton1 = getDataElementByValue(page, "additional-buttons")
       .getByRole("button")
       .nth(0);
@@ -335,7 +336,8 @@ test.describe("Functional tests", () => {
     const listButton3 = getDataElementByValue(page, "additional-buttons")
       .getByRole("button")
       .nth(2);
-    await page.keyboard.press("Space");
+
+    await page.keyboard.press("Tab");
     await expect(listButton1).toBeFocused();
     await page.keyboard.press("Tab");
     await expect(listButton2).toBeFocused();
@@ -363,8 +365,8 @@ test.describe("Functional tests", () => {
     const listButton3 = getDataElementByValue(page, "additional-buttons")
       .getByRole("button")
       .nth(2);
-    await continuePressingTAB(page, 3);
-    await expect(listButton3).toBeFocused();
+
+    await listButton3.focus();
     await page.keyboard.down("Meta");
     await page.keyboard.press("ArrowUp");
     await page.keyboard.up("Meta");
@@ -387,8 +389,8 @@ test.describe("Functional tests", () => {
     const listButton3 = getDataElementByValue(page, "additional-buttons")
       .getByRole("button")
       .nth(2);
-    await continuePressingTAB(page, 3);
-    await expect(listButton3).toBeFocused();
+
+    await listButton3.focus();
     await page.keyboard.down("Control");
     await page.keyboard.press("ArrowUp");
     await page.keyboard.up("Control");
@@ -411,8 +413,8 @@ test.describe("Functional tests", () => {
     const listButton3 = getDataElementByValue(page, "additional-buttons")
       .getByRole("button")
       .nth(2);
-    await continuePressingTAB(page, 3);
-    await expect(listButton3).toBeFocused();
+
+    await listButton3.focus();
     await page.keyboard.press("Home");
     await expect(listButton1).toBeFocused();
   });
@@ -545,7 +547,7 @@ test.describe(
       const listButton1 = getDataElementByValue(page, "additional-buttons")
         .getByRole("button")
         .nth(0);
-      await continuePressingTAB(page, 2);
+
       await page.keyboard.press("ArrowUp");
       await expect(listButton1).toBeFocused();
       await page.keyboard.press("ArrowUp");
@@ -570,8 +572,8 @@ test.describe(
       const listButton2 = getDataElementByValue(page, "additional-buttons")
         .getByRole("button")
         .nth(1);
-      await continuePressingTAB(page, 2);
-      await expect(listButton2).toBeFocused();
+
+      await listButton2.focus();
       await page.keyboard.press("Shift+Tab");
       await expect(listButton1).toBeFocused();
       await page.keyboard.press("Shift+Tab");
@@ -593,8 +595,8 @@ test.describe(
       const listButton3 = getDataElementByValue(page, "additional-buttons")
         .getByRole("button")
         .nth(2);
-      await continuePressingTAB(page, 3);
-      await expect(listButton3).toBeFocused();
+
+      await listButton3.focus();
       await page.keyboard.press("ArrowDown");
       await expect(listButton3).toBeFocused();
     });
@@ -605,7 +607,10 @@ test.describe(
     }) => {
       await mount(<WithWrapperTwoButtons />);
 
-      await page.keyboard.press("Tab");
+      const actionButton = getComponent(page, "multi-action-button")
+        .first()
+        .locator("button");
+      await actionButton.click();
       const listButton1 = getDataElementByValue(page, "additional-buttons")
         .getByRole("button")
         .nth(0);
@@ -615,7 +620,8 @@ test.describe(
       const listButton3 = getDataElementByValue(page, "additional-buttons")
         .getByRole("button")
         .nth(2);
-      await page.keyboard.press("Space");
+
+      await page.keyboard.press("Tab");
       await expect(listButton1).toBeFocused();
       await page.keyboard.press("Tab");
       await expect(listButton2).toBeFocused();
@@ -643,8 +649,8 @@ test.describe(
       const listButton3 = getDataElementByValue(page, "additional-buttons")
         .getByRole("button")
         .nth(2);
-      await continuePressingTAB(page, 3);
-      await expect(listButton3).toBeFocused();
+
+      await listButton3.focus();
       await page.keyboard.down("Meta");
       await page.keyboard.press("ArrowUp");
       await page.keyboard.up("Meta");
@@ -667,8 +673,8 @@ test.describe(
       const listButton3 = getDataElementByValue(page, "additional-buttons")
         .getByRole("button")
         .nth(2);
-      await continuePressingTAB(page, 3);
-      await expect(listButton3).toBeFocused();
+
+      await listButton3.focus();
       await page.keyboard.down("Control");
       await page.keyboard.press("ArrowUp");
       await page.keyboard.up("Control");
@@ -691,8 +697,8 @@ test.describe(
       const listButton3 = getDataElementByValue(page, "additional-buttons")
         .getByRole("button")
         .nth(2);
-      await continuePressingTAB(page, 3);
-      await expect(listButton3).toBeFocused();
+
+      await listButton3.focus();
       await page.keyboard.press("Home");
       await expect(listButton1).toBeFocused();
     });

diff --git a/src/components/split-button/split-button.component.tsx b/src/components/split-button/split-button.component.tsx
@@ -102,12 +102,18 @@ export const SplitButton = ({
     ...filterOutStyledSystemSpacingProps(rest),
   };
 
+  const handleToggleClick = () => {
+    // ensure button is focused when clicked (Safari)
+    toggleButton.current?.focus();
+    showButtons();
+  };
+
   const toggleButtonProps = {
     disabled,
     displayed: showAdditionalButtons,
     onTouchStart: showButtons,
     onKeyDown: handleToggleButtonKeyDown,
-    onClick: showButtons,
+    onClick: handleToggleClick,
     buttonType,
     size,
   };

diff --git a/src/hooks/__internal__/useChildButtons/useChildButtons.tsx b/src/hooks/__internal__/useChildButtons/useChildButtons.tsx
@@ -1,4 +1,4 @@
-import { useState, useRef, useEffect, useCallback } from "react";
+import { useState, useRef, useCallback } from "react";
 import Events from "../../../__internal__/utils/helpers/events";
 import useMenuKeyboardNavigation from "../useMenuKeyboardNavigation";
 
@@ -11,7 +11,6 @@ const useChildButtons = (
 
   const buttonNode = useRef<HTMLDivElement>(null);
   const childrenContainer = useRef<HTMLUListElement>(null);
-  const focusFirstChildButtonOnOpen = useRef(false);
 
   const hideButtons = useCallback(() => {
     setShowAdditionalButtons(false);
@@ -36,34 +35,26 @@ const useChildButtons = (
     []
   );
 
-  useEffect(() => {
-    const firstChild = getButtonChildren()?.[0];
-    if (
-      focusFirstChildButtonOnOpen.current &&
-      showAdditionalButtons &&
-      firstChild
-    ) {
-      focusFirstChildButtonOnOpen.current = false;
-      firstChild.focus();
-    }
-  }, [showAdditionalButtons, getButtonChildren]);
-
   const handleToggleButtonKeyDown = (
     ev: React.KeyboardEvent<HTMLButtonElement>
   ) => {
-    if (
+    const isToggleKey =
       Events.isEnterKey(ev) ||
       Events.isSpaceKey(ev) ||
       Events.isDownKey(ev) ||
-      Events.isUpKey(ev)
-    ) {
+      Events.isUpKey(ev) ||
+      (showAdditionalButtons && Events.isTabKey(ev));
+
+    if (isToggleKey) {
       ev.preventDefault();
 
       if (!showAdditionalButtons) {
         showButtons();
       }
 
-      focusFirstChildButtonOnOpen.current = true;
+      setTimeout(() => {
+        getButtonChildren()?.[0]?.focus();
+      });
     }
   };