From 4741a8470c36ca051605c97745561ced159c96cc Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Wed, 16 Oct 2024 17:46:01 +0530 Subject: [PATCH 1/4] fix: onOpenChange triggering when modal opens --- .changeset/early-ghosts-fix.md | 7 +++++++ packages/components/modal/src/use-modal.ts | 8 +++++++- packages/hooks/use-disclosure/src/index.ts | 11 +++++++---- 3 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 .changeset/early-ghosts-fix.md diff --git a/.changeset/early-ghosts-fix.md b/.changeset/early-ghosts-fix.md new file mode 100644 index 0000000000..6ce0af54c6 --- /dev/null +++ b/.changeset/early-ghosts-fix.md @@ -0,0 +1,7 @@ +--- +"@nextui-org/modal": minor +"@nextui-org/use-disclosure": minor +--- + +Added useEffect in useModal to fire onOpenChange when onOpen changes to true. +In useDisclosure, onOpenChange now depends on arg onOpen, than state(onOpen). diff --git a/packages/components/modal/src/use-modal.ts b/packages/components/modal/src/use-modal.ts index 66b6f7be63..931ce36076 100644 --- a/packages/components/modal/src/use-modal.ts +++ b/packages/components/modal/src/use-modal.ts @@ -3,7 +3,7 @@ import type {HTMLMotionProps} from "framer-motion"; import {AriaModalOverlayProps} from "@react-aria/overlays"; import {useAriaModalOverlay} from "@nextui-org/use-aria-modal-overlay"; -import {useCallback, useId, useRef, useState, useMemo, ReactNode} from "react"; +import {useCallback, useId, useRef, useState, useMemo, ReactNode, useEffect} from "react"; import {modal} from "@nextui-org/theme"; import { HTMLNextUIProps, @@ -129,6 +129,12 @@ export function useModal(originalProps: UseModalProps) { }, }); + useEffect(() => { + if (isOpen) { + onOpenChange?.(isOpen); + } + }, [isOpen]); + const {modalProps, underlayProps} = useAriaModalOverlay( { isDismissable, diff --git a/packages/hooks/use-disclosure/src/index.ts b/packages/hooks/use-disclosure/src/index.ts index c5bccc11df..91675e52e2 100644 --- a/packages/hooks/use-disclosure/src/index.ts +++ b/packages/hooks/use-disclosure/src/index.ts @@ -44,11 +44,14 @@ export function useDisclosure(props: UseDisclosureProps = {}) { onOpenPropCallbackRef?.(); }, [isControlled, onOpenPropCallbackRef]); - const onOpenChange = useCallback(() => { - const action = isOpen ? onClose : onOpen; + const onOpenChange = useCallback( + (isOpen: boolean) => { + const action = isOpen ? onOpen : onClose; - action(); - }, [isOpen, onOpen, onClose]); + action(); + }, + [isOpen, onOpen, onClose], + ); return { isOpen: !!isOpen, From e4749c690fd4624b0b6b82055923fa93f6168b93 Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Wed, 16 Oct 2024 18:49:28 +0530 Subject: [PATCH 2/4] chore: summarized changeset --- .changeset/early-ghosts-fix.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.changeset/early-ghosts-fix.md b/.changeset/early-ghosts-fix.md index 6ce0af54c6..b0b21239d7 100644 --- a/.changeset/early-ghosts-fix.md +++ b/.changeset/early-ghosts-fix.md @@ -1,7 +1,6 @@ --- -"@nextui-org/modal": minor -"@nextui-org/use-disclosure": minor +"@nextui-org/modal": patch +"@nextui-org/use-disclosure": patch --- -Added useEffect in useModal to fire onOpenChange when onOpen changes to true. -In useDisclosure, onOpenChange now depends on arg onOpen, than state(onOpen). +Added useEffect in useModal to fire onOpenChange and in useDisclosure, onOpenChange now accepts onOpen as parameter(#3887) From dbf2c49018a9d9a80a42d749397927c4ed109783 Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Wed, 16 Oct 2024 18:59:53 +0530 Subject: [PATCH 3/4] test: when modal opens onOpenChange is called --- .../components/modal/__tests__/modal.test.tsx | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/components/modal/__tests__/modal.test.tsx b/packages/components/modal/__tests__/modal.test.tsx index b207a2269c..73cf101430 100644 --- a/packages/components/modal/__tests__/modal.test.tsx +++ b/packages/components/modal/__tests__/modal.test.tsx @@ -3,6 +3,7 @@ import {render, fireEvent} from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import {Modal, ModalContent, ModalBody, ModalHeader, ModalFooter} from "../src"; +import {Button} from "../../button/src"; // e.g. console.error Warning: Function components cannot be given refs. // Attempts to access this ref will fail. Did you mean to use React.forwardRef()? @@ -91,6 +92,33 @@ describe("Modal", () => { expect(onClose).toHaveBeenCalled(); }); + test("should fire 'onOpenChange' callback when open button is clicked", async () => { + const onOpen = jest.fn(); + + const {getByLabelText} = render( + <> + + + + Modal header + Modal body + Modal footer + + + , + ); + + const openButton = getByLabelText("Open"); + + const user = userEvent.setup(); + + await user.click(openButton); + + expect(onOpen).toHaveBeenCalled(); + }); + it("should hide the modal when pressing the escape key", () => { const onClose = jest.fn(); From d20d2f24e45c9a065c91bc5820b743ee22cf1dda Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Wed, 16 Oct 2024 21:05:55 +0530 Subject: [PATCH 4/4] test: corrected the modal testcase --- .../components/modal/__tests__/modal.test.tsx | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/components/modal/__tests__/modal.test.tsx b/packages/components/modal/__tests__/modal.test.tsx index 73cf101430..619570ea62 100644 --- a/packages/components/modal/__tests__/modal.test.tsx +++ b/packages/components/modal/__tests__/modal.test.tsx @@ -1,9 +1,9 @@ import * as React from "react"; import {render, fireEvent} from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import {Button} from "@nextui-org/button"; -import {Modal, ModalContent, ModalBody, ModalHeader, ModalFooter} from "../src"; -import {Button} from "../../button/src"; +import {Modal, ModalContent, ModalBody, ModalHeader, ModalFooter, useDisclosure} from "../src"; // e.g. console.error Warning: Function components cannot be given refs. // Attempts to access this ref will fail. Did you mean to use React.forwardRef()? @@ -92,31 +92,36 @@ describe("Modal", () => { expect(onClose).toHaveBeenCalled(); }); - test("should fire 'onOpenChange' callback when open button is clicked", async () => { - const onOpen = jest.fn(); + const ModalWrapper = ({onOpenChange}) => { + const {isOpen, onOpen} = useDisclosure(); - const {getByLabelText} = render( + return ( <> - + Modal header Modal body Modal footer - , + ); + }; - const openButton = getByLabelText("Open"); + test("should fire 'onOpenChange' callback when open button is clicked and modal opens", async () => { + const onOpenChange = jest.fn(); + const {getByLabelText} = render(); + + const openButton = getByLabelText("Open"); const user = userEvent.setup(); await user.click(openButton); - expect(onOpen).toHaveBeenCalled(); + expect(onOpenChange).toHaveBeenCalled(); }); it("should hide the modal when pressing the escape key", () => {