e2e tests: framer-motion animations are not disabled by plugin disabling CSS animations

[ciampo]

What

The disable-animation plugin runs during e2e tests in order to make them "more stable" (see original PR).

But this set of styles don't have an impact on animations happening via the framer-motion library (as noted by @talldan in #43186 (review)) — this is because framer-motion "manually" sets the animated values as inline styles for each frame (instead of relying on native CSS animations and transforms).

Proposed next steps

The best solution that comes to my mind is:

• add a script to the disable-animation plugin, setting a global variable that can be read in app code
• add the MotionConfig component (see docs): when the global variable from the point before is defined, the reducedMotion option can be set to always, while normally it would be set to user

It should be noted that this is only a suboptimal solution:

• From the docs: "When motion is reduced, transform and layout animations will be disabled. Other animations, like opacity and backgroundColor, will persist." — so this wouldn't fix entirely the issue (disabling all transitions/animations)
• this is not great, since our tests wouldn't be running the same code that our user see

An alternative approach could be to manually check for that global "e2e_disable_css_animations" variable in the React code, and conditionally render an alternative to framer-motion components that doesn't animate (i.e. a normal div instead of a motion.div) — although this approach would also mean that our e2e tests would not actually test the same exact components that are rendered in our user's browsers

[talldan]

Puppeteer does have a way to emulate reduced motion, but prior attempts to use it didn't work - #18453.

There have been quite a few updates of puppeteer since, so things may be different.

[ciampo]

Should we be try to emulate reduced motion in addition to "force-disabling" all CSS animations, or as an alternative?

The original intention of the e2e tests plugin was to disable all animations in order to make e2e tests "more stable" — if that intention still holds true today, simulating reduced motion likely won't give the same results (some animations may still be happening) and we may need to find alternative options.

I also wonder if this plugin is necessary — as state before, by disabling all CSS animations we're making our e2e tests less realistic (compared to what the final user will interact with)

[talldan]

Should we be try to emulate reduced motion in addition to "force-disabling" all CSS animations, or as an alternative?

Possibly both? My main concern is test reliability, which is what makes me think both.

I also wonder if this plugin is necessary — as state before, by disabling all CSS animations we're making our e2e tests less realistic (compared to what the final user will interact with)

I think I've seen this discussion before somewhere (but I wasn't participating). For Puppeteer, the tests are already regularly flakey, and historically written in a way where they expect elements to be visible instantly. It may not be realistic to turn all animations on, a lot of tests would need to be updated to use waitForSelector.

For Playwright, the argument is a bit simpler. Animations being active does make tests run more slowly, but given Playwright does auto-waiting there should really be no difference between having animations on and off in terms of whether tests pass.

I think the best thing for the long term is to continue facilitating the migration to Playwright so that this is a simpler choice.

[WunderBart]

Unfortunately, some components still seem to fail with Playwright, even when the reduced motion is set and animations are disabled (natively). I recorded those components during the tests and noticed that they are animating into their default/initial state. I've checked if the transitionend event is fired for those components, and it is, just not instantly. This probably means that Playwright can't fast-forward those animations to completion as it claims in its documentation. I wouldn't say it's Playwright's fault, though – probably not even Framer-Motion's. At this point, I think it might have something to do with how we initialize those components that fail the screenshot test, as I don't think we want them to be animating into their initial states. Here's the list of those that I found to be problematic, along with the recordings of their initialization:

• Components/FontSizePicker: Default

  video.webm

• Components/FontSizePicker: With Custom Sizes Disabled

  video.webm

• Components/FontSizePicker: With Complex CSS Values

  video.webm

• Components/FontSizePicker: With Units

  video.webm

• Components/Popover: Dynamic Height

  video.webm

• Components/Popover: With Slot Outside Iframe

  video.webm

• Components/Popover: All Placements

  video.webm

• Components (Experimental)/Navigator: Default

  video.webm

Hope it helps! 🙌

[ciampo]

Thank you @WunderBart !

We can look into removing initial animation from these components:

• Popover
• ToggleGroupControl (responsible for the FontSizePicker animations)
• Navigator

During your tests, were the rest of the framer-motion animations (i.e. the ones happening as the result of user interaction) detected correctly by Playwright ?

[mirka]

Sorry, I was under the impression that ToggleGroupControl was migrated to framer-motion but I guess it's not? It does its own transition handling.

[ciampo]

Sorry, I was under the impression that ToggleGroupControl was migrated to framer-motion but I guess it's not? It does its own transition handling.

I had started working on it — I opened a first attempt, which was then superseded by a second attempt using shared layout animations, which couldn't be merged because of a bug in framer-motion

[ciampo]

I also had another thought — if we wanted to disable framer-motion animations, what if we mocked framer-motion components?(motion.div mocked to render a div, ...)

[WunderBart]

During your tests, were the rest of the framer-motion animations (i.e. the ones happening as the result of user interaction) detected correctly by Playwright ?

@ciampo I haven't tested any user interactions yet - just the initial part. Which components do you think could be problematic? I'd be happy to do some more testing to learn more about FM behavior 👍

edit: I guess I can start from the FontSizePicker as it seems to be a good starting point 😄

[ciampo]

@ciampo I haven't tested any user interactions yet - just the initial part. Which components do you think could be problematic? I'd be happy to do some more testing to learn more about FM behavior 👍

edit: I guess I can start from the FontSizePicker as it seems to be a good starting point 😄

You could test FontSizePicker as it's an interesting one (even if it doesn't use framer-motion for animations)

Of the ones you listed, Popover would be a good candidate, since it features often in the editor and is often animated

[WunderBart]

You could test FontSizePicker as it's an interesting one (even if it doesn't use framer-motion for animations)

Huh, I should have checked myself, but I assumed that FontSizePicker does use the FM animation, hence problems with stability in Playwright as well. Why would Playwright fail handling those animations as well then? 🤔 This one's getting even more interesting. I'll keep digging. /cc @mirka

[WunderBart]

@ciampo I haven't tested any user interactions yet - just the initial part. Which components do you think could be problematic? I'd be happy to do some more testing to learn more about FM behavior 👍
edit: I guess I can start from the FontSizePicker as it seems to be a good starting point 😄

You could test FontSizePicker as it's an interesting one (even if it doesn't use framer-motion for animations)

Of the ones you listed, Popover would be a good candidate, since it features often in the editor and is often animated

OK, so I've been testing the FontSizePicker (non-FM) and Popover (FM) a bit more and it looks like they both respond well to explicit stability checks for their animated elements. Those elements would be the button highlight for the Picker and for the Popover, the popover element itself. Here's how a succesful interaction test could be written for the Picker:

// font-size-picker.test.ts

test( 'Can change size to "Small"', async ( { page } ) => {
	const button = await page.locator( 'button[aria-label="Small"]' );
	await button.click();
	const buttonHighlight = await page.waitForSelector(
		'[aria-label="Font size"] > div[role=presentation]'
	);
	await buttonHighlight?.waitForElementState( 'stable' );

	expect( button ).toHaveAttribute( 'aria-checked', 'true' );
	expect( await page.screenshot() ).toMatchSnapshot();
} );

...and for the Popover component:

// popover.test.ts

test( 'Should pop over', async ( { page } ) => {
	await page.click( 'role=button' );
	const popover = await page.waitForSelector( '.components-popover' );
	await popover.waitForElementState( 'stable' );

	expect( await page.screenshot() ).toMatchSnapshot();
} );

I'm still not sure why turning animation off doesn't work, but I don't think it's that important right now. If we can reliably test with animations on, I think we should do it as it has way more value than the other way around. It shows the intent behind those animations because we explicitly need to account for them in the tests. For example, those initial animations that I mentioned above - if we were able to just turn them off in the first place, the tests would just pass ignoring the fact that they might be an unwanted side-effect! 💪 Let me know what you think, @ciampo!

[ciampo]

I was reading about waitForElement( 'stable' ) and it looks like it will only work for animations that cause the element to "move", and therefore it shouldn't be able to support waiting for, i.e., opacity changes — not sure if that would cause any issues.

Out of curiosity, would the .waitForElementState( 'stable' ); technique work also for other e2e tests that are not screenshot/snapshot based, for both non-FM and FM based animating components?

Would there ever be the option to remove the disable-animation plugin and just use .waitForElementState( 'stable' ); instead?

Specifically to screenshots, it looks like the toHaveScreenshot assertion should disable animations by default, although I'm not sure it would work with FM components

[WunderBart]

I was reading about waitForElement( 'stable' ) and microsoft/playwright#4055 it will only work for animations that cause the element to "move", and therefore it shouldn't be able to support waiting for, i.e., opacity changes — not sure if that would cause any issues.

If the only thing we need to wait for is the element's opacity (its bounding box is static), then for the screenshot to be done at the right moment, I suppose we'd need a custom waiter. This should be doable as we can wait for the opacity via getComputedStyle() in a similar manner as the stability check.

Out of curiosity, would the .waitForElementState( 'stable' ); technique work also for other e2e tests that are not screenshot/snapshot based, for both non-FM and FM based animating components?

Yes, as long as the bounding box of the element is moving, the stability check should keep working.

Specifically to screenshots, it looks like the toHaveScreenshot assertion should disable animations by default, although I'm not sure it would work with FM components

It doesn't look to be working ATM reliably, true. In my testing, it didn't even work reliably for the FontSizePicker component (non-FM), but as I mentioned above, it could be because the initial animation is triggered after the component is rendered. Anyway, I think we can drop the pursuit of disabling animations and include them in the tests as it provides more value and is well handled by Playwright.

[WunderBart]

Would there ever be the option to remove the disable-animation plugin and just use .waitForElementState( 'stable' ); instead?

Definitely for Playwright, which I think should be done by default. Are we disabling animations for Playwright as we did for Puppeteer? cc @kevin940726

[kevin940726]

Nope. We are not activating that plugin in Playwright tests.

If somehow something breaks while screenshotting, friendly reminder that we can always use expect.poll to wait for something to happen as an escape hatch.

await expect.poll(
	() => locator.evaluate( ( node ) => getComputedStyle( node ).opacity )
).toBe( '1' )