From c5f95b02af786486c747e80aca0bd4f3e55f6fcd Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 16 Apr 2025 20:24:52 +0200 Subject: [PATCH] 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 /* No transition classes */ ``` 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. --- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/transition/transition.tsx | 22 +++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 152ea70..da29474 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -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 diff --git a/packages/@headlessui-react/src/components/transition/transition.tsx b/packages/@headlessui-react/src/components/transition/transition.tsx index 930f318..3832529 100644 --- a/packages/@headlessui-react/src/components/transition/transition.tsx +++ b/packages/@headlessui-react/src/components/transition/transition.tsx @@ -330,7 +330,7 @@ function TransitionChildFn 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