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.

Sign up for GitHub

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

Jump to bottom

a11y: Add missing reduce-motion mixin #68282

Open
t-hamano opened this issue Dec 25, 2024 · 6 comments · May be fixed by #68284
Open

a11y: Add missing reduce-motion mixin #68282

t-hamano opened this issue Dec 25, 2024 · 6 comments · May be fixed by #68284
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@t-hamano
Copy link
Contributor

What problem does this address?

If we use transition or animation CSS, we are expected to use the reduce-motion mixin. Example:

.selector {
	transition: all 400ms linear;
	@include reduce-motion("transition");
}

This code compiles to the following to respect the user's preference and disable animations:

.selector {
	transition: all 400ms linear;
}
@media (prefers-reduced-motion:reduce){
  .selector {
    transition-delay:0s;
    transition-duration:0s;
  }
}

However, there are many places where transition or animation is used without this mixin.

What is your proposed solution?

We could update stylelint to enforce this rule, but since this is a custom rule, we would need to add our own plugin. I'm not sure if it would be worth creating a custom plugin to enforce this rule. For now, I think it's best to find the missing mixin and add it.

Steps to get started

  1. Search for transition: or animation: in the .scss files in the Gutenberg project.
  2. Make sure the value is something other than none.
  3. Check to see if the reduce-motion mixin exists. If it doesn't exist, we may need one.
  4. Identify the component that has that CSS applied.
  5. In your Chrome browser, change the "Emulate CSS media feature prefers-reduced-motion" setting to reduce.
    https://developer.chrome.com/docs/devtools/rendering/emulate-css#emulate_css_media_feature_prefers-reduced-motion
  6. Do something to that component that will cause a transition or animation (such as hover or focus).
  7. If transition or animation occurs, it means the user's animation preference is not being respected, so add the mixin.
@t-hamano t-hamano added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement. [Type] Task Issues or PRs that have been broken down into an individual action to take Good First Issue An issue that's suitable for someone looking to contribute for the first time and removed [Type] Enhancement A suggestion for improvement. labels Dec 25, 2024
@himanshupathak95 himanshupathak95 linked a pull request Dec 25, 2024 that will close this issue
Draft
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Dec 25, 2024
@himanshupathak95
Copy link
Contributor

Hey @t-hamano, Thanks for raising this.

I will raise a PR to fix this.

@t-hamano
Copy link
Contributor Author

@himanshupathak95 Thanks for addressing this issue.

If you don't mind, could you provide some screenshots of which components each of the reduce-motion mixins affects so we can make sure they work as expected?

Also, you don't need to tackle them all at once. If there are too many to tackle, you can tackle them package by package.

@himanshupathak95
Copy link
Contributor

himanshupathak95 commented Dec 25, 2024 •
edited
Loading

@t-hamano, I am thinking of doing this package by package since there are a lot of places that need to change and there must be distinction amongst them to test and discuss each package correctly in isolation.

I am thinking maybe we can create separate PRs to track packages specifically and execute the changes so we don't break anything. Please let me know what you think.

@t-hamano
Copy link
Contributor Author

Yes, I think it's a good idea to submit a PR for each package 👍

@mirka
Copy link
Member

mirka commented Dec 25, 2024

No strong opinion, but I wanted to note that in @wordpress/components we decided to adopt this standard pattern instead of a mixin:

@media not ( prefers-reduced-motion ) {
	transition: all 400ms linear;
}

It isn't any less ergonomic, it's immediately understandable without knowledge of a custom mixin implementation, and we end up with fewer lines of CSS. Would it be worth considering this pattern for the wider codebase as well?

@t-hamano
Copy link
Contributor Author

@mirka Thanks for the feedback!

Would it be worth considering this pattern for the wider codebase as well?

That's a good idea. I think it might be better to proceed in the following order:

  1. If the reduce-motion mixin itself is missing, wrap the transition or animation in the media query.
  2. Replace where the reduce-motion mixin is already used with the media query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

a11y: Add missing reduce-motion mixin himanshupathak95/gutenberg
3 participants
@mirka @t-hamano @himanshupathak95