Fix Dialog unmounting problem due to incorrect transitioncancel e…

…vent 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
tailwindlabs · Dec 6, 2022 · a6dea8a · a6dea8a
1 parent 5ef5cf9
commit a6dea8a
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 66 deletions.
diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md
@@ -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
 

diff --git a/packages/@headlessui-react/src/components/transitions/utils/transition.test.ts b/packages/@headlessui-react/src/components/transitions/utils/transition.test.ts
@@ -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'
@@ -26,7 +26,7 @@ it('should be possible to transition', async () => {
     )
   )
 
-  await new Promise((resolve) => {
+  await new Promise<void>((resolve) => {
     transition(
       element,
       {
@@ -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,
       {
@@ -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>`)
@@ -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,
       {
@@ -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) /

diff --git a/packages/@headlessui-react/src/components/transitions/utils/transition.ts b/packages/@headlessui-react/src/components/transitions/utils/transition.ts
@@ -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
@@ -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
 }
@@ -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()
@@ -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()
     })
   })
 

diff --git a/packages/@headlessui-react/src/hooks/use-transition.ts b/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'
@@ -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)
       })
     )