Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix enter transitions for the Transition component #3074

Merged
merged 27 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f27367d
improve `transition` logic
RobinMalfait Mar 30, 2024
123fd32
simplify check
RobinMalfait Mar 30, 2024
3513a70
updating playgrounds to improve transitions
RobinMalfait Mar 30, 2024
d0e4787
update changelog
RobinMalfait Apr 2, 2024
025fd7d
track in-flight transitions
RobinMalfait Apr 2, 2024
5c2d990
document `disposables()` and `useDisposables()` functions
RobinMalfait Apr 3, 2024
fb59c52
ensure we alway return a cleanup function
RobinMalfait Apr 3, 2024
3204ea4
move comment
RobinMalfait Apr 3, 2024
f08b995
apply `enterTo` or `leaveTo` classes once we are done transitioning
RobinMalfait Apr 3, 2024
bf11592
cleanup & simplify logic
RobinMalfait Apr 3, 2024
5fcd736
update comment
RobinMalfait Apr 3, 2024
5b86274
fix another bug + update tests
RobinMalfait Apr 3, 2024
b28c2a5
add failing test as playground
RobinMalfait Apr 3, 2024
7b2fb84
simplify event callbacks
RobinMalfait Apr 3, 2024
ce083ca
use existing `useOnDisappear` hook
RobinMalfait Apr 3, 2024
893563d
remove repro
RobinMalfait Apr 3, 2024
b905bf8
only unmount if we are not transitioning ourselves
RobinMalfait Apr 3, 2024
8682bac
`show` is already guaranteed to be a boolean
RobinMalfait Apr 3, 2024
6aa5789
cleanup temporary test changes
RobinMalfait Apr 3, 2024
9acfe6c
Update packages/@headlessui-react/src/components/transition/utils/tra…
RobinMalfait Apr 5, 2024
5b796d9
Update packages/@headlessui-react/src/components/transition/utils/tra…
RobinMalfait Apr 5, 2024
b3cabac
Update packages/@headlessui-react/src/components/transition/utils/tra…
RobinMalfait Apr 5, 2024
53b8471
Update packages/@headlessui-react/src/components/transition/utils/tra…
RobinMalfait Apr 5, 2024
53d8afb
Update packages/@headlessui-react/src/utils/disposables.ts
RobinMalfait Apr 5, 2024
1c60486
Update packages/@headlessui-react/src/components/transition/transitio…
RobinMalfait Apr 5, 2024
ae8c51d
Update packages/@headlessui-react/src/components/transition/transitio…
RobinMalfait Apr 5, 2024
3b310f1
run prettier
RobinMalfait Apr 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix another bug + update tests
  • Loading branch information
