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

Base styles: Improve the reduce-motion mixin #55566

Open
afercia opened this issue Oct 24, 2023 · 2 comments
Open

Base styles: Improve the reduce-motion mixin #55566

afercia opened this issue Oct 24, 2023 · 2 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Base styles /packages/base-styles [Type] Enhancement A suggestion for improvement.

Comments

@afercia
Copy link
Contributor

afercia commented Oct 24, 2023

Description

Splitting this out from #55547

Ideally, the reduce-motion scss mixin should be able to reset any CSS transition / animation. However, this is not the case. Thdre are a few occurrences in the codebase where specific, ad-hoc, media query prefers-reduced-motion are in place because the mixin doesn't cover all the cases to properly reset a transition or animation.

Ideally, these specific ad-hoc media query should be avoided because they are redundant code, hard to maintain, and pronoe to bugs. A unique, centralized, mixin that works for all cases is certainly better.

The specific case in #55547 didn't allow to use the reduce-motion mixin because the animation for the 'saving' buttons state uses the CSS property animation-iteration-count with a value of infinite. This property is not reset by the reduce-motion mixin.

@mixin reduce-motion($property: "") {
@if $property == "transition" {
@media (prefers-reduced-motion: reduce) {
transition-duration: 0s;
transition-delay: 0s;
}
} @else if $property == "animation" {
@media (prefers-reduced-motion: reduce) {
animation-duration: 1ms;
animation-delay: 0s;
}
} @else {
@media (prefers-reduced-motion: reduce) {
transition-duration: 0s;
transition-delay: 0s;
animation-duration: 1ms;
animation-delay: 0s;
}
}
}

Notice the mixin sets animation-duration to 1ms instead of 0. This is because of this reason, which dates to more than 3 years ago. I'm not sure whther that concern is still valid. Regardles, the effect of:
animation-duration: 1ms;
on the saving buttons animation would just make the animation very fast because animation-iteration-count is still infinite.

At the very least, the mixing:

  • Should set animation-iteration-count to 0 (or maybe 1).
  • We should check whether animation-duration: 1ms; is still needed. Ideally, this should be set to 0.
  • Any other property used in other animations should be evaluated to check whether it can be reset by the mixin,
  • Any 'ad-hoc' usage of prefers-reduced-motion in the codebase should be replaced by the mixin.
  • An new animation should be crafted since the beginning taking into consideration that it needs to be compatible with the reduce-motion mixin.

Step-by-step reproduction instructions

  • Testing would require altering the code and building, so this is not for everyone.
  • Add @include reduce-motion("animation"); after this line:
    animation: components-button__busy-animation 2500ms infinite linear;
  • If Improve Button saving state accessibility. #55547 has already been merged, replace the ad-hoc media query with @include reduce-motion("animation");
  • Build.
  • Enable 'Reduce motion' in your operating system user preferences.
  • Edit a post.
  • Click the 'Update' primary button.
  • Observe the button animation still runs.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Enhancement A suggestion for improvement. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Base styles /packages/base-styles labels Oct 24, 2023
@afercia
Copy link
Contributor Author

afercia commented Oct 24, 2023

The enqueue_disable_animations_stylesheet E2E 'pugin' should be updated as well.

https://github.com/WordPress/gutenberg/blob/1ee154f51d6b0a743fc7c99ba5959371237bbb0e/packages/e2e-tests/plugins/disable-animations.php

@afercia
Copy link
Contributor Author

afercia commented Oct 24, 2023

Additionally, check any @keyframes animation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Base styles /packages/base-styles [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

1 participant