From 5ae39226b4102106f08e997129aa7cef49548ecd Mon Sep 17 00:00:00 2001 From: Ed Leeks Date: Mon, 8 Nov 2021 16:43:28 +0000 Subject: [PATCH 1/2] feat(focus-trap): add flag to trigger refocus of either last focused element first element Add `focusFlag` to trigger the refocusing of an element in the trap. Also adds a listener to support maintaining a record of last focused element which will be focused when the flag is set. --- .../focus-trap/focus-trap.component.js | 41 +++- .../focus-trap/focus-trap.spec.js | 217 +++++++++++++----- 2 files changed, 204 insertions(+), 54 deletions(-) diff --git a/src/__internal__/focus-trap/focus-trap.component.js b/src/__internal__/focus-trap/focus-trap.component.js index 0ab28f63b8..79ae2a69cb 100644 --- a/src/__internal__/focus-trap/focus-trap.component.js +++ b/src/__internal__/focus-trap/focus-trap.component.js @@ -28,7 +28,9 @@ const FocusTrap = ({ const [focusableElements, setFocusableElements] = useState(); const [firstElement, setFirstElement] = useState(); const [lastElement, setLastElement] = useState(); - const { isAnimationComplete } = useContext(ModalContext); + const [currentFocusedElement, setCurrentFocusedElement] = useState(); + const { isAnimationComplete, triggerRefocusFlag } = useContext(ModalContext); + const hasNewInputs = useCallback( (candidate) => { if (!focusableElements || candidate.length !== focusableElements.length) { @@ -129,6 +131,43 @@ const FocusTrap = ({ }; }, [firstElement, lastElement, focusableElements, bespokeTrap]); + const updateCurrentFocusedElement = useCallback(() => { + const element = focusableElements?.find( + (el) => el === document.activeElement + ); + + if (element) { + setCurrentFocusedElement(element); + } + }, [focusableElements]); + + useEffect(() => { + document.addEventListener("focusin", updateCurrentFocusedElement); + + return () => { + document.removeEventListener("focusin", updateCurrentFocusedElement); + }; + }, [updateCurrentFocusedElement]); + + const refocusTrap = useCallback(() => { + /* istanbul ignore else */ + if ( + currentFocusedElement && + !currentFocusedElement.hasAttribute("disabled") + ) { + // the trap breaks if it tries to refocus a disabled element + setElementFocus(currentFocusedElement); + } else if (firstElement) { + setElementFocus(firstElement); + } + }, [currentFocusedElement, firstElement]); + + useEffect(() => { + if (triggerRefocusFlag) { + refocusTrap(); + } + }, [triggerRefocusFlag, refocusTrap]); + return
{children}
; }; diff --git a/src/__internal__/focus-trap/focus-trap.spec.js b/src/__internal__/focus-trap/focus-trap.spec.js index b156735196..ebfee7d454 100644 --- a/src/__internal__/focus-trap/focus-trap.spec.js +++ b/src/__internal__/focus-trap/focus-trap.spec.js @@ -9,14 +9,25 @@ import { ModalContext } from "../../components/modal/modal.component"; jest.useFakeTimers(); // eslint-disable-next-line -const MockComponent = ({ children, ...rest }) => { +const MockComponent = ({ children, triggerRefocusFlag, ...rest }) => { const ref = useRef(); - + const [isDisabled, setIsDisabled] = useState(false); return ( - +
- {children} + {React.Children.map(children, (child) => { + if (child?.props?.id === "disable-on-focus") { + return React.cloneElement(child, { + onFocus: () => setIsDisabled(true), + disabled: isDisabled, + }); + } + + return child; + })}
@@ -24,6 +35,7 @@ const MockComponent = ({ children, ...rest }) => { }; describe("FocusTrap", () => { + let wrapper; const element = document.createElement("div"); const htmlElement = document.body.appendChild(element); const tabKey = new KeyboardEvent("keydown", { key: "Tab" }); @@ -34,24 +46,79 @@ describe("FocusTrap", () => { }); const otherKey = new KeyboardEvent("keydown", { keyCode: 32 }); - describe("when autoFocus is false", () => { - let wrapper; + describe("triggerRefocusFlag", () => { + afterEach(() => { + wrapper.unmount(); + }); - beforeEach(() => { + it("refocuses the last element that had focus within the trap when flag is set", () => { wrapper = mount( - + , { attachTo: htmlElement } ); + act(() => { + document.querySelectorAll("input")[0].focus(); + }); + expect(wrapper.update().find("input").at(0)).toBeFocused(); + act(() => { + document.querySelectorAll("input")[0].blur(); + }); + expect(wrapper.update().find("input").at(0)).not.toBeFocused(); + act(() => { + wrapper.setProps({ triggerRefocusFlag: true }); + }); + wrapper.update(); + expect(wrapper.update().find("input").at(0)).toBeFocused(); }); - afterEach(() => { - wrapper.unmount(); + it("refocuses the first element within the trap when flag is set", () => { + wrapper = mount( + + + + , + { attachTo: htmlElement } + ); + act(() => { + wrapper.setProps({ triggerRefocusFlag: true }); + }); + wrapper.update(); + expect(wrapper.update().find("button").at(0)).toBeFocused(); }); + it("refocuses the first element if last element that had focus becomes disabled", () => { + wrapper = mount( + + + + , + { attachTo: htmlElement } + ); + act(() => { + document.querySelectorAll("input")[0].focus(); + }); + expect(wrapper.update().find("input").at(0)).toBeFocused(); + act(() => { + wrapper.setProps({ triggerRefocusFlag: true }); + }); + wrapper.update(); + expect(wrapper.update().find("button").at(0)).toBeFocused(); + }); + }); + + describe("when autoFocus is false", () => { it("should not focus the first focusable element by default", () => { + wrapper = mount( + + + + , + { attachTo: htmlElement } + ); + expect(document.activeElement).toMatchObject( document.querySelectorAll("body")[0] ); @@ -59,7 +126,7 @@ describe("FocusTrap", () => { }); describe("when a focusFirstElement callback is provided", () => { - let wrapper, onFocus; + let onFocus; beforeEach(() => { onFocus = jest @@ -81,8 +148,10 @@ describe("FocusTrap", () => { expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); - document.dispatchEvent(shiftTabKey); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); @@ -92,7 +161,9 @@ describe("FocusTrap", () => { expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) ); @@ -102,18 +173,24 @@ describe("FocusTrap", () => { expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) ); }); it("should move to the first focusable item if TAB pressed on last focusable item", () => { - document.querySelectorAll("button")[1].focus(); + act(() => { + document.querySelectorAll("button")[1].focus(); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) ); - document.dispatchEvent(tabKey); + act(() => { + document.dispatchEvent(tabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); @@ -135,7 +212,9 @@ describe("FocusTrap", () => { }); it("calls the function with expected arguments on TAB press", () => { - document.dispatchEvent(tabKey); + act(() => { + document.dispatchEvent(tabKey); + }); expect(bespokeFn).toHaveBeenCalledWith( tabKey, document.querySelectorAll("button")[0], @@ -144,7 +223,9 @@ describe("FocusTrap", () => { }); it("calls the function with expected arguments on SHIFT + TAB press", () => { - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(bespokeFn).toHaveBeenCalledWith( shiftTabKey, document.querySelectorAll("button")[0], @@ -155,8 +236,6 @@ describe("FocusTrap", () => { describe("when FocusTrap wraps an element", () => { describe("and element has focusable items inside", () => { - let wrapper; - beforeEach(() => { wrapper = mount( @@ -174,7 +253,9 @@ describe("FocusTrap", () => { }); it("should not move if different key than TAB is pressed", () => { - document.querySelectorAll("button")[1].focus(); + act(() => { + document.querySelectorAll("button")[1].focus(); + }); document.dispatchEvent(otherKey); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) @@ -182,19 +263,27 @@ describe("FocusTrap", () => { }); it("should back to the last item when use `shift + tab` on first focusable item", () => { - document.querySelectorAll("button")[0].focus(); + act(() => { + document.querySelectorAll("button")[0].focus(); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) ); }); it("should back to the first item when use `shift + tab`", () => { - document.querySelectorAll("button")[1].focus(); - document.dispatchEvent(shiftTabKey); + act(() => { + document.querySelectorAll("button")[1].focus(); + }); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); @@ -204,18 +293,24 @@ describe("FocusTrap", () => { expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); - document.dispatchEvent(tabKey); + act(() => { + document.dispatchEvent(tabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) ); }); it("should move to the first focusable item if TAB pressed on last focusable item", () => { - document.querySelectorAll("button")[1].focus(); + act(() => { + document.querySelectorAll("button")[1].focus(); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) ); - document.dispatchEvent(tabKey); + act(() => { + document.dispatchEvent(tabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); @@ -223,8 +318,6 @@ describe("FocusTrap", () => { }); describe("and element does not have focusable items", () => { - let wrapper; - beforeEach(() => { wrapper = mount( @@ -236,7 +329,9 @@ describe("FocusTrap", () => { it("should block tabbing if `tab` pressed", () => { document.getElementById("myComponent").focus(); - document.dispatchEvent(tabKey); + act(() => { + document.dispatchEvent(tabKey); + }); expect(document.activeElement).toMatchObject(wrapper); }); @@ -248,8 +343,6 @@ describe("FocusTrap", () => { }); describe("and some children elements are disabled", () => { - let wrapper; - beforeEach(() => { wrapper = mount( @@ -267,12 +360,18 @@ describe("FocusTrap", () => { }); it("only focuses those that are not", () => { - document.querySelectorAll("button")[0].focus(); - document.dispatchEvent(tabKey); + act(() => { + document.querySelectorAll("button")[0].focus(); + }); + act(() => { + document.dispatchEvent(tabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(2) ); - document.dispatchEvent(tabKey); + act(() => { + document.dispatchEvent(tabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); @@ -281,8 +380,6 @@ describe("FocusTrap", () => { }); describe("when first focusable elements are radio buttons", () => { - let wrapper; - beforeEach(() => { wrapper = mount( @@ -310,13 +407,17 @@ describe("FocusTrap", () => { describe("when focus on the first button and shift-tab pressed", () => { it("should loop focus to the last focusable element", () => { - document.dispatchEvent(tabKey); + act(() => { + document.dispatchEvent(tabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) ); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find('input[type="radio"]').at(0) @@ -330,7 +431,9 @@ describe("FocusTrap", () => { wrapper.find('input[type="radio"]').at(0) ); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) @@ -340,13 +443,16 @@ describe("FocusTrap", () => { describe("when focus on second radio button shift-tab pressed", () => { it("should loop focus to the last focusable element", () => { - document.querySelectorAll('input[type="radio"]')[1].focus(); - + act(() => { + document.querySelectorAll('input[type="radio"]')[1].focus(); + }); expect(document.activeElement).toMatchObject( wrapper.find('input[type="radio"]').at(1) ); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(1) @@ -356,8 +462,6 @@ describe("FocusTrap", () => { }); describe("when trap contains radio buttons", () => { - let wrapper; - beforeEach(() => { wrapper = mount( @@ -385,13 +489,17 @@ describe("FocusTrap", () => { describe("when focus on first radio button shift-tab pressed", () => { it("should loop focus to the last focusable element", () => { - document.dispatchEvent(tabKey); + act(() => { + document.dispatchEvent(tabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find('input[type="radio"]').at(0) ); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) @@ -401,13 +509,16 @@ describe("FocusTrap", () => { describe("when focus on second radio button shift-tab pressed", () => { it("should loop focus to the last focusable element", () => { - document.querySelectorAll('input[type="radio"]')[1].focus(); - + act(() => { + document.querySelectorAll('input[type="radio"]')[1].focus(); + }); expect(document.activeElement).toMatchObject( wrapper.find('input[type="radio"]').at(1) ); - document.dispatchEvent(shiftTabKey); + act(() => { + document.dispatchEvent(shiftTabKey); + }); expect(document.activeElement).toMatchObject( wrapper.find("button").at(0) @@ -476,7 +587,7 @@ describe("FocusTrap", () => { ); }; - const wrapper = mount( + wrapper = mount( , From 5b9c5f75465ccd0145322710ef112e213961083e Mon Sep 17 00:00:00 2001 From: Ed Leeks Date: Tue, 9 Nov 2021 14:17:26 +0000 Subject: [PATCH 2/2] fix(modal, modal-manager): modal and manager now set a refocus flag when stacked modals are closed When there are at least two `Modal`s stacked and one closes the `ModalManager` will trigger a callback to set a flag which is passed to the `FocusTrap` via `ModalContext` and triggers refocussing of an element within the top most `Modal` fix #4502 --- .../modal/__internal__/modal-manager.js | 39 +++++++++++++--- .../modal/__internal__/modal-manager.spec.js | 44 ++++++++++++++++--- src/components/modal/modal.component.js | 10 ++++- 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/src/components/modal/__internal__/modal-manager.js b/src/components/modal/__internal__/modal-manager.js index 7ed4e98af3..1839358e5f 100644 --- a/src/components/modal/__internal__/modal-manager.js +++ b/src/components/modal/__internal__/modal-manager.js @@ -1,26 +1,55 @@ class ModalManagerInstance { #modalList = []; - addModal = (modal) => { - this.#modalList.push(modal); + #getTopModal() { + if (!this.#modalList.length) { + return {}; + } + + return this.#modalList[this.#modalList.length - 1]; + } + + addModal = (modal, setTriggerRefocusFlag) => { + const { + modal: topModal, + setTriggerRefocusFlag: setTrapFlag, + } = this.#getTopModal(); + + if (topModal && setTrapFlag) { + setTrapFlag(false); + } + + this.#modalList.push({ modal, setTriggerRefocusFlag }); }; isTopmost(modal) { - if (!modal || !this.#modalList.length) { + const { modal: topModal } = this.#getTopModal(); + + if (!modal || !topModal) { return false; } - return this.#modalList.indexOf(modal) === this.#modalList.length - 1; + return modal === topModal; } removeModal(modal) { - const modalIndex = this.#modalList.indexOf(modal); + const modalIndex = this.#modalList.findIndex(({ modal: m }) => m === modal); if (modalIndex === -1) { return; } this.#modalList.splice(modalIndex, 1); + + if (!this.#modalList.length) { + return; + } + + const { setTriggerRefocusFlag } = this.#getTopModal(); + + if (setTriggerRefocusFlag) { + setTriggerRefocusFlag(true); + } } clearList() { diff --git a/src/components/modal/__internal__/modal-manager.spec.js b/src/components/modal/__internal__/modal-manager.spec.js index fe4918830f..2f4d73b54e 100644 --- a/src/components/modal/__internal__/modal-manager.spec.js +++ b/src/components/modal/__internal__/modal-manager.spec.js @@ -3,10 +3,21 @@ import ModalManager from "./modal-manager"; describe("ModalManager", () => { describe("when the addModal method has been called", () => { it("then the element passed in an attribute should be the topmost element", () => { - const mockModal = { foo: "bar" }; + const cb1 = jest.fn(); + const cb2 = jest.fn(); - ModalManager.addModal(mockModal); - expect(ModalManager.isTopmost(mockModal)).toBe(true); + const mockModal1 = { foo: "foo" }; + const mockModal2 = { bar: "bar" }; + + ModalManager.addModal(mockModal1, cb1); + expect(ModalManager.isTopmost(mockModal1)).toBe(true); + expect(cb1).not.toHaveBeenCalled(); + + ModalManager.addModal(mockModal2, cb2); + expect(ModalManager.isTopmost(mockModal1)).toBe(false); + expect(ModalManager.isTopmost(mockModal2)).toBe(true); + expect(cb1).toHaveBeenCalledWith(false); + expect(cb2).not.toHaveBeenCalled(); }); }); @@ -23,13 +34,22 @@ describe("ModalManager", () => { describe("when the removeModal method has been called", () => { it("then the element passed in an attribute should not be the topmost element", () => { - const mockModal = { foo: "bar" }; + const cb1 = jest.fn(); + const cb2 = jest.fn(); + + const mockModal1 = { foo: "foo" }; + const mockModal2 = { bar: "bar" }; ModalManager.clearList(); - ModalManager.addModal(mockModal); - ModalManager.removeModal(mockModal); + ModalManager.addModal(mockModal1, cb1); + ModalManager.addModal(mockModal2, cb2); + ModalManager.removeModal(mockModal2); + expect(ModalManager.isTopmost(mockModal2)).toBe(false); + expect(cb1).toHaveBeenCalledWith(true); - expect(ModalManager.isTopmost(mockModal)).toBe(false); + ModalManager.removeModal(mockModal1); + + expect(ModalManager.isTopmost(mockModal1)).toBe(false); }); it("then nothing happens if removed modal is not found", () => { @@ -39,5 +59,15 @@ describe("ModalManager", () => { ModalManager.addModal(mockModal); ModalManager.removeModal({ some: "value" }); }); + + it("does not trigger refocus if no callback is found for passed modal", () => { + const mockModal1 = { foo: "foo" }; + const mockModal2 = { bar: "bar" }; + + ModalManager.clearList(); + ModalManager.addModal(mockModal1); + ModalManager.addModal(mockModal2); + ModalManager.removeModal(mockModal2); + }); }); }); diff --git a/src/components/modal/modal.component.js b/src/components/modal/modal.component.js index c751c4a664..f1f4615fbe 100644 --- a/src/components/modal/modal.component.js +++ b/src/components/modal/modal.component.js @@ -24,6 +24,7 @@ const Modal = ({ const modalRegistered = useRef(false); const originalOverflow = useRef(undefined); const [isAnimationComplete, setAnimationComplete] = useState(false); + const [triggerRefocusFlag, setTriggerRefocusFlag] = useState(false); const setOverflow = useCallback(() => { if ( @@ -113,7 +114,7 @@ const Modal = ({ const registerModal = useCallback(() => { /* istanbul ignore else */ if (!modalRegistered.current) { - ModalManager.addModal(ref.current); + ModalManager.addModal(ref.current, setTriggerRefocusFlag); modalRegistered.current = true; } @@ -181,7 +182,12 @@ const Modal = ({ {content && ( - + {content}