Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Widgets Customizer should work without animations and transitions enabled #32024

Closed
kevin940726 opened this issue May 20, 2021 · 13 comments
Closed
Labels
[Feature] Widgets Customizer Ability to add and edit blocks in Customize → Widgets. [Type] Bug An existing feature does not function as intended

Comments

@kevin940726
Copy link
Member

Description

Possibly related failing tests: https://github.com/WordPress/gutenberg/runs/2620274056

Currently, the Widgets Customzier only works with "Disable the CSS animations" plugin disabled. It's because the customizer API rely on transitionend to work. We might want to find a new way to workaround that until we fixes it in core.

Step-by-step reproduction instructions

  1. Enable "Disable the CSS animations" test plugin (It's easier to enable it via wp-env in the test environment)
  2. Go to Appearance -> Customize -> Widgets

Expected behaviour

The editor should load.

Actual behaviour

The editor doesn't load.

Screenshots or screen recording (optional)

image

WordPress information

  • WordPress version: trunk
  • Gutenberg version: trunk
  • Are all plugins except Gutenberg deactivated? Disable the CSS animations plugin should be activated
  • Are you using a default theme (e.g. Twenty Twenty-One)? Yes

Device information

  • Device: Desktop
  • Operating system: macOS 11.3.1
  • Browser: Brave 1.23.71 (Chromium 90)
@kevin940726 kevin940726 added [Type] Bug An existing feature does not function as intended [Feature] Widgets Customizer Ability to add and edit blocks in Customize → Widgets. labels May 20, 2021
@kevin940726 kevin940726 changed the title Widgets Customizer should work without animations and transitions enbled. Widgets Customizer should work without animations and transitions enabled May 20, 2021
@adamziel
Copy link
Contributor

The bug is caused by the following rule: transition-duration: 0s !important, presumably due to some race condition with transitionend event involved. If I change it to transition-duration: 0.01s !important then everything suddenly works. I am going to try to identify the problematic part of the JS code.

@adamziel
Copy link
Contributor

adamziel commented Jun 22, 2021

This subscriber isn't getting called with transition-duration: 0s:

https://github.com/WordPress/gutenberg/blob/8667e9688867004755dce39225f185baf9558f3d/packages/customize-widgets/src/components/customize-widgets/index.js#L30-35

I don't know the customizer code very well so I'll leave it at that. We could go drill down all the way to the root cause or just go for the 0.01s workaround. The latter should be fine considering this is just a test plugin, what do you think?

@kevin940726
Copy link
Member Author

This is actually a bug in core, so I'm not sure why I didn't post it there 😅.

Yes, we can solve it by patching the test plugin, but I'm afraid that the plugin exists for a reason? I wonder if it's an accessibility issue if we don't support it. Maybe some browsers or extensions will automatically register similar override as well? @tellthemachines might have some insights that I'm not aware of 🙇‍♂️.

As for the root cause is along these lines:

https://github.com/WordPress/wordpress-develop/blob/bdf7dd6299e4453f9d7438de0a0e138d950ec0bf/src/js/_enqueues/wp/customize/controls.js#L1294-L1305

The completeCallback, which the Widgets customizer depends on, never fires if there's no transitionend event. However, we could probably fix it here though if that's easier.

@tellthemachines
Copy link
Contributor

I wonder if it's an accessibility issue if we don't support it.

We should definitely support animations being turned off! Some users have them disabled at a system level for a11y reasons.

I'm not sure why we're using that plugin for the e2e tests though. Might be a matter of speed? Not having to wait for animations to finish?

@getsource
Copy link
Member

I did a bit of looking into this.

I’m not sure it’s ideal, but this looks to fix the problem referenced in the description:

diff --git wp-admin/js/customize-controls.js wp-admin/js/customize-controls.js
index c8ee3de8..e5d07929 100644
--- wp-admin/js/customize-controls.js
+++ wp-admin/js/customize-controls.js
@@ -906,7 +906,7 @@
 			'WebkitTransition': 'webkitTransitionEnd'
 		};
 		prop = _.find( _.keys( transitions ), function( prop ) {
-			return ! _.isUndefined( el.style[ prop ] );
+			return ! _.isUndefined( el.style[ prop ] ) && ! _.isEmpty( el.style[ prop ] );
 		} );
 		if ( prop ) {
 			return transitions[ prop ];

Some background:

I believe the plugin folks are talking about is this one, from the Gutenberg end to end tests. As mentioned by others, it disables animations by setting the times/delay to 0ms.

This does, in fact, currently break the Widgets Editor in the Customizer:

Disable.Motion.Plugin.Enabled.mov

After the applying the diff above, it looks like this:

Disable.Motion.Plugin.Enabled.After.Fix.mov

As far as I can tell, the plugin’s behavior is different from the behavior of turning on “Reduce Motion” in OSs/Browsers through the prefers-reduced-motion CSS media feature, which has some documentation here.

While prefers-reduced-motion is supported in some areas of WordPress, it does not appear to be honored with this part of the Customizer.

When I enable Reduce Motion in OSX, and test in Firefox, I see the same animation.
Reduce Motion disabled:

No.Reduce.Motion.mov

Reduce Motion enabled:

Reduce.Motion.mov

A little good news, as far as the problem described in the ticket, is that this means that the Widgets Editor in the Customizer looks to load with Reduce Motion in the OS enabled.

The animation still happening, even with Reduce Motion, looks like a separate bug needs to be fixed, though. I did a bit of looking and didn’t find an existing issue, so thinking it would be a good idea to open a separate ticket in trac for it.

Hopefully this helps!

@getsource
Copy link
Member

getsource commented Jun 28, 2021

I made a Core trac ticket for Reduce Motion for the Customizer sliding transitions mentioned, here:
https://core.trac.wordpress.org/ticket/53542

@adamziel
Copy link
Contributor

@getsource great job getting to the bottom of that!

I’m not sure it’s ideal, but this looks to fix the problem referenced in the description:

Would you please create a new diff with that fix so that we can mark this issue as closed? :-)

