Fix Transition
component from incorrectly exposing the Closing
state
#3696
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
Transition
component or atransition
prop and you had nested components with thetransition
prop (or a nestedTransitionChild
component) and the parent transition finishes before any of its children.To visualize this, it would happen in this situation:
With the
transition
prop, internally these components would render a wrapperTransition
component.The
Dialog
will look at the open/closed state provided by theTransition
component to know whether to unmount its children or not.The
Dialog
component also has some internal hooks to make it behave as a dialog. One of those hooks is theuseScrollLock
hook. This hook will be enabled if theDialog
is open and disabled when it's closed.If you are using the
Transition
component or thetransition
prop, then we have to make sure that theuseScrollLock
gets disabled immediate, and not wait until the transition completes. This is done by looking at theClosing
state. The reason for this is that disabling theuseScrollLock
also 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
Closing
state was incorrectly derived from the transition state. That state was only looking at the current component (in the example above, theDialog
component) but not at any of the child components.Since the
Dialog
didn't have any transitions itself, theClosing
state was only briefly there.If there is no
Closing
state, then theuseScrollLock
is looking at theopen
state of theDialog
. Because other child components were still transitioning, theDialog
was still in an open state. This actually re-enabled theuseScrollLock
hook. Because from theDialog
s perspective no transitions were happening anymore.Eventually the transitions of all the children completed causing the
Transition
and thus theDialog
to unmount. This in turn caused theuseScrollLock
hook 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
Closing
state also keeps the full hierarchy into account and not only the state of the current element.