Skip to content

Commit

Permalink
fix(multi-action-button, split-button): ensure child button are acces…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@nuria1110
nuria1110 committed Oct 18, 2024
1 parent 421c4a6 commit d97c823
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 42 deletions.
2 changes: 2 additions & 0 deletions src/components/multi-action-button/multi-action-button.component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
60 changes: 33 additions & 27 deletions src/components/multi-action-button/multi-action-button.pw.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
import {
assertCssValueIsApproximately,
checkAccessibility,
continuePressingTAB,
} from "../../../playwright/support/helper";
import { SIZE, CHARACTERS } from "../../../playwright/support/constants";
import {
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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();
});
Expand All @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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();
});
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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();
});
Expand All @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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();
});
Expand Down
8 changes: 7 additions & 1 deletion src/components/split-button/split-button.component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
27 changes: 13 additions & 14 deletions src/hooks/__internal__/useChildButtons/useChildButtons.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ const useChildButtons = (
) => {
const [showAdditionalButtons, setShowAdditionalButtons] = useState(false);
const [minWidth, setMinWidth] = useState(0);
const [
focusFirstChildButtonOnOpen,
setFocusFirstChildButtonOnOpen,
] = useState(false);

const buttonNode = useRef<HTMLDivElement>(null);
const childrenContainer = useRef<HTMLUListElement>(null);
const focusFirstChildButtonOnOpen = useRef(false);

const hideButtons = useCallback(() => {
setShowAdditionalButtons(false);
Expand All @@ -38,32 +41,28 @@ const useChildButtons = (

useEffect(() => {
const firstChild = getButtonChildren()?.[0];
if (
focusFirstChildButtonOnOpen.current &&
showAdditionalButtons &&
firstChild
) {
focusFirstChildButtonOnOpen.current = false;
if (focusFirstChildButtonOnOpen && showAdditionalButtons && firstChild) {
setFocusFirstChildButtonOnOpen(false);
firstChild.focus();
}
}, [showAdditionalButtons, getButtonChildren]);
}, [showAdditionalButtons, getButtonChildren, focusFirstChildButtonOnOpen]);

const handleToggleButtonKeyDown = (
ev: React.KeyboardEvent<HTMLButtonElement>
) => {
if (
const isToggleKey =
Events.isEnterKey(ev) ||
Events.isSpaceKey(ev) ||
Events.isDownKey(ev) ||
Events.isUpKey(ev)
) {
Events.isUpKey(ev);

if (isToggleKey || (showAdditionalButtons && Events.isTabKey(ev))) {
ev.preventDefault();
setFocusFirstChildButtonOnOpen(true);

if (!showAdditionalButtons) {
if (isToggleKey && !showAdditionalButtons) {
showButtons();
}

focusFirstChildButtonOnOpen.current = true;
}
};

Expand Down

0 comments on commit d97c823

Please sign in to comment.