Fix transition bug on Firefox, triggered by clicking the `PopoverButton` in rapid succession #3452

RobinMalfait · 2024-09-04T11:55:36Z

We recently landed a fix for Popovers not closing correctly when using the transition prop (#3448). Once this fix was published, some users still ran into issues using Firefox on Windows (see: tailwindlabs/tailwindui-issues#1625).

One fun thing I discovered is that transitions somehow behave differently based on where they are triggered from (?). What I mean by this is that holding down the space key on the button does properly open/close the Popover. But if you rapidly click the button, the Popover will eventually get stuck.

Note: when testing this, I made sure that the handling of the space key (in a keydown handler) and the clicking of the mouse (handled in a click handler) called the exact same code. It still happened.

The debugging continues…

One thing I noticed is that when the Popover gets stuck, it meant that a transition didn't properly complete.

The current implementation of the internal useTransition(…) hook has to wait for all the transitions to finish. This is done using a waitForTransition(…) helper. This helper sets up some event listeners (transitionstart, transitionend, …) and waits for them to fire.

This seems to be unreliable on Firefox for some unknown reason.

I knew the code for waiting for transitions wasn't ideal, so I wanted to see if using the native node.getAnimations() simplifies this and makes it work in general.

Lo and behold, it did! 🎉

This now has multiple benefits:

It works as expected on Firefox
The code is much much simpler
Uses native features

The getAnimations(…) function is supported in all modern browsers (since 2020). At the time it was too early to rely on it, but right now it should be safe to use.

Fixes: tailwindlabs/tailwindui-issues#1625

vercel · 2024-09-04T11:55:39Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
headlessui-react	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 4, 2024 0:11am
headlessui-vue	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 4, 2024 0:11am

…mations`

Up until now, we've tried to do this ourselves by listening to the correct events. The new `node.getAnimations()` API is a much simpler API to use. The only requirement is that we call `node.getAnimations()` in a `requestAnimationFrame` so that the browser can flush all the changes. We couldn't do this before, because we needed to setup the event listeners to prevent race conditions. Now there are no race conditions, in fact, if all transitions already complete before we can call the `waitForTransition`, then the `node.getAnimations()` list will be empty and we can call the `done()` function. The `Element.prototype.getAnimations` has been available in browsers since mid 2020, but at the time it was too new to use. Now seems like a safe time to use this. See: https://developer.mozilla.org/en-US/docs/Web/API/Element/getAnimations#browser_compatibility

The order slightly changed here, but it looks more correct now as well. The `child-2-1` and `child-2-2` have the same duration/delay, so the order that they are rendered in seems to be the correct order. That said, these DOM nodes are on the same level (siblings), so for these callbacks we are testing here the order doesn't really matter. The order that _does_ matter is that the `after*` events of a parent fire _after_ the `after*` events of the children, which it still does.

thecrypticace · 2024-09-04T12:43:44Z

packages/@headlessui-react/src/components/transition/transition.test.tsx

        'child-2-1: afterEnter',
+        'child-2-2: afterEnter',


Do you know why the order of these changed?

It's based on the order that the setTimeout is being called. I can get the old behavior by applying this diff:

diff --git a/packages/@headlessui-react/jest.setup.js b/packages/@headlessui-react/jest.setup.js index 771a30cb..9e656463 100644 --- a/packages/@headlessui-react/jest.setup.js +++ b/packages/@headlessui-react/jest.setup.js @@ -15,9 +15,11 @@ globalThis.IS_REACT_ACT_ENVIRONMENT = true this.duration = duration } - finished = new Promise((resolve) => { - setTimeout(resolve, this.duration) - }) + get finished() { + return new Promise((resolve) => { + setTimeout(resolve, this.duration) + }) + } } }

But the exact reason why I am still unsure about. The order is also only different on the same level (siblings) which is the less important part I think. The more important part is the relation between child and parent and that that order is correct.

That said, note that the order here (and in the old code) was determined by the setTimeout (not by the real events because JSDOM doesn't have proper support for them all).

I could revert the change to the test, and apply the diff above. But it feels incorrect since that would rely on us accessing the finished property, instead of relying on the creation of the CSSTransition object.

yeah that's fine — was just curious and hoping it wasn't a side effect of some "problem"

FWIW, if you look at the browser example of this:

Main: https://headlessui-react.vercel.app/transitions/component-examples/modal

PR: https://headlessui-react-git-fix-improve-transition-20b73f-tailwindlabs.vercel.app/transitions/component-examples/modal

The output is in the exact same order (notice the order of the siblings "overlay" and "panel")

Main version:

Events: 2024-09-04T14:04:49.428Z - [Root] Before enter 2024-09-04T14:04:49.430Z - [Overlay] Before enter 2024-09-04T14:04:49.430Z - [Panel] Before enter 2024-09-04T14:04:49.765Z - [Overlay] After enter 2024-09-04T14:04:49.794Z - [Panel] After enter 2024-09-04T14:04:49.794Z - [Root] After enter 2024-09-04T14:04:50.873Z - [Overlay] Before leave 2024-09-04T14:04:50.873Z - [Panel] Before leave 2024-09-04T14:04:50.873Z - [Root] Before leave 2024-09-04T14:04:51.116Z - [Overlay] After leave 2024-09-04T14:04:51.124Z - [Panel] After leave 2024-09-04T14:04:51.124Z - [Root] After leave

PR version:

Events: 2024-09-04T14:04:17.440Z - [Root] Before enter 2024-09-04T14:04:17.441Z - [Overlay] Before enter 2024-09-04T14:04:17.441Z - [Panel] Before enter 2024-09-04T14:04:17.781Z - [Overlay] After enter 2024-09-04T14:04:17.820Z - [Panel] After enter 2024-09-04T14:04:17.820Z - [Root] After enter 2024-09-04T14:04:19.504Z - [Overlay] Before leave 2024-09-04T14:04:19.504Z - [Panel] Before leave 2024-09-04T14:04:19.504Z - [Root] Before leave 2024-09-04T14:04:19.765Z - [Overlay] After leave 2024-09-04T14:04:19.767Z - [Panel] After leave 2024-09-04T14:04:19.767Z - [Root] After leave

razzeee · 2024-09-09T13:27:14Z

We now have jest tests failing with


    TypeError: node.getAnimations is not a function

      at waitForTransition (node_modules/@headlessui/react/dist/headlessui.dev.cjs:3752:26)
      at node_modules/@headlessui/react/dist/headlessui.dev.cjs:3739:13
      at invokeTheCallbackFunction (node_modules/jsdom/lib/jsdom/living/generated/Function.js:19:26)
      at runAnimationFrameCallbacks (node_modules/jsdom/lib/jsdom/browser/Window.js:603:13)
      at Timeout.<anonymous> (node_modules/jsdom/lib/jsdom/browser/Window.js:581:11)

corneliusroemer · 2024-09-10T14:00:51Z

Indeed, also having a regression, see #3469

jrmuizel · 2024-10-03T17:52:15Z

@RobinMalfait we got a bug report on linktr.ee that I believe might be fixed by this. https://bugzilla.mozilla.org/show_bug.cgi?id=1916176

I'd like to better understand why Firefox isn't firing the transitionend events that you expect. Do you have a standalone test case that reproduces this?

vercel bot deployed to Preview – headlessui-vue September 4, 2024 11:56 View deployment

RobinMalfait added 3 commits September 4, 2024 13:56

add basic polyfills for CSSTransition and `Element.prototype.getAni…

bbf345d

…mations`

RobinMalfait force-pushed the fix/improve-transition-management branch from fe1b1a6 to aa9e189 Compare September 4, 2024 11:57

vercel bot deployed to Preview – headlessui-vue September 4, 2024 11:58 View deployment

vercel bot deployed to Preview – headlessui-react September 4, 2024 11:59 View deployment

RobinMalfait changed the title ~~Improve waiting for transitions~~ Fix bug when waiting for transitions when clicking the PopoverButton in rapid succession on Firefox Sep 4, 2024

update changelog

03da4d5

RobinMalfait force-pushed the fix/improve-transition-management branch from 4bdddbe to 03da4d5 Compare September 4, 2024 12:05

RobinMalfait changed the title ~~Fix bug when waiting for transitions when clicking the PopoverButton in rapid succession on Firefox~~ Fix transition bug on Firefox, triggered by clicking the PopoverButton in rapid succession Sep 4, 2024

RobinMalfait requested a review from thecrypticace September 4, 2024 12:06

vercel bot deployed to Preview – headlessui-vue September 4, 2024 12:06 View deployment

vercel bot deployed to Preview – headlessui-react September 4, 2024 12:07 View deployment

use consistent cancelled naming

03d80f4

vercel bot deployed to Preview – headlessui-vue September 4, 2024 12:11 View deployment

vercel bot deployed to Preview – headlessui-react September 4, 2024 12:11 View deployment

thecrypticace reviewed Sep 4, 2024

View reviewed changes

thecrypticace approved these changes Sep 4, 2024

View reviewed changes

RobinMalfait merged commit 971ff6b into main Sep 4, 2024
8 checks passed

RobinMalfait deleted the fix/improve-transition-management branch September 4, 2024 14:09

RobinMalfait mentioned this pull request Sep 10, 2024

2.1.5 breaks tests due to node.getAnimations #3468

Closed

roomman mentioned this pull request Sep 10, 2024

TypeError: node.getAnimations is not a function #3470

Closed

k2xl mentioned this pull request Sep 26, 2024

[Snyk] Upgrade @headlessui/react from 2.1.2 to 2.1.5 sspenst/thinky.gg#1172

Closed

This was referenced Sep 26, 2024

[Snyk] Upgrade @headlessui/react from 2.1.3 to 2.1.5 PetersonGuo/PetersonGuo#91

Open

[Snyk] Upgrade @headlessui/react from 2.1.3 to 2.1.6 PetersonGuo/PetersonGuo#95

Open

k2xl mentioned this pull request Oct 1, 2024

[Snyk] Upgrade @headlessui/react from 2.1.2 to 2.1.6 sspenst/thinky.gg#1176

Closed

PetersonGuo mentioned this pull request Oct 3, 2024

[Snyk] Upgrade @headlessui/react from 2.1.3 to 2.1.7 PetersonGuo/PetersonGuo#96

Open

k2xl mentioned this pull request Oct 3, 2024

[Snyk] Upgrade @headlessui/react from 2.1.2 to 2.1.7 sspenst/thinky.gg#1182

Closed

PetersonGuo mentioned this pull request Oct 4, 2024

[Snyk] Upgrade @headlessui/react from 2.1.3 to 2.1.8 PetersonGuo/PetersonGuo#97

Open

k2xl mentioned this pull request Oct 4, 2024

[Snyk] Upgrade @headlessui/react from 2.1.2 to 2.1.8 sspenst/thinky.gg#1184

Closed

NiallJoeMaher mentioned this pull request Oct 4, 2024

[Snyk] Upgrade @headlessui/react from 2.1.4 to 2.1.8 codu-code/codu#1062

Merged

snyk-io bot mentioned this pull request Oct 6, 2024

[Snyk] Upgrade @headlessui/react from 2.0.3 to 2.1.8 Opetushallitus/jod-design-system#157

Closed

PetersonGuo mentioned this pull request Oct 30, 2024

[Snyk] Upgrade @headlessui/react from 2.1.3 to 2.1.9 PetersonGuo/PetersonGuo#107

Open

shacthulu mentioned this pull request Nov 4, 2024

[Snyk] Upgrade @headlessui/react from 2.1.2 to 2.1.10 shacthulu/chadteat-website#14

Open

PetersonGuo mentioned this pull request Nov 6, 2024

[Snyk] Upgrade @headlessui/react from 2.1.3 to 2.1.10 PetersonGuo/PetersonGuo#112

Open

0xbalazstoth mentioned this pull request Nov 6, 2024

[Snyk] Upgrade @headlessui/react from 2.1.2 to 2.1.10 0xbalazstoth/datveez#2

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix transition bug on Firefox, triggered by clicking the `PopoverButton` in rapid succession #3452

Fix transition bug on Firefox, triggered by clicking the `PopoverButton` in rapid succession #3452

RobinMalfait commented Sep 4, 2024 •

edited

Loading

vercel bot commented Sep 4, 2024 •

edited

Loading

thecrypticace Sep 4, 2024

RobinMalfait Sep 4, 2024

RobinMalfait Sep 4, 2024

thecrypticace Sep 4, 2024

RobinMalfait Sep 4, 2024

thecrypticace Sep 4, 2024

razzeee commented Sep 9, 2024

corneliusroemer commented Sep 10, 2024

jrmuizel commented Oct 3, 2024

Fix transition bug on Firefox, triggered by clicking the PopoverButton in rapid succession #3452

Fix transition bug on Firefox, triggered by clicking the PopoverButton in rapid succession #3452

Conversation

RobinMalfait commented Sep 4, 2024 • edited Loading

vercel bot commented Sep 4, 2024 • edited Loading

thecrypticace Sep 4, 2024

Choose a reason for hiding this comment

RobinMalfait Sep 4, 2024

Choose a reason for hiding this comment

RobinMalfait Sep 4, 2024

Choose a reason for hiding this comment

thecrypticace Sep 4, 2024

Choose a reason for hiding this comment

RobinMalfait Sep 4, 2024

Choose a reason for hiding this comment

thecrypticace Sep 4, 2024

Choose a reason for hiding this comment

razzeee commented Sep 9, 2024

corneliusroemer commented Sep 10, 2024

jrmuizel commented Oct 3, 2024

Fix transition bug on Firefox, triggered by clicking the `PopoverButton` in rapid succession #3452

Fix transition bug on Firefox, triggered by clicking the `PopoverButton` in rapid succession #3452

RobinMalfait commented Sep 4, 2024 •

edited

Loading

vercel bot commented Sep 4, 2024 •

edited

Loading