Fix Transition component from incorrectly exposing the Closing state#3696
Merged
RobinMalfait merged 3 commits intomainfrom Apr 16, 2025
Merged
Fix Transition component from incorrectly exposing the Closing state#3696RobinMalfait merged 3 commits intomainfrom
Transition component from incorrectly exposing the Closing state#3696RobinMalfait merged 3 commits intomainfrom
Conversation
The `opening` and `closing` state was based on the transition data of the *current* element. This means that if the parent stops transitioning before children that the `Opening` or `Closing` state is also gone. But should should be based on the full hierarchy such that the whole `Dialog` is still closing if the `Dialog` or any of its children is transitioning out.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Transition component from incorrectly re-enabling scroll lockingTransition component from incorrectly exposing the Closing state
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes an issue where the scroll locking logic was incorrectly re-enabled in Dialogs if you were using a
Transitioncomponent or atransitionprop and you had nested components with thetransitionprop (or a nestedTransitionChildcomponent) and the parent transition finishes before any of its children.To visualize this, it would happen in this situation:
With the
transitionprop, internally these components would render a wrapperTransitioncomponent.The
Dialogwill look at the open/closed state provided by theTransitioncomponent to know whether to unmount its children or not.The
Dialogcomponent also has some internal hooks to make it behave as a dialog. One of those hooks is theuseScrollLockhook. This hook will be enabled if theDialogis open and disabled when it's closed.If you are using the
Transitioncomponent or thetransitionprop, then we have to make sure that theuseScrollLockgets disabled immediate, and not wait until the transition completes. This is done by looking at theClosingstate. The reason for this is that disabling theuseScrollLockalso means that we restore the scroll position. But if you in the meantime navigate to a different page which also changes the scroll position, then we would restore the scroll position on a totally different page.We already had this logic setup, but the problem is that the
Closingstate was incorrectly derived from the transition state. That state was only looking at the current component (in the example above, theDialogcomponent) but not at any of the child components.Since the
Dialogdidn't have any transitions itself, theClosingstate was only briefly there.If there is no
Closingstate, then theuseScrollLockis looking at theopenstate of theDialog. Because other child components were still transitioning, theDialogwas still in an open state. This actually re-enabled theuseScrollLockhook. Because from theDialogs perspective no transitions were happening anymore.Eventually the transitions of all the children completed causing the
Transitionand thus theDialogto unmount. This in turn caused theuseScrollLockhook to also clean up and restore the scroll position.But as you might have guessed, now this second time, it's restoring after the transition is done.
Luckily, the fix is simple. Make sure that the
Closingstate also keeps the full hierarchy into account and not only the state of the current element.