Skip to content

fix(Modal): show header close button when onClose is provided#1660

Open
KrotovM wants to merge 1 commit intothemesberg:mainfrom
KrotovM:fix/modal-close-button-onclose-1658
Open

fix(Modal): show header close button when onClose is provided#1660
KrotovM wants to merge 1 commit intothemesberg:mainfrom
KrotovM:fix/modal-close-button-onclose-1658

Conversation

@KrotovM
Copy link

@KrotovM KrotovM commented Mar 6, 2026

Summary

The Modal header close (X) button was only shown when dismissible={true}, which tied the close control to backdrop/Escape behavior and broke the case where onClose is used without dismissible (see #1658).

Changes:

  • ModalHeader: The close button is rendered when onClose is provided, regardless of dismissible.
  • dismissible: Still only controls backdrop click and Escape key dismissal; behavior and docs unchanged.
  • Tests: Updated and added cases: close button when onClose is provided; no close button when onClose is not provided; backdrop does not close when dismissible is true but onClose is not provided.
  • Docs: Clarified in modal.mdx that the header close (X) button is shown when onClose is passed.

Motivation: Restore the expected behavior (close button when onClose is set), match the documented meaning of dismissible.

Related issues

Fixes #1658.

Breaking API changes

None.

No props were added or removed and the theme was not changed. This only restores and clarifies behavior:

  • If you use onClose without dismissible: The header close button now appears (and works) again; no code change needed.
  • If you use dismissible={true} and onClose: Behavior is unchanged; no code change needed.
  • If you intentionally hid the close button by omitting onClose: Behavior is unchanged; no code change needed.

Summary by CodeRabbit

  • Bug Fixes

    • Updated modal close button behavior: the header close (X) button now appears when the onClose prop is provided and triggers the callback when clicked.
  • Documentation

    • Clarified modal documentation regarding close button visibility and behavior based on the onClose prop.

@vercel
Copy link

vercel bot commented Mar 6, 2026

@KrotovM is attempting to deploy a commit to the Bergside Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot
Copy link

changeset-bot bot commented Mar 6, 2026

⚠️ No Changeset found

Latest commit: d7892a6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 48c5cd64-e4c4-454a-8040-cb129f4498ef

📥 Commits

Reviewing files that changed from the base of the PR and between 0f526aa and d7892a6.

📒 Files selected for processing (3)
  • apps/web/content/docs/components/modal.mdx
  • packages/ui/src/components/Modal/Modal.test.tsx
  • packages/ui/src/components/Modal/ModalHeader.tsx

📝 Walkthrough

Walkthrough

The Modal component's close button visibility logic is refactored from using a dismissible prop to using the presence of an onClose handler. When onClose is provided, the header close (X) button is shown and invokes the handler; when absent, the button is not rendered. Documentation and tests are updated accordingly.

Changes

Cohort / File(s) Summary
Modal Close Button Control Refactor
packages/ui/src/components/Modal/ModalHeader.tsx, packages/ui/src/components/Modal/Modal.test.tsx, apps/web/content/docs/components/modal.mdx
Changed close button rendering condition from dismissible prop to onClose presence; test logic updated with new TestModalWithoutOnClose component to verify backdrop behavior differs based on onClose availability; documentation clarified to reflect new behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • Docs wrong: onClose vs dismissible #1658: Addresses identical behavior—controlling close (X) button rendering and backdrop click behavior by switching from dismissible prop to onClose presence condition.

Possibly related PRs

Suggested reviewers

  • SutuSebastian

Poem

🐰 A modal once had a dismissible friend,
But now onClose decides when the story will end,
The X button knows: no handler, no show,
A cleaner condition—much more apropos! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: decoupling the Modal header close button rendering from dismissible and making it appear when onClose is provided.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs wrong: onClose vs dismissible

1 participant