RobinMalfait committed Apr 3, 2024
commit 5b8627400fea2a572a5297a3ae62220db0947d1f
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ exports[`Setup API transition classes should be possible to passthrough the tran
exports[`Setup API transition classes should be possible to passthrough the transition classes and immediately apply the enter transitions when appear is set to true 1`] = `
<div
class="enter enter-from"
style=""
>
Children
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ describe('Setup API', () => {
<div
class="foo1
foo2 foo1 foo2 leave"
style=""
reinink marked this conversation as resolved.
Show resolved Hide resolved
>
Children
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
let immediate = appear && show && initial

let transitionDirection = (() => {
if (immediate) return 'enter'
if (!ready) return 'idle'
if (skip) return 'idle'
return show ? 'enter' : 'leave'
Expand Down Expand Up @@ -413,7 +414,6 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_

let isTransitioning = useRef(false)
useTransition({
immediate,
container,
classes,
direction: transitionDirection,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ it('should be possible to transition', async () => {
expect(snapshots[0].content).toEqual('<div></div>')

// Start of transition
expect(snapshots[1].content).toEqual('<div class="enter enterFrom"></div>')
expect(snapshots[1].content).toEqual('<div style="" class="enter enterFrom"></div>')

// NOTE: There is no `enter enterTo`, because we didn't define a duration. Therefore it is not
// necessary to put the classes on the element and immediately remove them.

// Cleanup phase
expect(snapshots[2].content).toEqual('<div class="enterTo entered"></div>')
expect(snapshots[2].content).toEqual('<div style="" class="entered enterTo enter"></div>')

d.dispose()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,25 +150,36 @@ export function transition(
// Mark the transition as in-flight.
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
if (inFlight) inFlight.current = true

// Wait for the transition, once the transition is complete we can cleanup.
// This is registered first to prevent race conditions, otherwise it could
// happen that the transition is already done before we start waiting for
// the actual event.
d.add(
waitForTransition(node, () => {
removeClasses(node, ...classes.base, ...base)
addClasses(node, ...classes.base, ...classes.entered, ...to)

// Mark the transition as done.
if (inFlight) inFlight.current = false

return _done()
})
)

// Initiate the transition by applying the new classes.
removeClasses(node, ...classes.base, ...base, ...from)
addClasses(node, ...classes.base, ...base, ...to)
// BUG: This is a workaround for a bug in all major browsers.
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
//
// 1. When an element is just mounted
// 2. And you apply a transition to it (e.g.: via a class)
// 3. When you use `getComputedStyle` and read any returned value
// 4. Then the `transition` immediately jumps to the end state
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
//
// This means that no transition happen at all. To fix this, we delay the
// actual transition by one frame. This fixes the bug.
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
d.nextFrame(() => {
// Wait for the transition, once the transition is complete we can cleanup.
// This is registered first to prevent race conditions, otherwise it could
// happen that the transition is already done before we start waiting for
// the actual event.
d.add(
waitForTransition(node, () => {
removeClasses(node, ...classes.base, ...base)
addClasses(node, ...classes.base, ...classes.entered, ...to)

// Mark the transition as done.
if (inFlight) inFlight.current = false

return _done()
})
)

// Initiate the transition by applying the new classes.
removeClasses(node, ...classes.base, ...base, ...from)
addClasses(node, ...classes.base, ...base, ...to)
})

return d.dispose
}
Expand Down
28 changes: 5 additions & 23 deletions packages/@headlessui-react/src/hooks/use-transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@ import { transition } from '../components/transition/utils/transition'
import { useDisposables } from './use-disposables'
import { useIsMounted } from './use-is-mounted'
import { useIsoMorphicEffect } from './use-iso-morphic-effect'
import { useLatestValue } from './use-latest-value'

interface TransitionArgs {
immediate: boolean
container: MutableRefObject<HTMLElement | null>
classes: MutableRefObject<{
base: string[]
Expand All @@ -26,47 +24,31 @@ interface TransitionArgs {
onStop: MutableRefObject<(direction: TransitionArgs['direction']) => void>
}

export function useTransition({
immediate,
container,
direction,
classes,
onStart,
onStop,
}: TransitionArgs) {
export function useTransition({ container, direction, classes, onStart, onStop }: TransitionArgs) {
let mounted = useIsMounted()
let d = useDisposables()

let latestDirection = useLatestValue(direction)

// Track whether the transition is in flight or not. This will help us for
// cancelling mid-transition because in that case we don't have to force
// clearing existing transitions. See: `prepareTransition` in the `transition`
// file.
let inFlight = useRef(false)

useIsoMorphicEffect(() => {
if (!immediate) return

latestDirection.current = 'enter'
}, [immediate])

useIsoMorphicEffect(() => {
let node = container.current
if (!node) return // We don't have a DOM node (yet)
if (latestDirection.current === 'idle') return // We don't need to transition
if (direction === 'idle') return // We don't need to transition
if (!mounted.current) return

onStart.current(latestDirection.current)
onStart.current(direction)

d.add(
transition(node, {
direction: latestDirection.current,
direction,
classes: classes.current,
inFlight,
done() {
d.dispose()
onStop.current(latestDirection.current)
onStop.current(direction)
},
})
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,12 @@ describe('Events', () => {
let leaveDuration = 75

withStyles(`
.enter-1, .enter-2, .leave-1, .leave-2 {
transition-property: opacity;
transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1);
transition-duration: 150ms;
}

.enter-1 { transition-duration: ${enterDuration * 1}ms; }
.enter-2 { transition-duration: ${enterDuration * 2}ms; }
.enter-from { opacity: 0%; }
Expand Down
Loading