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

prefers-reduce-motion globally instead of duplicating it? #29862

Closed
ffoodd opened this issue Dec 16, 2019 · 5 comments
Closed

prefers-reduce-motion globally instead of duplicating it? #29862

ffoodd opened this issue Dec 16, 2019 · 5 comments
Labels

Comments

@ffoodd
Copy link
Member

ffoodd commented Dec 16, 2019

Af of v4.4.1 — and since #25249 — the transition() mixins unsets transition when user prefers-reduced-motion (which is great).

However there are at least two caveats with this way of unsetting transitions:

  1. the whole media query is duplicated a lot (each time the mixin is used, 16 times in 4.4.1)
  2. it's only done for this specific mixin, however there a few components using animations or transitions without using this mixin (progress bar, for example).

What would you think about reducing motion globally and drastically?

My main resource for this is Andy Bell's "Modern CSS reset" but there're a lot more resources out there.

Coupling with $enable-prefers-reduced-motion-media-query, it might look like this:

@if $enable-prefers-reduced-motion-media-query {
  @media (prefers-reduced-motion: reduce) {
    * {
      transition-duration: .01ms !important;
      animation-duration: .01ms !important;
      animation-iteration-count: 1 !important;
    }
  }
}

And it should be shorter and more efficient than 16 times of scoped transition: none. I guess it could be either in _transitions.scss or in _reboot.scss; both of them would make sense to me.

I don't know if / on which version it could be done, but I'd be pleased to make a PR if it does interest you.

@MartijnCuppens
Copy link
Member

See #27986 (comment):

This will override all transitions everywhere and that could lead to unexpected behaviour on custom code or css plugins. The animation is something we can't do because the spinners would freeze.

Another option is to use a PostCSS plugin to combine these rules, but AFAIK such plugin does not exist yet.

@ffoodd
Copy link
Member Author

ffoodd commented Dec 17, 2019

Makes sense, however to me the spinner is the only component whose animation is significant. And it could easily be fixed by displaying visually hidden text when user prefers-reduce-motion, specifically for the spinner.

So if applying this globally is not an option, there's another one I'd want to suggest for the redundancy: could we use a placeholder (extended in the transition mixins) to group transition resets?

Something like:

// In _reboot.scss, probably
@if $enable-prefers-reduced-motion-media-query {
  @media (prefers-reduced-motion: reduce) {
     %no-transition {
        transition: none !important;
     }
  }
}

// In scss/mixins/_transition.scss
@mixin transition($transition...) {
  @if $enable-transitions {
    @if length($transition) == 0 {
      transition: $transition-base;
    } @else {
      transition: $transition;
    }
  }

  @if $enable-prefers-reduced-motion-media-query {
    @extend %no-transition;
  }
}

Do you think it could solve the redundancy thing?

@MartijnCuppens
Copy link
Member

That could work! Feel free to test this and make a PR if you can get this to work.

@patrickhlauke
Copy link
Member

i have no strong opinion here, but mention that having a single high-level animation suppression assumes that users that set prefers-reduced-motion don't want ANY animation, while this is not necessarily the case (only certain types of animations are particularly problematic/disruptive for users). the current approach allows for some nuance.

also, this general big hammer on animations will "bleed out" to any author-defined extra animations that they may have set up in their additional CSS.

MartijnCuppens added a commit that referenced this issue Feb 15, 2020
…29862"

This reverts commit cd7e5d1.

Co-authored-by: Martijn Cuppens <martijn.cuppens@gmail.com>
@MartijnCuppens
Copy link
Member

Since in our test results, the @extend technique seemed to increase the bundle size, I'm gonna close this.

olsza pushed a commit to olsza/bootstrap that referenced this issue Oct 3, 2020
…wbs#29862"

This reverts commit cd7e5d1.

Co-authored-by: Martijn Cuppens <martijn.cuppens@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants