From 1e7f0d1e2ae017ce4f5ed28efd2e48ad8a19539c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 19 Jun 2024 12:07:48 +0200 Subject: [PATCH 01/13] add optional `start` and `end` events to `useTransitionData` This will be used when we implement the `` component purely using the `useTransitionData` information. But because there is a hierarchy between `` and `` we need to know when transitions start and end. --- .../@headlessui-react/src/hooks/use-transition-data.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/@headlessui-react/src/hooks/use-transition-data.ts b/packages/@headlessui-react/src/hooks/use-transition-data.ts index 8e93692fa8..14f71c26ec 100644 --- a/packages/@headlessui-react/src/hooks/use-transition-data.ts +++ b/packages/@headlessui-react/src/hooks/use-transition-data.ts @@ -42,7 +42,11 @@ export type TransitionData = { export function useTransitionData( enabled: boolean, elementRef: MutableRefObject, - show: boolean + show: boolean, + events?: { + start?(show: boolean): void + end?(show: boolean): void + } ): [visible: boolean, data: TransitionData] { let [visible, setVisible] = useState(show) @@ -72,6 +76,8 @@ export function useTransitionData( return } + events?.start?.(show) + return transition(node, { inFlight, prepare() { @@ -137,6 +143,8 @@ export function useTransitionData( if (!show) { setVisible(false) } + + events?.end?.(show) }, }) }, From 7cc8809cb438c4330c6fc40b43f58ed78243cd26 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 19 Jun 2024 12:14:23 +0200 Subject: [PATCH 02/13] implement `` and `` on top of `useTransitionData()` --- .../src/components/transition/transition.tsx | 225 +++++++----------- 1 file changed, 88 insertions(+), 137 deletions(-) diff --git a/packages/@headlessui-react/src/components/transition/transition.tsx b/packages/@headlessui-react/src/components/transition/transition.tsx index 2ab9916276..0965fcc5e0 100644 --- a/packages/@headlessui-react/src/components/transition/transition.tsx +++ b/packages/@headlessui-react/src/components/transition/transition.tsx @@ -4,6 +4,7 @@ import React, { Fragment, createContext, useContext, + useEffect, useMemo, useRef, useState, @@ -13,14 +14,12 @@ import React, { } from 'react' import { useDisposables } from '../../hooks/use-disposables' import { useEvent } from '../../hooks/use-event' -import { useFlags } from '../../hooks/use-flags' import { useIsMounted } from '../../hooks/use-is-mounted' import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect' import { useLatestValue } from '../../hooks/use-latest-value' import { useOnDisappear } from '../../hooks/use-on-disappear' import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete' import { useSyncRefs } from '../../hooks/use-sync-refs' -import { useTransition } from '../../hooks/use-transition' import { useTransitionData } from '../../hooks/use-transition-data' import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' import type { Props, ReactTag } from '../../types' @@ -29,6 +28,7 @@ import { match } from '../../utils/match' import { RenderFeatures, RenderStrategy, + compact, forwardRefWithAs, render, type HasDisplayName, @@ -38,17 +38,7 @@ import { type ContainerElement = MutableRefObject -type TransitionDirection = 'enter' | 'leave' | 'idle' - -/** - * Split class lists by whitespace - * - * We can't check for just spaces as all whitespace characters are - * invalid in a class name, so we have to split on ANY whitespace. - */ -function splitClasses(classes: string = '') { - return classes.split(/\s+/).filter((className) => className.length > 1) -} +type TransitionDirection = 'enter' | 'leave' /** * Check if we should forward the ref to the child element or not. This is to @@ -224,11 +214,7 @@ function useNesting(done?: () => void, parent?: NestingContextValues) { let chains = useRef< Record][]> - >({ - enter: [], - leave: [], - idle: [], - }) + >({ enter: [], leave: [] }) let onStart = useEvent( ( @@ -328,13 +314,13 @@ function TransitionChildFn(null) let requiresRef = shouldForwardRef(props) let transitionRef = useSyncRefs(...(requiresRef ? [container, ref] : ref === null ? [] : [ref])) - let strategy = rest.unmount ?? true ? RenderStrategy.Unmount : RenderStrategy.Hidden + let strategy = theirProps.unmount ?? true ? RenderStrategy.Unmount : RenderStrategy.Hidden let { show, appear, initial } = useTransitionContext() @@ -362,24 +348,6 @@ function TransitionChildFn { @@ -394,43 +362,6 @@ function TransitionChildFn { - if (immediate) return 'enter' - if (!ready) return 'idle' - if (skip) return 'idle' - return show ? 'enter' : 'leave' - })() as TransitionDirection - - let transitionStateFlags = useFlags(0) - - let beforeEvent = useEvent((direction: TransitionDirection) => { - return match(direction, { - enter: () => { - transitionStateFlags.addFlag(State.Opening) - events.current.beforeEnter?.() - }, - leave: () => { - transitionStateFlags.addFlag(State.Closing) - events.current.beforeLeave?.() - }, - idle: () => {}, - }) - }) - - let afterEvent = useEvent((direction: TransitionDirection) => { - return match(direction, { - enter: () => { - transitionStateFlags.removeFlag(State.Opening) - events.current.afterEnter?.() - }, - leave: () => { - transitionStateFlags.removeFlag(State.Closing) - events.current.afterLeave?.() - }, - idle: () => {}, - }) - }) - let isTransitioning = useRef(false) let nesting = useNesting(() => { @@ -443,77 +374,97 @@ function TransitionChildFn { - isTransitioning.current = true - nesting.onStart(container, direction, beforeEvent) - }), - onStop: useLatestValue((direction) => { - isTransitioning.current = false - nesting.onStop(container, direction, afterEvent) - - if (direction === 'leave' && !hasChildren(nesting)) { - // When we don't have children anymore we can safely unregister from the - // parent and hide ourselves. - setState(TreeStates.Hidden) - unregister(container) - } - }), + let start = useEvent((show: boolean) => { + isTransitioning.current = true + let direction: TransitionDirection = show ? 'enter' : 'leave' + + nesting.onStart(container, direction, (direction) => { + if (direction === 'enter') beforeEnter?.() + else if (direction === 'leave') beforeLeave?.() + }) }) - let theirProps = rest - let ourProps = { ref: transitionRef } + let end = useEvent((show: boolean) => { + let direction: TransitionDirection = show ? 'enter' : 'leave' - // Already apply the `enter` and `enterFrom` on the server if required - if (immediate) { - theirProps = { - ...theirProps, - className: classNames(rest.className, ...classes.current.enter, ...classes.current.enterFrom), - } - } + isTransitioning.current = false + nesting.onStop(container, direction, (direction) => { + if (direction === 'enter') afterEnter?.() + else if (direction === 'leave') afterLeave?.() + }) - // If we are re-rendering while we are transitioning, then we should ensure that the classes are - // not mutated by React itself because we are handling the transition ourself. - else if (isTransitioning.current) { - // When we re-render while we are in the middle of the transition, then we should take the - // incoming className and the current classes that are applied. - // - // This is a bit dirty, but we need to make sure React is not applying changes to the class - // attribute while we are transitioning. - theirProps.className = classNames(rest.className, container.current?.className) - if (theirProps.className === '') delete theirProps.className - } + if (direction === 'leave' && !hasChildren(nesting)) { + // When we don't have children anymore we can safely unregister from the + // parent and hide ourselves. + setState(TreeStates.Hidden) + unregister(container) + } + }) - // If we were never transitioning, or we're not transitioning anymore, then - // apply the `enterTo` and `leaveTo` classes as the final state. - else { - theirProps.className = classNames( - rest.className, - container.current?.className, - ...match(transitionDirection, { - enter: [...classes.current.enterTo, ...classes.current.entered], - leave: classes.current.leaveTo, - idle: [], - }) - ) - if (theirProps.className === '') delete theirProps.className - } + useEffect(() => { + if (requiresRef) return + + // When we don't transition, then we can complete the transition + // immediately. + start(show) + end(show) + }, [show, requiresRef]) + + let enabled = (() => { + // If we don't require a ref, then we can't transition. + if (!requiresRef) return false + + // If the server handoff isn't completed yet, we can't transition. + if (!ready) return false + + // If we start in a `show` state but without the `appear` prop, then we skip + // the initial transition. + if (skip) return false + + return true + })() + + // Ignoring the `visible` state because this doesn't handle the hierarchy. If + // a leave transition on the `` is done, but there is still a + // child `` busy, then `visible` would be `false`, while + // `state` would still be `TreeStates.Visible`. + let [, slot] = useTransitionData(enabled, container, show, { start, end }) + + let ourProps = compact({ + ref: transitionRef, + className: + classNames( + // Incoming classes if any + theirProps.className, + + // Already apply these classes immediately + immediate && enter, + immediate && enterFrom, + + // Map data attributes to `enter`, `enterFrom` and `enterTo` classes + slot.enter && enter, + slot.enter && slot.closed && enterFrom, + slot.enter && !slot.closed && enterTo, + + // Map data attributes to `leave`, `leaveFrom` and `leaveTo` classes + slot.leave && leave, + slot.leave && !slot.closed && leaveFrom, + slot.leave && slot.closed && leaveTo, + + // Map data attributes to `entered` class (backwards compatibility) + !slot.transition && show && entered + )?.trim() || undefined, // If `className` is an empty string, we can omit it + }) - let [, slot] = useTransitionData(ready, container, show) + let openClosedState = 0 + if (state === TreeStates.Visible) openClosedState |= State.Open + if (state === TreeStates.Hidden) openClosedState |= State.Closed + if (slot.enter) openClosedState |= State.Opening + if (slot.leave) openClosedState |= State.Closing return ( - + {render({ ourProps, theirProps, From bcd6b2435978f74dad663abbf3fe9b19f4574d18 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 19 Jun 2024 12:14:57 +0200 Subject: [PATCH 03/13] update tests Due to a timing issue bug, we updated the snapshot tests in https://github.com/tailwindlabs/headlessui/pull/3273 incorrectly so this commit fixes that which is why there are a lot of changes. Most tests have `show={true}` but not `appear` which means that they should _not_ transition which means that no data attributes should be present. --- .../__snapshots__/transition.test.tsx.snap | 158 +++--------------- .../components/transition/transition.test.tsx | 14 +- 2 files changed, 23 insertions(+), 149 deletions(-) diff --git a/packages/@headlessui-react/src/components/transition/__snapshots__/transition.test.tsx.snap b/packages/@headlessui-react/src/components/transition/__snapshots__/transition.test.tsx.snap index df81926dec..62cf8201b3 100644 --- a/packages/@headlessui-react/src/components/transition/__snapshots__/transition.test.tsx.snap +++ b/packages/@headlessui-react/src/components/transition/__snapshots__/transition.test.tsx.snap @@ -4,29 +4,11 @@ exports[`Setup API nested should be possible to change the underlying DOM tag of
-
-