diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index be3c86407..ed81d0670 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- Nothing yet! +### Fixed + +- Fix transition bug on Firefox, triggered by clicking the `PopoverButton` in rapid succession ([#3452](https://github.com/tailwindlabs/headlessui/pull/3452)) ## [2.1.4] - 2024-09-03 diff --git a/packages/@headlessui-react/jest.setup.js b/packages/@headlessui-react/jest.setup.js index ef4af5454..771a30cb9 100644 --- a/packages/@headlessui-react/jest.setup.js +++ b/packages/@headlessui-react/jest.setup.js @@ -1 +1,46 @@ globalThis.IS_REACT_ACT_ENVIRONMENT = true + +// These are not 1:1 perfect polyfills, but they implement the parts we need for +// testing. The implementation of the `getAnimations` uses the `setTimeout` +// approach we used in the past. +// +// This is only necessary because JSDOM does not implement `getAnimations` or +// `CSSTransition` yet. This is a temporary solution until JSDOM implements +// these features. Or, until we use proper browser tests using Puppeteer or +// Playwright. +{ + if (typeof CSSTransition === 'undefined') { + globalThis.CSSTransition = class CSSTransition { + constructor(duration) { + this.duration = duration + } + + finished = new Promise((resolve) => { + setTimeout(resolve, this.duration) + }) + } + } + + if (typeof Element.prototype.getAnimations !== 'function') { + Element.prototype.getAnimations = function () { + let { transitionDuration, transitionDelay } = getComputedStyle(this) + + let [durationMs, delayMs] = [transitionDuration, transitionDelay].map((value) => { + let [resolvedValue = 0] = value + .split(',') + // Remove falsy we can't work with + .filter(Boolean) + // Values are returned as `0.3s` or `75ms` + .map((v) => (v.includes('ms') ? parseFloat(v) : parseFloat(v) * 1000)) + .sort((a, z) => z - a) + + return resolvedValue + }) + + let totalDuration = durationMs + delayMs + if (totalDuration === 0) return [] + + return [new CSSTransition(totalDuration)] + } + } +} diff --git a/packages/@headlessui-react/src/components/transition/transition.test.tsx b/packages/@headlessui-react/src/components/transition/transition.test.tsx index 5911fbc7d..137373393 100644 --- a/packages/@headlessui-react/src/components/transition/transition.test.tsx +++ b/packages/@headlessui-react/src/components/transition/transition.test.tsx @@ -1050,8 +1050,8 @@ describe('Events', () => { 'child-2-2: beforeEnter', 'child-1: afterEnter', - 'child-2-2: afterEnter', 'child-2-1: afterEnter', + 'child-2-2: afterEnter', 'child-2: afterEnter', 'root: afterEnter', @@ -1064,8 +1064,8 @@ describe('Events', () => { 'root: beforeLeave', 'child-1: afterLeave', - 'child-2-2: afterLeave', 'child-2-1: afterLeave', + 'child-2-2: afterLeave', 'child-2: afterLeave', 'root: afterLeave', @@ -1078,8 +1078,8 @@ describe('Events', () => { 'child-2-2: beforeEnter', 'child-1: afterEnter', - 'child-2-2: afterEnter', 'child-2-1: afterEnter', + 'child-2-2: afterEnter', 'child-2: afterEnter', 'root: afterEnter', @@ -1092,8 +1092,8 @@ describe('Events', () => { 'root: beforeLeave', 'child-1: afterLeave', - 'child-2-2: afterLeave', 'child-2-1: afterLeave', + 'child-2-2: afterLeave', 'child-2: afterLeave', 'root: afterLeave', ]) diff --git a/packages/@headlessui-react/src/hooks/use-transition.ts b/packages/@headlessui-react/src/hooks/use-transition.ts index ae2602e6f..6c83f466c 100644 --- a/packages/@headlessui-react/src/hooks/use-transition.ts +++ b/packages/@headlessui-react/src/hooks/use-transition.ts @@ -1,6 +1,5 @@ import { useRef, useState, type MutableRefObject } from 'react' import { disposables } from '../utils/disposables' -import { once } from '../utils/once' import { useDisposables } from './use-disposables' import { useFlags } from './use-flags' import { useIsoMorphicEffect } from './use-iso-morphic-effect' @@ -211,84 +210,43 @@ function transition( // This means that no transition happens at all. To fix this, we delay the // actual transition by one frame. 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, done)) - // Initiate the transition by applying the new classes. run() + + // Wait for the transition, once the transition is complete we can cleanup. + // We wait for a frame such that the browser has time to flush the changes + // to the DOM. + d.requestAnimationFrame(() => { + d.add(waitForTransition(node, done)) + }) }) return d.dispose } -function waitForTransition(node: HTMLElement, _done: () => void) { - let done = once(_done) +function waitForTransition(node: HTMLElement | null, done: () => void) { let d = disposables() - if (!node) return d.dispose - // Safari returns a comma separated list of values, so let's sort them and take the highest value. - let { transitionDuration, transitionDelay } = getComputedStyle(node) - - let [durationMs, delayMs] = [transitionDuration, transitionDelay].map((value) => { - let [resolvedValue = 0] = value - .split(',') - // Remove falsy we can't work with - .filter(Boolean) - // Values are returned as `0.3s` or `75ms` - .map((v) => (v.includes('ms') ? parseFloat(v) : parseFloat(v) * 1000)) - .sort((a, z) => z - a) - - return resolvedValue + let cancelled = false + d.add(() => { + cancelled = true }) - let totalDuration = durationMs + delayMs - - if (totalDuration !== 0) { - if (process.env.NODE_ENV === 'test') { - let dispose = d.setTimeout(() => { - done() - dispose() - }, totalDuration) - } else { - let disposeGroup = d.group((d) => { - // Mark the transition as done when the timeout is reached. This is a fallback in case the - // transitionrun event is not fired. - let cancelTimeout = d.setTimeout(() => { - done() - d.dispose() - }, totalDuration) - - // The moment the transitionrun event fires, we should cleanup the timeout fallback, because - // then we know that we can use the native transition events because something is - // transitioning. - d.addEventListener(node, 'transitionrun', (event) => { - if (event.target !== event.currentTarget) return - cancelTimeout() - - d.addEventListener(node, 'transitioncancel', (event) => { - if (event.target !== event.currentTarget) return - done() - disposeGroup() - }) - }) - }) - - d.addEventListener(node, 'transitionend', (event) => { - if (event.target !== event.currentTarget) return - done() - d.dispose() - }) - } - } else { - // No transition is happening, so we should cleanup already. Otherwise we have to wait until we - // get disposed. + let transitions = node.getAnimations().filter((animation) => animation instanceof CSSTransition) + // If there are no transitions, we can stop early. + if (transitions.length === 0) { done() + return d.dispose } + // Wait for all the transitions to complete. + Promise.allSettled(transitions.map((transition) => transition.finished)).then(() => { + if (!cancelled) { + done() + } + }) + return d.dispose }