Widgets Editor in Customizer doesn't load with E2E tests + 0ms Animations

[kirasong]

Moving this Gutenberg issue into trac:
​https://github.com/WordPress/gutenberg/issues/32024

"Widgets Customizer should work without animations and transitions enabled"

Gutenberg End to End tests use a plugin that sets Animations / Transitions to 0ms. When enabled, this results in an empty Widgets Editor in the Customizer, which breaks tests. It would also have an effect on anyone applying user styles to reduce animations to 0ms, but doesn't affect Reduce Motion, which is a separate issue (#53542).

Videos and discovery, along with an initial patch I'll attach shortly, here:
​https://github.com/WordPress/gutenberg/issues/32024#issuecomment-869647305

[zieladam]

LGTM, thank you for getting to the bottom of that!

[desrosj]

@mikeschroder @zieladam This looks good to me.

One thing that's not clear to me is whether this will need to be backported to the 5.8 branch, or if this only affects Gutenberg when running WordPress trunk.

My assumption is that because the tests started failing during the 5.8 cycle, it would be beneficial to backport this to the 5.8 branch, but the priority is not clear. It seems like waiting until 5.8 is released and including it for 5.8.1 would be alright.

[kirasong]

This is my assumption as well.

@kevin940726 as you created the issue on the Gutenberg repo, do you have any preferences?

[kevin940726]

AFAIK, the issue only affects e2e tests in gutenberg, and we do have a workaround (by simply disabling the test plugin), so no rush for it to be included in 5.8. #53542 should probably has higher priority than this since it affects end users as well.

[kirasong]

In 51389:

Customizer: Skip animations when they have no duration.

In addition to skipping animations when a related style doesn't Exist, now checks to see if animation styles are Empty as well.

This resolves a case where the Gutenberg End to End tests were failing, due to running with animations disabled.

This change should also help some users who are intentionally overriding styles to remove animations.

See ​https://github.com/WordPress/gutenberg/issues/32024 for the original Gutenberg issue.

Props zieladam, isabel_brison, kevin940726, desrosj, mikeschroder.
Fixes #53562.
See #53542.

[kirasong]

Reopening for 5.8.1 consideration.

I don't think fixed-major is exactly right, but the closest keyword I could think of.

Please do feel free to modify it if there's a better one.

[SergeyBiryukov]

I think fixed-major is exactly right for something that's in trunk and is also being considered for the next minor release :)

[kevin940726]

I think this fix causes a regression that the focus is now lost when expanding/collapsing sections, while it should default to focusing on the back button (the first element). This is also the reason that e2e tests in gutenberg is now constantly failing.

I'm not sure I understand the code either. el is just a dummy <div> created for obtaining the normalized transition name. I don't think this is the correct place to fix it?

I mentioned in the gutenberg issue that I think the fix should fall along the lines here: ​https://github.com/WordPress/wordpress-develop/blob/bdf7dd6299e4453f9d7438de0a0e138d950ec0bf/src/js/_enqueues/wp/customize/controls.js#L1294-L1305. It should fire completeCallback regardless of if there's a transitionend event or not. Or, we could just go with #53542 and drop this :)

[kirasong]

Replying to kevin940726:

I think this fix causes a regression that the focus is now lost when expanding/collapsing sections, while it should default to focusing on the back button (the first element). This is also the reason that e2e tests in gutenberg is now constantly failing.

Will check into this along with @mamaduka.

I'm not sure I understand the code either. el is just a dummy <div> created for obtaining the normalized transition name. I don't think this is the correct place to fix it?

I mentioned in the gutenberg issue that I think the fix should fall along the lines here: ​https://github.com/WordPress/wordpress-develop/blob/bdf7dd6299e4453f9d7438de0a0e138d950ec0bf/src/js/_enqueues/wp/customize/controls.js#L1294-L1305. It should fire completeCallback regardless of if there's a transitionend event or not. Or, we could just go with #53542 and drop this :)

I made the change in the current location because it seems like it is where detection of whether transitions are supported or not is happening. Of course, if this is causing side effects, let's fix it elsewhere (or in addition).

I think if we fix this, assuming I understand properly, then it still helps with an edge case for users that have CSS overrides, but agreed that #53542 would be great. I don't think I have the requisite core CSS knowledge to handle that one appropriately -- I'm hoping that someone who does will be able to help. Regardless, I agree it'd be great to have it fixed, and if we can use that method in the tests instead, that's excellent.

Edit: A quick note that I think if this causes things to break, it probably means that things are broken when transitions aren't supported in the general case.

[kirasong]

Okay, after digging into this, I think some of my assumptions about normalizedTransitionendEventName were off.

Apologies everyone.

This needs either a revert or a fix forward.
As I'd prefer the latter, but it's late at this point, going to look into it further tomorrow.

[kirasong]

In 51417:

Customizer: Don’t always set normalizedTransitionendEventName to null.

Reverts [51389].

Unprops mikeschroder.
See #53562.

[kirasong]

Went ahead and reverted this to fix the focus issues, and because I haven't found the right fix yet.

Looked into this a bit with @makaduka today.

Essentially, the previous commit [51389] was always setting normalizedTransitionendEventName to null, which while an incorrect fix, had the effect of firing completeCallback regardless of whether there was a transitionend event or not.

However, when completeCallback is called ​at that location, the final focus of the back button doesn't happen.

While not common, I believe this also means that when transitions aren't supported by the browser, users would run into the same bug with focus.

I experimented with changing where this happens, but haven't yet been able to find a way to fire completeCallback in a location where the rest of the process happens as expected. Any help is is appreciated! (cc: @kevin940726 @westonruter)

@mamaduka pointed out that transitionend would also ​not be called if transitions are disabled via prefers-reduced-motion (#53542).

I think this means that if we want to remove the transitions entirely (rather than make them very short, for instance), we'd need to adjust this Customizer JS to support it, regardless of the method that removes the animations.

[kevin940726]

I believe we can just add a condition in places like ​L1268 with something like isReducedMotion.

// Return if CSS transitions are not supported.
if ( ! normalizedTransitionendEventName || isReducedMotion ) {
	if ( completeCallback ) {
		completeCallback();
	}
	return;
}

We can obtain isReducedMotion by running window.matchMedia('(prefers-reduced-motion: reduce)').matches.

As for browsers that don't support transitionend, we no longer support those legacy browsers either, so I don't think we need to handle that.

The focus loss is still a valid problem though, and I agree we have to fix that in the Customizer somehow before we fix this.

[kirasong]

Yes, thank you! This is exactly what I was thinking as well.

I hope we can figure out how to solve the focus loss 🤔

[kevin940726]

I submitted a patch in #53542. I think we can close this in favor of that.

Gutenberg has also recently ​enabled reduce-motion in e2e tests. Hence, I think the fix in #53542 should do the job :)