Fix Transition component from incorrectly exposing the Closing state (#3696)

This PR fixes an issue where the scroll locking logic was incorrectly
re-enabled in Dialogs if you were using a `Transition` component or a
`transition` prop _and_ you had nested components with the `transition`
prop (or a nested `TransitionChild` component) _and_ the parent
transition finishes before any of its children.

To visualize this, it would happen in this situation:

```tsx
<Dialog transition> /* No transition classes */
  <DialogBackdrop transition className="duration-500" />
  <DialogPanel transition className="duration-200" />
  </DialogPanel>
</Dialog>
```

With the `transition` prop, internally these components would render a
wrapper `Transition` component.
The `Dialog` will look at the open/closed state provided by the
`Transition` 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 the `useScrollLock` hook. This hook will
be enabled if the `Dialog` is open and disabled when it's closed.

If you are using the `Transition` component or the `transition` prop,
then we have to make sure that the `useScrollLock` gets disabled
immediate, and not wait until the transition completes. This is done by
looking at the `Closing` state. The reason for this is that disabling
the `useScrollLock` 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, the
`Dialog` component) but not at any of the child components.

Since the `Dialog` didn't have any transitions itself, the `Closing`
state was only briefly there.

If there is no `Closing` state, then the `useScrollLock` is looking at
the `open` state of the `Dialog`. Because other child components were
still transitioning, the `Dialog` was still in an open state. This
actually **re-enabled** the `useScrollLock` hook. Because from the
`Dialog`s perspective no transitions were happening anymore.

Eventually the transitions of all the children completed causing the
`Transition` and thus the `Dialog` to unmount. This in turn caused the
`useScrollLock` 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.
This commit is contained in:
Robin Malfait
2025-04-16 20:24:52 +02:00
committed by GitHub
parent e10f54bc12
commit c5f95b02af
2 changed files with 12 additions and 11 deletions
+1
View File
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Improve `Menu` component performance ([#3685](https://github.com/tailwindlabs/headlessui/pull/3685))
- Improve `Listbox` component performance ([#3688](https://github.com/tailwindlabs/headlessui/pull/3688))
- Open `Menu` and `Listbox` on `mousedown` ([#3689](https://github.com/tailwindlabs/headlessui/pull/3689))
- Fix `Transition` component from incorrectly exposing the `Closing` state ([#3696](https://github.com/tailwindlabs/headlessui/pull/3696))
## [2.2.1] - 2025-04-04
@@ -330,7 +330,7 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
let { show, appear, initial } = useTransitionContext()
let [state, setState] = useState(show ? TreeStates.Visible : TreeStates.Hidden)
let [treeState, setState] = useState(show ? TreeStates.Visible : TreeStates.Hidden)
let parentNesting = useParentNesting()
let { register, unregister } = parentNesting
@@ -343,26 +343,26 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
if (!container.current) return
// Make sure that we are visible
if (show && state !== TreeStates.Visible) {
if (show && treeState !== TreeStates.Visible) {
setState(TreeStates.Visible)
return
}
return match(state, {
return match(treeState, {
[TreeStates.Hidden]: () => unregister(container),
[TreeStates.Visible]: () => register(container),
})
}, [state, container, register, unregister, show, strategy])
}, [treeState, container, register, unregister, show, strategy])
let ready = useServerHandoffComplete()
useIsoMorphicEffect(() => {
if (!requiresRef) return
if (ready && state === TreeStates.Visible && container.current === null) {
if (ready && treeState === TreeStates.Visible && container.current === null) {
throw new Error('Did you forget to passthrough the `ref` to the actual DOM node?')
}
}, [container, state, ready, requiresRef])
}, [container, treeState, ready, requiresRef])
// Skipping initial transition
let skip = initial && !appear
@@ -470,10 +470,10 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
})
let openClosedState = 0
if (state === TreeStates.Visible) openClosedState |= State.Open
if (state === TreeStates.Hidden) openClosedState |= State.Closed
if (transitionData.enter) openClosedState |= State.Opening
if (transitionData.leave) openClosedState |= State.Closing
if (treeState === TreeStates.Visible) openClosedState |= State.Open
if (treeState === TreeStates.Hidden) openClosedState |= State.Closed
if (show && treeState === TreeStates.Hidden) openClosedState |= State.Opening
if (!show && treeState === TreeStates.Visible) openClosedState |= State.Closing
let render = useRender()
@@ -485,7 +485,7 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
theirProps,
defaultTag: DEFAULT_TRANSITION_CHILD_TAG,
features: TransitionChildRenderFeatures,
visible: state === TreeStates.Visible,
visible: treeState === TreeStates.Visible,
name: 'Transition.Child',
})}
</OpenClosedProvider>