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 Dialog unmounting problem due to incorrect transitioncancel event in the Transition component on Android #2071

Merged
merged 3 commits into from Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix crash when using `multiple` mode without `value` prop (uncontrolled) for `Listbox` and `Combobox` components ([#2058](https://github.com/tailwindlabs/headlessui/pull/2058))
- Apply `enter` and `enterFrom` classes in SSR for `Transition` component ([#2059](https://github.com/tailwindlabs/headlessui/pull/2059))
- Allow passing in your own `id` prop ([#2060](https://github.com/tailwindlabs/headlessui/pull/2060))
- Fix `Dialog` unmounting problem due to incorrect `transitioncancel` event in the `Transition` component on Android ([#2071](https://github.com/tailwindlabs/headlessui/pull/2071))

## [1.7.4] - 2022-11-03

Expand Down
@@ -1,4 +1,4 @@
import { Reason, transition } from './transition'
import { transition } from './transition'

import { reportChanges } from '../../../test-utils/report-dom-node-changes'
import { disposables } from '../../../utils/disposables'
Expand Down Expand Up @@ -26,7 +26,7 @@ it('should be possible to transition', async () => {
)
)

await new Promise((resolve) => {
await new Promise<void>((resolve) => {
transition(
element,
{
Expand Down Expand Up @@ -83,7 +83,7 @@ it('should wait the correct amount of time to finish a transition', async () =>
)
)

let reason = await new Promise((resolve) => {
await new Promise<void>((resolve) => {
transition(
element,
{
Expand All @@ -101,7 +101,6 @@ it('should wait the correct amount of time to finish a transition', async () =>
})

await new Promise((resolve) => d.nextFrame(resolve))
expect(reason).toBe(Reason.Ended)

// Initial render:
expect(snapshots[0].content).toEqual(`<div style="transition-duration: ${duration}ms;"></div>`)
Expand Down Expand Up @@ -153,7 +152,7 @@ it('should keep the delay time into account', async () => {
)
)

let reason = await new Promise((resolve) => {
await new Promise<void>((resolve) => {
transition(
element,
{
Expand All @@ -171,7 +170,6 @@ it('should keep the delay time into account', async () => {
})

await new Promise((resolve) => d.nextFrame(resolve))
expect(reason).toBe(Reason.Ended)

let estimatedDuration = Number(
(snapshots[snapshots.length - 1].recordedAt - snapshots[snapshots.length - 2].recordedAt) /
Expand Down
Expand Up @@ -10,15 +10,7 @@ function removeClasses(node: HTMLElement, ...classes: string[]) {
node && classes.length > 0 && node.classList.remove(...classes)
}

export enum Reason {
// Transition succesfully ended
Ended = 'ended',

// Transition was cancelled
Cancelled = 'cancelled',
}

function waitForTransition(node: HTMLElement, done: (reason: Reason) => void) {
function waitForTransition(node: HTMLElement, done: () => void) {
let d = disposables()

if (!node) return d.dispose
Expand All @@ -41,49 +33,26 @@ function waitForTransition(node: HTMLElement, done: (reason: Reason) => void) {
let totalDuration = durationMs + delayMs

if (totalDuration !== 0) {
let listeners: (() => void)[] = []

if (process.env.NODE_ENV === 'test') {
listeners.push(
d.setTimeout(() => {
done(Reason.Ended)
listeners.splice(0).forEach((dispose) => dispose())
}, totalDuration)
)
let dispose = d.setTimeout(() => {
done()
dispose()
}, totalDuration)
} else {
listeners.push(
d.addEventListener(node, 'transitionrun', (event) => {
if (event.target !== event.currentTarget) return

// Cleanup "old" listeners
listeners.splice(0).forEach((dispose) => dispose())

// Register new listeners
listeners.push(
d.addEventListener(node, 'transitionend', (event) => {
if (event.target !== event.currentTarget) return

done(Reason.Ended)
listeners.splice(0).forEach((dispose) => dispose())
}),
d.addEventListener(node, 'transitioncancel', (event) => {
if (event.target !== event.currentTarget) return

done(Reason.Cancelled)
listeners.splice(0).forEach((dispose) => dispose())
})
)
})
)
let dispose = d.addEventListener(node, 'transitionend', (event) => {
if (event.target !== event.currentTarget) return
done()
dispose()
})
}
} else {
// No transition is happening, so we should cleanup already. Otherwise we have to wait until we
// get disposed.
done(Reason.Ended)
done()
}

// If we get disposed before the transition finishes, we should cleanup anyway.
d.add(() => done(Reason.Cancelled))
d.add(() => done())

return d.dispose
}
Expand All @@ -100,7 +69,7 @@ export function transition(
entered: string[]
},
show: boolean,
done?: (reason: Reason) => void
done?: () => void
) {
let direction = show ? 'enter' : 'leave'
let d = disposables()
Expand Down Expand Up @@ -144,13 +113,11 @@ export function transition(
removeClasses(node, ...from)
addClasses(node, ...to)

waitForTransition(node, (reason) => {
if (reason === Reason.Ended) {
removeClasses(node, ...base)
addClasses(node, ...classes.entered)
}
waitForTransition(node, () => {
removeClasses(node, ...base)
addClasses(node, ...classes.entered)

return _done(reason)
return _done()
})
})

Expand Down
13 changes: 3 additions & 10 deletions packages/@headlessui-react/src/hooks/use-transition.ts
@@ -1,8 +1,7 @@
import { MutableRefObject } from 'react'

import { Reason, transition } from '../components/transitions/utils/transition'
import { transition } from '../components/transitions/utils/transition'
import { disposables } from '../utils/disposables'
import { match } from '../utils/match'

import { useDisposables } from './use-disposables'
import { useIsMounted } from './use-is-mounted'
Expand Down Expand Up @@ -47,15 +46,9 @@ export function useTransition({ container, direction, classes, onStart, onStop }
onStart.current(latestDirection.current)

dd.add(
transition(node, classes.current, latestDirection.current === 'enter', (reason) => {
transition(node, classes.current, latestDirection.current === 'enter', () => {
dd.dispose()

match(reason, {
[Reason.Ended]() {
onStop.current(latestDirection.current)
},
[Reason.Cancelled]: () => {},
})
onStop.current(latestDirection.current)
})
)

Expand Down