fix(modal): onOpenChange triggering when modal opens
Closes #3887
📝 Description
The changes in this PR makes the onOpenChange fire when modal opens, making it consistent.
⛳️ Current behavior (updates)
- onOpenChange fires only when modal closes and not when it opens.
- onOpenChange in useDisclosure decides which action to fire on isOpen state.
🚀 New behavior
- onOpenChange fires on both states consistently.
- Added useEffect, so when isOpen changes to true, onOpenChange is called.
- onOpenChange in useDisclosure decides which action to fire on basis of isOpen argument, used to do isOpen state earlier.
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
- onClose function in modal is basically coming from useOuterlayTriggerState, but onOpen is just a normal function. Thats why we need that useEffect to fire onOpenChange explicitly.
- Make sure to pass the isOpen variable when you are wrapping onOpenChange inside a function. Like this onOpenChange(isOpen).
Summary by CodeRabbit
-
New Features
- Improved modal functionality with enhanced responsiveness to state changes.
- Updated disclosure logic to better handle open and close actions based on boolean parameters.
-
Bug Fixes
- Fixed issues related to side effects not triggering correctly when the modal opens.
-
Tests
- Added a new test case to verify the
onOpenChangecallback is triggered when the "Open" button is clicked.
- Added a new test case to verify the
🦋 Changeset detected
Latest commit: d20d2f24e45c9a065c91bc5820b743ee22cf1dda
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 3 packages
| Name | Type |
|---|---|
| @nextui-org/modal | Patch |
| @nextui-org/use-disclosure | Patch |
| @nextui-org/react | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
@sanuj21 is attempting to deploy a commit to the NextUI Inc Team on Vercel.
A member of the Team first needs to authorize it.
Walkthrough
The changes in this pull request involve modifications to the useModal and useDisclosure hooks. A useEffect hook has been added in useModal to trigger the onOpenChange callback when the modal's isOpen state changes to true. Additionally, the onOpenChange function in useDisclosure has been updated to accept a boolean parameter, enhancing its responsiveness to state changes.
Changes
| File Path | Change Summary |
|---|---|
| .changeset/early-ghosts-fix.md | Added a useEffect in useModal to trigger onOpenChange on modal open; updated onOpenChange in useDisclosure to accept a boolean. |
| packages/hooks/use-disclosure/src/index.ts | Updated onOpenChange function signature to accept a boolean parameter. |
| packages/components/modal/tests/modal.test.tsx | Added a test case to verify that onOpenChange is triggered when the "Open" button is clicked. |
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
onOpenChange should fire when the modal opens (#3887) |
✅ | |
| Ensure side effects are executed when modal opens (#3887) | ✅ |
Possibly related PRs
-
#3495: This PR addresses a related issue with the modal component, specifically fixing the double scrollbar problem when using
scrollBehavior="inside", which is relevant to the changes made in theuseModalhook in the main PR. -
#3691: This PR focuses on adjusting the modal's position when the keyboard appears, which relates to the overall functionality and responsiveness of the modal, similar to the enhancements made in the
useDisclosurehook in the main PR.
Suggested reviewers
- wingkwong
- jrgarciadev
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-
I pushed a fix in commit <commit_id>, please review it. -
Generate unit testing code for this file. -
Open a follow-up GitHub issue for this discussion.
-
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:-
@coderabbitai generate unit testing code for this file. -
@coderabbitai modularize this function.
-
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:-
@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase. -
@coderabbitai read src/utils.ts and generate unit testing code. -
@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format. -
@coderabbitai help me debug CodeRabbit configuration file.
-
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.
CodeRabbit Commands (Invoked using PR comments)
-
@coderabbitai pauseto pause the reviews on a PR. -
@coderabbitai resumeto resume the paused reviews. -
@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository. -
@coderabbitai full reviewto do a full review from scratch and review all the files again. -
@coderabbitai summaryto regenerate the summary of the PR. -
@coderabbitai resolveresolve all the CodeRabbit review comments. -
@coderabbitai configurationto show the current CodeRabbit configuration for the repository. -
@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
@chirokas
Thanks for the comment! In basic use cases, it seems that the approach in your example would work well. However, I think this behavior of the Modal is also a bug. I'll be sure to pay attention to onChange in useDisclosure when making fixes. Thank you!