Skip to content

fix(modal): make dismiss idempotent when modal is not presented#2711

Open
janicduplessis wants to merge 1 commit into
gorhom:masterfrom
janicduplessis:fix/modal-dismiss-before-present
Open

fix(modal): make dismiss idempotent when modal is not presented#2711
janicduplessis wants to merge 1 commit into
gorhom:masterfrom
janicduplessis:fix/modal-dismiss-before-present

Conversation

@janicduplessis

@janicduplessis janicduplessis commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Motivation

Calling dismiss() on a modal whose status is INITIAL — never presented, or already self-dismissed and reset — permanently breaks it: every subsequent present() silently no-ops. handleDismiss falls through the early-exit and sets statusRef to DISMISSING, nothing ever resets it (no BottomSheet is mounted to fire onClose), and handlePortalRender bails on DISMISSING forever after.

This breaks the common declarative wrapper pattern:

useEffect(() => {
  if (visible) modalRef.current?.present();
  else modalRef.current?.dismiss();
}, [visible]);

A backdrop/swipe close resets the status to INITIAL, so the wrapper's next dismiss() hits exactly this case.

This PR treats INITIAL as already-closed in handleDismiss, mirroring CLOSED/MINIMIZED, so dismiss() is idempotent. Full analysis and a Snack repro are in #2669.

Verified in a real app on iOS after upgrading to 5.2.14: with this change, present → user close → dismiss() → present cycles work repeatedly.

Fixes #2669

Before / after

Minimal repro (scripted): dismiss() on a never-presented modal, then present() twice.

Before — present() never works after the initial dismiss():

upstream-before2.mp4

After — dismiss() is a safe no-op and present() opens the sheet:

upstream-after2.mp4

Calling dismiss() while the modal status is INITIAL (never presented, or
already fully dismissed and reset) fell through the early-exit and left
statusRef stuck at DISMISSING, after which every present() silently
no-oped in handlePortalRender. Treat INITIAL as already-closed so
dismiss() is safe to call from declarative wrappers that mirror an
'open' prop.

Fixes gorhom#2669
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.

[Bug]: dismiss() before first present() corrupts statusRef, blocking subsequent present()

1 participant