Skip to content

Commit a6dea8a

Browse files
authored
Fix Dialog unmounting problem due to incorrect transitioncancel event in the Transition component on Android (#2071)
* remove `transitioncancel` logic On Desktop Sarai, Chrome, and on mobile iOS Safari the `transitioncancel` is never called on outside click of the Dialog. However, on mobile Android Chrome it _is_ called, and the `transitionend` is never triggered for _some_ reason. According to the MDN docs: > If the transitioncancel event is fired, the transitionend event will not fire. > > — https://developer.mozilla.org/en-US/docs/Web/API/Element/transitioncancel_event When testing this, I never got into the `transitionend` when I got into the `transitioncancel` first. But, once I removed the `transitioncancel` logic, the `transitionend` code _was_ being called. The code is now both simpler, and works again. The nice part is that we never did anything with the `cancel` event. We marked it as done using the `Reason.Cancelled` and that's about it. * cleanup transition completion `Reason` * update changelog
1 parent 5ef5cf9 commit a6dea8a

File tree

4 files changed

+25
-66
lines changed

4 files changed

+25
-66
lines changed

packages/@headlessui-react/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
- Fix crash when using `multiple` mode without `value` prop (uncontrolled) for `Listbox` and `Combobox` components ([#2058](https://github.com/tailwindlabs/headlessui/pull/2058))
1818
- Apply `enter` and `enterFrom` classes in SSR for `Transition` component ([#2059](https://github.com/tailwindlabs/headlessui/pull/2059))
1919
- Allow passing in your own `id` prop ([#2060](https://github.com/tailwindlabs/headlessui/pull/2060))
20+
- Fix `Dialog` unmounting problem due to incorrect `transitioncancel` event in the `Transition` component on Android ([#2071](https://github.com/tailwindlabs/headlessui/pull/2071))
2021

2122
## [1.7.4] - 2022-11-03
2223

packages/@headlessui-react/src/components/transitions/utils/transition.test.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Reason, transition } from './transition'
1+
import { transition } from './transition'
22

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

29-
await new Promise((resolve) => {
29+
await new Promise<void>((resolve) => {
3030
transition(
3131
element,
3232
{
@@ -83,7 +83,7 @@ it('should wait the correct amount of time to finish a transition', async () =>
8383
)
8484
)
8585

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

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

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

156-
let reason = await new Promise((resolve) => {
155+
await new Promise<void>((resolve) => {
157156
transition(
158157
element,
159158
{
@@ -171,7 +170,6 @@ it('should keep the delay time into account', async () => {
171170
})
172171

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

176174
let estimatedDuration = Number(
177175
(snapshots[snapshots.length - 1].recordedAt - snapshots[snapshots.length - 2].recordedAt) /

packages/@headlessui-react/src/components/transitions/utils/transition.ts

Lines changed: 17 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,7 @@ function removeClasses(node: HTMLElement, ...classes: string[]) {
1010
node && classes.length > 0 && node.classList.remove(...classes)
1111
}
1212

13-
export enum Reason {
14-
// Transition succesfully ended
15-
Ended = 'ended',
16-
17-
// Transition was cancelled
18-
Cancelled = 'cancelled',
19-
}
20-
21-
function waitForTransition(node: HTMLElement, done: (reason: Reason) => void) {
13+
function waitForTransition(node: HTMLElement, done: () => void) {
2214
let d = disposables()
2315

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

4335
if (totalDuration !== 0) {
44-
let listeners: (() => void)[] = []
45-
4636
if (process.env.NODE_ENV === 'test') {
47-
listeners.push(
48-
d.setTimeout(() => {
49-
done(Reason.Ended)
50-
listeners.splice(0).forEach((dispose) => dispose())
51-
}, totalDuration)
52-
)
37+
let dispose = d.setTimeout(() => {
38+
done()
39+
dispose()
40+
}, totalDuration)
5341
} else {
54-
listeners.push(
55-
d.addEventListener(node, 'transitionrun', (event) => {
56-
if (event.target !== event.currentTarget) return
57-
58-
// Cleanup "old" listeners
59-
listeners.splice(0).forEach((dispose) => dispose())
60-
61-
// Register new listeners
62-
listeners.push(
63-
d.addEventListener(node, 'transitionend', (event) => {
64-
if (event.target !== event.currentTarget) return
65-
66-
done(Reason.Ended)
67-
listeners.splice(0).forEach((dispose) => dispose())
68-
}),
69-
d.addEventListener(node, 'transitioncancel', (event) => {
70-
if (event.target !== event.currentTarget) return
71-
72-
done(Reason.Cancelled)
73-
listeners.splice(0).forEach((dispose) => dispose())
74-
})
75-
)
76-
})
77-
)
42+
let dispose = d.addEventListener(node, 'transitionend', (event) => {
43+
if (event.target !== event.currentTarget) return
44+
done()
45+
dispose()
46+
})
7847
}
7948
} else {
8049
// No transition is happening, so we should cleanup already. Otherwise we have to wait until we
8150
// get disposed.
82-
done(Reason.Ended)
51+
done()
8352
}
8453

8554
// If we get disposed before the transition finishes, we should cleanup anyway.
86-
d.add(() => done(Reason.Cancelled))
55+
d.add(() => done())
8756

8857
return d.dispose
8958
}
@@ -100,7 +69,7 @@ export function transition(
10069
entered: string[]
10170
},
10271
show: boolean,
103-
done?: (reason: Reason) => void
72+
done?: () => void
10473
) {
10574
let direction = show ? 'enter' : 'leave'
10675
let d = disposables()
@@ -144,13 +113,11 @@ export function transition(
144113
removeClasses(node, ...from)
145114
addClasses(node, ...to)
146115

147-
waitForTransition(node, (reason) => {
148-
if (reason === Reason.Ended) {
149-
removeClasses(node, ...base)
150-
addClasses(node, ...classes.entered)
151-
}
116+
waitForTransition(node, () => {
117+
removeClasses(node, ...base)
118+
addClasses(node, ...classes.entered)
152119

153-
return _done(reason)
120+
return _done()
154121
})
155122
})
156123

packages/@headlessui-react/src/hooks/use-transition.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { MutableRefObject } from 'react'
22

3-
import { Reason, transition } from '../components/transitions/utils/transition'
3+
import { transition } from '../components/transitions/utils/transition'
44
import { disposables } from '../utils/disposables'
5-
import { match } from '../utils/match'
65

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

4948
dd.add(
50-
transition(node, classes.current, latestDirection.current === 'enter', (reason) => {
49+
transition(node, classes.current, latestDirection.current === 'enter', () => {
5150
dd.dispose()
52-
53-
match(reason, {
54-
[Reason.Ended]() {
55-
onStop.current(latestDirection.current)
56-
},
57-
[Reason.Cancelled]: () => {},
58-
})
51+
onStop.current(latestDirection.current)
5952
})
6053
)
6154

0 commit comments

Comments
 (0)