-
-
Notifications
You must be signed in to change notification settings - Fork 923
fix: prevent animateOnMount from overriding running close animations #2529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
fix: prevent animateOnMount from overriding running close animations #2529
Conversation
|
Patch: |
|
It's also worth mentioning that the consumer should always update the state in |
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
Not stale. Would love to see this and other critical fixes being merged as 5.2.X is currently very broken 😐 |
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
Not stale.. this bot is too aggressive and should be removed |
Motivation
The BottomSheetModal component has a critical issue when
bottomSheetRef.current?.present()andbottomSheetRef.current?.close()are called programmatically throughuseEffecthooks triggered by state changes, causing the modal to remain open instead of closing properly.Problem:
When
openchanges rapidly (e.g., in Storybook controls or programmatic state changes), theuseEffectcallsbottomSheetRef.current?.present()andbottomSheetRef.current?.close()in quick succession. TheanimateOnMountcall from the present action overrides thehandleForceCloseanimation from the close action, causing the modal to stay open and become frozen - unable to be closed afterward.Example:
Root Cause:
In the
evaluatePositionfunction, when!isAnimatedOnMount.valueis true (which happens when the modal is being presented), the code immediately callsanimateToPositionwithout checking if there's already a running animation that should be respected. This allows mount animations to override ongoing close animations, especially problematic when React's state batching and effect scheduling cause rapid successive calls.Expected vs Actual Behavior:
open = true→bottomSheetRef.current?.present()→open = false→bottomSheetRef.current?.close()→ Modal closes properlyopen = true→bottomSheetRef.current?.present()→open = false→bottomSheetRef.current?.close()→open = true(rapid) →bottomSheetRef.current?.present()→ Modal stays open and becomes frozenTechnical Details:
The issue occurs in
BottomSheet.tsxlines 965-985 where the mount animation logic doesn't respect running animations. The fix adds a check foranimationStatus === ANIMATION_STATUS.RUNNINGbefore handling mount animations, ensuring that force close operations are not interrupted by subsequent present calls triggered by React's state update timing.Impact:
This affects programmatic usage of BottomSheetModal where state-driven present/dismiss calls are common, such as in Storybook controls, form workflows, conditional rendering, or user interaction handlers. The fix ensures consistent and predictable modal behavior regardless of React's state update timing and prevents the modal from becoming permanently frozen.