diff --git a/cypress/integration/common/menu.feature b/cypress/integration/common/menu.feature index a1ecc6c864..39193fcbe9 100644 --- a/cypress/integration/common/menu.feature +++ b/cypress/integration/common/menu.feature @@ -119,31 +119,27 @@ Feature: Menu component When I click closeIcon Then Menu is in fullscreen mode is not visible - @ignore - # testing incorrect behaviour, ignored as that behaviour will be implemented as part of FE-5092 + @positive Scenario: Check that close icon is focused in Fullscreen Menu Given I open "Menu Test" component page "menu-full-screen-story" - When I hit Tab key 1 times + When I continue to hit Tab key 1 times Then closeIcon has the border outline color "rgb(255, 181, 0)" and width "3px" - @ignore - # testing incorrect behaviour, ignored as that behaviour will be implemented as part of FE-5092 + @positive Scenario: Check that inner Menu is available with tabbing in Fullscreen Menu Given I open "Menu Test" component page "menu-full-screen-story" - When I hit Tab key 5 times + When I continue to hit Tab key 5 times Then "fourth" inner menu element is focused - @ignore - # testing incorrect behaviour, ignored as that behaviour will be implemented as part of FE-5092 + @positive Scenario: Check that previous inner Menu is available with shift tabbing in Fullscreen Menu Given I open "Menu Test" component page "menu-full-screen-story" - And I hit Tab key 6 times + And I continue to hit Tab key 6 times When I press Shift Tab on focused element Then "fourth" inner menu element is focused - @ignore - # testing incorrect behaviour, ignored as that behaviour will be implemented as part of separate task + @positive Scenario: Check that inner Menu without link is NOT available with tabbing in Fullscreen Menu Given I open "Menu Test" component page "menu-full-screen-story" - When I hit Tab key 8 times + When I continue to hit Tab key 8 times Then inner menu without active redirection is not focused \ No newline at end of file diff --git a/cypress/locators/menu/index.js b/cypress/locators/menu/index.js index d5600b0fb7..e1ba8d298d 100644 --- a/cypress/locators/menu/index.js +++ b/cypress/locators/menu/index.js @@ -26,5 +26,7 @@ export const submenuItem = (index) => export const menuCanvas = () => cy.get(DLS_ROOT); export const fullscreenMenu = (index) => cy.get(FULLSCREEN_MENU).find("div").eq(index); +export const fullScreenMenuItem = (index) => + cy.get(`${FULLSCREEN_MENU} ${MENU}`).find(`li:nth-child(${index})`); export const menu = () => cy.get(MENU); export const menuItem = () => cy.get(MENU_ITEM); diff --git a/cypress/support/step-definitions/menu-steps.js b/cypress/support/step-definitions/menu-steps.js index eeda440548..aea0b59a49 100644 --- a/cypress/support/step-definitions/menu-steps.js +++ b/cypress/support/step-definitions/menu-steps.js @@ -12,6 +12,7 @@ import { fullscreenMenu, menu, menuItem, + fullScreenMenuItem, } from "../../locators/menu"; import { searchInput, searchCrossIcon } from "../../locators/search/index"; import { positionOfElement, keyCode } from "../helper"; @@ -181,21 +182,21 @@ Then("Menu is in fullscreen mode is not visible", () => { }); Then("{string} inner menu element is focused", (position) => { - menuComponent(positionOfElement(position)) + fullScreenMenuItem(positionOfElement(position)) .find("ul > li") .eq(1) .children() .children() .should("have.css", "box-shadow") .and("contain", "rgb(255, 181, 0)"); - menuComponent(positionOfElement(position)) + fullScreenMenuItem(positionOfElement(position)) .find("ul > li") .eq(1) .children() .children() .should("have.css", "background-color") .and("contain", "rgb(0, 126, 69)"); - menuComponent(positionOfElement(position)) + fullScreenMenuItem(positionOfElement(position)) .find("ul > li") .eq(1) .children() diff --git a/src/__internal__/focus-trap/focus-trap-utils.js b/src/__internal__/focus-trap/focus-trap-utils.js index 572ce67d6a..18c82f95a6 100644 --- a/src/__internal__/focus-trap/focus-trap-utils.js +++ b/src/__internal__/focus-trap/focus-trap-utils.js @@ -1,12 +1,36 @@ const defaultFocusableSelectors = 'button:not([disabled]), [href], input:not([type="hidden"]):not([disabled]), select:not([disabled]), textarea:not([disabled]), [tabindex]'; +const waitForVisibleAndFocus = (element) => { + const INTERVAL = 10; + const MAX_TIME = 100; + let timeSoFar = 0; + + const stylesMatch = () => { + const actualStyles = window.getComputedStyle(element); + return actualStyles.visibility === "visible"; + }; + + const check = () => { + /* istanbul ignore else */ + if (stylesMatch()) { + element.focus(); + } else if (timeSoFar < MAX_TIME) { + setTimeout(check, INTERVAL); + timeSoFar += INTERVAL; + } + // just "fail" silently if maxTime exceeded - callback will never be called + }; + + check(); +}; + function setElementFocus(element) { if (typeof element === "function") { element(); } else { const el = element.current || element; - el.focus(); + waitForVisibleAndFocus(el); } } diff --git a/src/components/menu/menu-full-screen/menu-full-screen.component.js b/src/components/menu/menu-full-screen/menu-full-screen.component.js index 9060decf13..819f2ab337 100644 --- a/src/components/menu/menu-full-screen/menu-full-screen.component.js +++ b/src/components/menu/menu-full-screen/menu-full-screen.component.js @@ -1,4 +1,4 @@ -import React, { useContext, useLayoutEffect, useRef } from "react"; +import React, { useContext, useRef } from "react"; import PropTypes from "prop-types"; import { @@ -14,6 +14,7 @@ import IconButton from "../../icon-button"; import Icon from "../../icon"; import Portal from "../../portal"; import MenuDivider from "../menu-divider/menu-divider.component"; +import useModalFocus from "../../../hooks/__internal__/useModalFocus"; const MenuFullscreen = ({ children, @@ -26,6 +27,8 @@ const MenuFullscreen = ({ const menuContentRef = useRef(); const { menuType } = useContext(MenuContext); + const focusProps = useModalFocus(isOpen); + const handleKeyDown = (ev) => { /* istanbul ignore else */ if (Events.isEscKey(ev)) { @@ -33,20 +36,6 @@ const MenuFullscreen = ({ } }; - useLayoutEffect(() => { - const checkTransitionEnd = () => { - menuContentRef.current.focus(); - }; - - const wrapperRef = menuWrapperRef.current; - - if (isOpen) { - wrapperRef.addEventListener("transitionend", checkTransitionEnd); - } else { - wrapperRef.removeEventListener("transitionend", checkTransitionEnd); - } - }, [isOpen]); - const scrollVariants = { light: "light", dark: "dark", @@ -64,7 +53,7 @@ const MenuFullscreen = ({ return (
  • - + {React.Children.map(children, (child, index) => ( { +// eslint-disable-next-line react/prop-types +const TestMenu = ({ startPosition, isOpen }) => ( + + + Menu Item One + + {}} submenu="Menu Item Two"> + + Submenu Item One + + + Submenu Item Two + + + + Menu Item Three + + + Menu Item Four + + + + Submenu Item One + + + Submenu Item Two + + + + Menu Item Six + + +); + +const render = ({ menuType = "light", ...props }) => { return mount( - - - Menu Item One - - {}} submenu="Menu Item Two"> - - Submenu Item One - - - Submenu Item Two - - - - Menu Item Three - - - Menu Item Four - - - - Submenu Item One - - - Submenu Item Two - - - - Menu Item Six - - + ); }; @@ -210,17 +215,23 @@ describe("MenuFullscreen", () => { }); describe("focus behaviour", () => { - it("focuses the content container on open of menu", () => { - wrapper = render({ isOpen: true }); + it("focuses the menu wrapper on open of menu", () => { + wrapper = mount(); + wrapper.setProps({ isOpen: true }); const element = wrapper.find(StyledMenuFullscreen).getDOMNode(); - const event = new Event("transitionend", { + const startEvent = new Event("transitionstart", { + bubbles: true, + cancellable: true, + }); + const endEvent = new Event("transitionend", { bubbles: true, cancellable: true, }); - element.dispatchEvent(event); + element.dispatchEvent(startEvent); + element.dispatchEvent(endEvent); - expect(wrapper.find(StyledMenuWrapper)).toBeFocused(); + expect(wrapper.find(StyledMenuFullscreen)).toBeFocused(); }); }); }); diff --git a/src/components/menu/menu-full-screen/menu-full-screen.style.js b/src/components/menu/menu-full-screen/menu-full-screen.style.js index d47b8a8ac8..bd6a9b0c1d 100644 --- a/src/components/menu/menu-full-screen/menu-full-screen.style.js +++ b/src/components/menu/menu-full-screen/menu-full-screen.style.js @@ -13,6 +13,7 @@ const StyledMenuFullscreen = styled.div` bottom: 0; height: 100vh; width: 100%; + outline: none; a, button, diff --git a/src/hooks/__internal__/useModalFocus/useModalFocus.spec.tsx b/src/hooks/__internal__/useModalFocus/useModalFocus.spec.tsx index 908247965a..74db2208e4 100644 --- a/src/hooks/__internal__/useModalFocus/useModalFocus.spec.tsx +++ b/src/hooks/__internal__/useModalFocus/useModalFocus.spec.tsx @@ -25,18 +25,13 @@ describe("useModalFocus custom hook", () => { wrapper.unmount(); }); - it("the container initially has no tabindex", () => { - expect(wrapper.find("div").props().tabIndex).toBeUndefined(); - }); - - it("when the open argument is true, the wrapper has tabindex -1", () => { - wrapper.find("button").simulate("click"); - expect(wrapper.find("div").props().tabIndex).toBe(-1); + it("the container initially has tabindex 0", () => { + expect(wrapper.find("div").props().tabIndex).toBe(0); }); it("when open and the wrapper is blurred, the tabindex is removed", () => { wrapper.find("button").simulate("click"); - expect(wrapper.find("div").props().tabIndex).toBe(-1); + expect(wrapper.find("div").props().tabIndex).toBe(0); wrapper.find("div").simulate("blur"); expect(wrapper.find("div").props().tabIndex).toBeUndefined(); }); diff --git a/src/hooks/__internal__/useModalFocus/useModalFocus.ts b/src/hooks/__internal__/useModalFocus/useModalFocus.ts index 4833c44e02..78188a2f74 100644 --- a/src/hooks/__internal__/useModalFocus/useModalFocus.ts +++ b/src/hooks/__internal__/useModalFocus/useModalFocus.ts @@ -1,16 +1,19 @@ import { useState, useEffect } from "react"; export default function useModalFocus(open: boolean) { - const [tabIndex, setTabIndex] = useState(undefined); + const [tabIndex, setTabIndex] = useState(0); useEffect(() => { + // ideally would set to -1, but that fails Cypress tests due to Cypress bug. + // It's OK to set to 0 instead as the tabindex is removed on blur anyway. if (open) { - setTabIndex(-1); + setTabIndex(0); } }, [open]); // need to remove the container's tabindex after it has been used for initial focus, - // otherwise it becomes impossible to scroll any content using the keyboard + // otherwise (if tabindex is 0) the container would be reachable by keyboard tabbing which we don't want, + // and if the tabindex is -1 it makes it impossible to scroll any content using the keyboard const onBlur = () => { /* istanbul ignore else */ if (open) {