diff --git a/.changeset/nervous-plums-own.md b/.changeset/nervous-plums-own.md new file mode 100644 index 00000000000..9e8e721f488 --- /dev/null +++ b/.changeset/nervous-plums-own.md @@ -0,0 +1,5 @@ +--- +"@chakra-ui/modal": patch +--- + +Fix `useAriaHidden` hook dependency to make it work as expected diff --git a/packages/modal/src/use-modal.ts b/packages/modal/src/use-modal.ts index ad464107f10..bf2f594166b 100644 --- a/packages/modal/src/use-modal.ts +++ b/packages/modal/src/use-modal.ts @@ -1,7 +1,7 @@ import { useIds } from "@chakra-ui/hooks" import { callAllHandlers } from "@chakra-ui/utils" import { mergeRefs, PropGetter } from "@chakra-ui/react-utils" -import { hideOthers, Undo } from "aria-hidden" +import { hideOthers } from "aria-hidden" import { KeyboardEvent, MouseEvent, @@ -203,19 +203,14 @@ export function useAriaHidden( ref: RefObject, shouldHide: boolean, ) { - useEffect(() => { - if (!ref.current) return undefined - - let undo: Undo | null = null + // save current ref in a local var to trigger the effect on identity change + const currentElement = ref.current - if (shouldHide && ref.current) { - undo = hideOthers(ref.current) - } + useEffect(() => { + // keep using `ref.current` inside the effect + // it may have changed during render and the execution of the effect + if (!ref.current || !shouldHide) return undefined - return () => { - if (shouldHide) { - undo?.() - } - } - }, [shouldHide, ref]) + return hideOthers(ref.current) + }, [shouldHide, ref, currentElement]) } diff --git a/packages/modal/tests/use-modal.test.tsx b/packages/modal/tests/use-modal.test.tsx new file mode 100644 index 00000000000..1dcfcf9fb08 --- /dev/null +++ b/packages/modal/tests/use-modal.test.tsx @@ -0,0 +1,36 @@ +import { renderHook } from "@testing-library/react-hooks" +import { hideOthers } from "aria-hidden" +import { MutableRefObject } from "react" +import { useAriaHidden } from "../src" + +jest.mock("aria-hidden") + +beforeEach(() => { + jest.clearAllMocks() +}) + +describe("useAriaHidden", () => { + it("should be triggered if ref.current is changed", () => { + const ref: MutableRefObject = { current: null } + + renderHook(() => useAriaHidden(ref, true)) + expect(hideOthers).not.toBeCalled() + + ref.current = document.createElement("div") + + renderHook(() => useAriaHidden(ref, true)) + expect(hideOthers).toBeCalledWith(ref.current) + }) + + it("shouldn't be triggered if `shouldHide` is `false`", () => { + const ref: MutableRefObject = { current: null } + + renderHook(() => useAriaHidden(ref, true)) + expect(hideOthers).not.toBeCalled() + + ref.current = document.createElement("div") + + renderHook(() => useAriaHidden(ref, false)) + expect(hideOthers).not.toBeCalled() + }) +})