Skip to content

Remove animations from various places for users with prefer-reduced-motion#3972

Merged
canova merged 1 commit into
firefox-devtools:mainfrom
canova:prefer-reduced-motion-animations
Apr 5, 2022
Merged

Remove animations from various places for users with prefer-reduced-motion#3972
canova merged 1 commit into
firefox-devtools:mainfrom
canova:prefer-reduced-motion-animations

Conversation

@canova

@canova canova commented Apr 4, 2022

Copy link
Copy Markdown
Member

Fixes #3969 and some more animations we have.
This PR removes the animation from a few places for users with prefer-reduced-motion:

  1. Home view slide-in animation for texts and images during page load.
  2. Stripes animation of the top loading bar while publishing a profile.
  3. Loading animation with the colorful boxes (while either fetching a profile from Firefox or while downloading an profile)
  4. Profile viewer animation that happens while loading a profile or publishing a profile.
  5. Filter navigator bar item animations while adding or removing a new item.

Deploy preview / Production
You can activate the prefer-reduced-motion by following the steps here: #3969 (comment)

@canova canova requested a review from julienw April 4, 2022 11:54
@codecov

codecov Bot commented Apr 4, 2022

Copy link
Copy Markdown

Codecov Report

Merging #3972 (a8b4b68) into main (bb24b1a) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head a8b4b68 differs from pull request most recent head f32ec59. Consider uploading reports for the commit f32ec59 to get more accurate results

@@           Coverage Diff           @@
##             main    #3972   +/-   ##
=======================================
  Coverage   87.26%   87.26%           
=======================================
  Files         279      279           
  Lines       23934    23924   -10     
  Branches     6325     6317    -8     
=======================================
- Hits        20885    20877    -8     
+ Misses       2809     2807    -2     
  Partials      240      240           
Impacted Files Coverage Δ
src/profile-logic/tracks.js 83.40% <0.00%> (+0.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb24b1a...f32ec59. Read the comment docs.

@julienw julienw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!
I looked at the code and tried the deploy preview, and I think most "animation" uses are now removed. Except maybe one:

  • the SVG icon when uploading res/img/svg/sharing-animated-dark-12.svg. But maybe that's OK because it's fairly small.

However I can see a lot of transition uses. Some of them are not problematic because they're small, but I can see a few that would be good to be removed:

  • in the menu buttons, when the panel is dismissed
  • the component showing up when doing drag and drop from a file
  • The transition when reordering tracks
  • The introduction on the search input
  • Transitions in the active tab view, when opening tracks
  • Possibly the transition when the "+" button appears when doing a selection
  • The help about scrolling in Viewport

I'm totally fine with touching those in other PRs.

/* Do not animate the publish animation for stripes at the top loading bar. */
@media (prefers-reduced-motion) {
.menuButtonsPublishUploadBarInner {
animation: none;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Optional: in general I think I'd rather have @media (prefers-reduced-motion: no-preference) that adds the animation instead of @media (prefers-reduced-motion) { removing it. I think it makes it easier to maintain in the longer term, reducing the risk of typos.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When I was adding them, I wanted to follow this logic:

  1. If a CSS class is added for solely that animation/transition, then wrap that class with @media (prefers-reduced-motion: no-preference)
  2. If a CSS class is not added for that animation and includes other properties, I added @media (prefers-reduced-motion) to reset the animations.

I think 2nd item makes it cleaner, but happy to discuss for a follow-up if you want.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can use the same class both in @media and outside @media, I think it's OK to specify the same class twice (actually you're doing it with the solution 2 as well!), and on the long run it will be easier to maintain.
For example, if we change the class name in the future, it's easy to forget to update the "negative" @media query (that is, one that removes the animation) because most of us don't use the app with prefers-reduced-motion on. However with the "positive" @media query it's easier to see that the animation is missing when not using prefers-reduced-motion.

Here is my rationale :-)

Comment thread src/components/app/Home.css

@canova canova left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the review!

Thanks! I looked at the code and tried the deploy preview, and I think most "animation" uses are now removed. Except maybe one:

  • the SVG icon when uploading res/img/svg/sharing-animated-dark-12.svg. But maybe that's OK because it's fairly small.

I'm not sure about it. Indeed, it could be too small and not that disturbing, but I think it's nice to double check with the original reporter since I'm not really sure.

However I can see a lot of transition uses. Some of them are not problematic because they're small, but I can see a few that would be good to be removed:

  • in the menu buttons, when the panel is dismissed

  • the component showing up when doing drag and drop from a file

  • The transition when reordering tracks

  • The introduction on the search input

  • Transitions in the active tab view, when opening tracks

  • Possibly the transition when the "+" button appears when doing a selection

  • The help about scrolling in Viewport

I'm totally fine with touching those in other PRs.

From what original reporter expressed, I got the impression that simple transitions with fade-in/fade-out effects might not be a problem. I think it's mostly related to elements moving around and when they do not scroll with the other part of the page. But I'm using a lot of "maybe"s in these sentences :) It would be definitely good to check with the person who is having this issue. I will file a seperate issue for the things you listed so we can check.
Filed #3973 for that.

/* Do not animate the publish animation for stripes at the top loading bar. */
@media (prefers-reduced-motion) {
.menuButtonsPublishUploadBarInner {
animation: none;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When I was adding them, I wanted to follow this logic:

  1. If a CSS class is added for solely that animation/transition, then wrap that class with @media (prefers-reduced-motion: no-preference)
  2. If a CSS class is not added for that animation and includes other properties, I added @media (prefers-reduced-motion) to reset the animations.

I think 2nd item makes it cleaner, but happy to discuss for a follow-up if you want.

@canova canova force-pushed the prefer-reduced-motion-animations branch from a8b4b68 to f32ec59 Compare April 5, 2022 14:08
@canova canova merged commit 0f064c7 into firefox-devtools:main Apr 5, 2022
@canova canova deleted the prefer-reduced-motion-animations branch April 5, 2022 14:12
@canova canova mentioned this pull request Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

https://profiler.firefox.com/ front-page should disable its animations (on front page and loading screens) for users with prefer-reduced-motion

2 participants