@getsource
Copy link
Member

Sure! I'll get a core trac ticket created with a patch.

I'll link to it here as soon as it's there. This might be tomorrow for me at this point, as I've had a few issues with my core dev environment during the process of creation + testing.

@getsource
Copy link
Member

Ticket with patch created here:
https://core.trac.wordpress.org/ticket/53562

pento pushed a commit to WordPress/wordpress-develop that referenced this issue Jul 9, 2021
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 WordPress/gutenberg#32024 for the original Gutenberg issue.

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

git-svn-id: https://develop.svn.wordpress.org/trunk@51389 602fd350-edb4-49c9-b593-d223f7449a82
nylen pushed a commit to nylen/wordpress-develop-svn that referenced this issue Jul 9, 2021
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 WordPress/gutenberg#32024 for the original Gutenberg issue.

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

git-svn-id: https://develop.svn.wordpress.org/trunk@51389 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this issue Jul 9, 2021
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 WordPress/gutenberg#32024 for the original Gutenberg issue.

Props zieladam, isabel_brison, kevin940726, desrosj, mikeschroder.
Fixes #53562.
See #53542.
Built from https://develop.svn.wordpress.org/trunk@51389


git-svn-id: http://core.svn.wordpress.org/trunk@51000 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this issue Jul 9, 2021
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 WordPress/gutenberg#32024 for the original Gutenberg issue.

Props zieladam, isabel_brison, kevin940726, desrosj, mikeschroder.
Fixes #53562.
See #53542.
Built from https://develop.svn.wordpress.org/trunk@51389


git-svn-id: https://core.svn.wordpress.org/trunk@51000 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@kevin940726
Copy link
Member Author

I opened a PR in core which should solve this issue: WordPress/wordpress-develop#1608.

Once it's merged, we only need to tidy up some code in the tests :).

@getsource
Copy link
Member

Thank you, this looks great! I'll take a look, test, and leave a comment on the core tickets as well today.

nylen pushed a commit to nylen/wordpress-develop-svn that referenced this issue Aug 27, 2021
…r animations.

Disables Customizer animations when media query `prefers-reduced-motion: reduce` returns true.

Continues the effort to reduce motion within the WordPress admin panel when users have enabled this feature in their operating system or browser.

It has the additional effect of making the Block Editor's end-to-end tests run faster, as explored initially with a different approach in #53562.

See WordPress/gutenberg#32024 for the related Gutenberg issue.

See https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-reduced-motion for ways to enable this feature on various platforms.

Props kevin940726, isabel_brison, zieladam, youknowriad, desrosj, mamaduka, mikeschroder.
Previously [51389], [50028], [47813].
Fixes #53542.

git-svn-id: https://develop.svn.wordpress.org/trunk@51677 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this issue Aug 27, 2021
…r animations.

Disables Customizer animations when media query `prefers-reduced-motion: reduce` returns true.

Continues the effort to reduce motion within the WordPress admin panel when users have enabled this feature in their operating system or browser.

It has the additional effect of making the Block Editor's end-to-end tests run faster, as explored initially with a different approach in #53562.

See WordPress/gutenberg#32024 for the related Gutenberg issue.

See https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-reduced-motion for ways to enable this feature on various platforms.

Props kevin940726, isabel_brison, zieladam, youknowriad, desrosj, mamaduka, mikeschroder.
Previously [51389], [50028], [47813].
Fixes #53542.
Built from https://develop.svn.wordpress.org/trunk@51677


git-svn-id: http://core.svn.wordpress.org/trunk@51283 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this issue Aug 27, 2021
…r animations.

Disables Customizer animations when media query `prefers-reduced-motion: reduce` returns true.

Continues the effort to reduce motion within the WordPress admin panel when users have enabled this feature in their operating system or browser.

It has the additional effect of making the Block Editor's end-to-end tests run faster, as explored initially with a different approach in #53562.

See WordPress/gutenberg#32024 for the related Gutenberg issue.

See https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-reduced-motion for ways to enable this feature on various platforms.

Props kevin940726, isabel_brison, zieladam, youknowriad, desrosj, mamaduka, mikeschroder.
Previously [51389], [50028], [47813].
Fixes #53542.
Built from https://develop.svn.wordpress.org/trunk@51677


git-svn-id: https://core.svn.wordpress.org/trunk@51283 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@getsource
Copy link
Member

Okay, thanks again everyone!
I just made a follow-up commit, here: https://core.trac.wordpress.org/changeset/51677

Please let me know if anything else is necessary here.

@noisysocks
Copy link
Member

Closing this as I see that https://core.trac.wordpress.org/ticket/53542 is now closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widgets Customizer Ability to add and edit blocks in Customize → Widgets. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

5 participants