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

Edit Site: Standardize reduced motion handling using media queries #68419

Conversation

himanshupathak95
Copy link
Contributor

@himanshupathak95 himanshupathak95 commented Dec 31, 2024

Part of: #68282

What?

Replace the reduce-motion mixin with @media not (prefers-reduced-motion) media queries across all components in the edit-site package. This standardizes how we handle reduced motion preferences throughout the codebase.

Testing Instructions

  • Enable the Site Editor

  • Test each component that has animations:

    • Open/close the editor interface
    • Navigate between different pages/templates
    • Open the font library modal
    • Hover over global style variations
    • Use the site hub navigation
  • Verify animations work normally

  • In your system preferences, enable reduced motion:

    • Chrome DevTools: Open DevTools > More Tools > Rendering > "Emulate CSS media feature prefers-reduced-motion" > select "reduce"
  • Verify that animations are disabled when reduced motion is enabled

Screencast

Screen.Recording.2025-01-02.at.07.57.14.mov
Screen.Recording.2025-01-02.at.08.16.30.mov

@himanshupathak95 himanshupathak95 marked this pull request as ready for review January 2, 2025 02:47
Copy link

github-actions bot commented Jan 2, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: himanshupathak95 <[email protected]>
Co-authored-by: t-hamano <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@himanshupathak95
Copy link
Contributor Author

Hey @t-hamano, Please review these changes whenever you get a moment and let me know if the changes are as expected.

Thanks 😇

@t-hamano t-hamano added [Type] Code Quality Issues or PRs that relate to code quality [Package] Edit Site /packages/edit-site labels Jan 2, 2025
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! It's working correctly in every component.

@t-hamano t-hamano added the props-bot Adding this label triggers the Props Bot workflow for a PR. label Jan 2, 2025
@github-actions github-actions bot removed the props-bot Adding this label triggers the Props Bot workflow for a PR. label Jan 2, 2025
@t-hamano t-hamano merged commit 93f8928 into WordPress:trunk Jan 2, 2025
75 of 77 checks passed
@github-actions github-actions bot added this to the Gutenberg 20.0 milestone Jan 2, 2025
@gziolo
Copy link
Member

gziolo commented Jan 2, 2025

These changes broke Storybook integration on CI:

Screenshot 2025-01-02 at 11 22 52

Can someone look into it?

@im3dabasia
Copy link
Contributor

These changes broke Storybook integration on CI:

Thank you @gziolo

I debugged and found that the issue arises from this place. All other file changes are not causing any issues, but this one is. The preprocessor is converting the SCSS to the following, as shown in the screenshot above. It was incorrectly compiling the not and the &.

Code

@include break-medium {
top: $canvas-padding;
bottom: $canvas-padding;
width: calc(100% - #{$canvas-padding});
.edit-site-resizable-frame__inner-content {
box-shadow: $elevation-x-small;
// This ensure the radius work properly.
overflow: hidden;
@media not (prefers-reduced-motion) {
transition: border-radius, box-shadow 0.4s;
}
.edit-site-layout:not(.is-full-canvas) & {
border-radius: $radius-large;
}
&:hover {
box-shadow: $elevation-large;
}
}
}

The previous compiled code with wrong parenthisis

[1] 2575 │ @media (min-width: 782px) and (not (prefers-reduced-motion)) {

Changing the code to the following solves the issue: Reference

@media (prefers-reduced-motion: no-preference) {
	transition: border-radius, box-shadow 0.4s;
}

New compiled code:

@media (min-width: 782px) and (prefers-reduced-motion: no-preference) {
  .edit-site-layout__canvas .edit-site-resizable-frame__inner-content {
    transition: border-radius, box-shadow 0.4s;
  }
}

@mirka
Copy link
Member

mirka commented Jan 2, 2025

@im3dabasia Thanks for looking into it! I think we can go with that hotfix for now.

Though, we'll want some additional measures to prevent this from happening again:

  • I will prepare a new CI check to ensure that Storybook-breaking PRs don't get merged into trunk (Check Storybook build on CI for PRs #68466).
  • The compiled CSS @media (min-width: 782px) and (not (prefers-reduced-motion)) seems to be valid, according to sources like the W3C CSS Validator and MDN. Did you figure out what module is emitting the error? It could be that the module is outdated, or has a bug.

cc @WordPress/gutenberg-components for awareness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Edit Site /packages/edit-site [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants