-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: #4298 respect prefers-reduced-motion for loading indicators #4299
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: master
Are you sure you want to change the base?
Conversation
|
Please review the PR when convenient. I’m happy to make any changes if needed. |
| @media (prefers-reduced-motion: no-preference) { | ||
| transform: translateY(-50%) rotate(225deg); | ||
| } | ||
| @media (prefers-reduced-motion: reduce) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this useless?
Where is the transition that this line would undo?
| @media (prefers-reduced-motion: no-preference) { | ||
| transform: translateY(-50%) rotate(225deg); | ||
| } | ||
| @media (prefers-reduced-motion: reduce) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this useless?
Where is the transition that this line would undo?
| padding 0.2s ease-out, | ||
| background-color 0.2s ease-out; | ||
| } | ||
| @media (prefers-reduced-motion: reduce) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this useless?
Where is the transition that this line would undo?
| height: auto; | ||
| } | ||
| } | ||
| @media (prefers-reduced-motion: reduce) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this useless?
Where is the transition that this line would undo?
It looks to me that all these changes are bs
| box-shadow: oklch(0% 0 0/ 0.25) 0px 25px 50px -12px; | ||
| overflow-y: auto; | ||
| overscroll-behavior: contain; | ||
| @media (prefers-reduced-motion: reduce) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to use the general pattern of only applying the motion transition when prefers-reduced-motion: no-preference. The non-motion transition should be applied always.
| mask-repeat: no-repeat; | ||
| mask-position: center; | ||
| mask-image: url("data:image/svg+xml,%3Csvg width='24' height='24' stroke='black' viewBox='0 0 24 24' xmlns='http://www.w3.org/2000/svg'%3E%3Cg transform-origin='center'%3E%3Ccircle cx='12' cy='12' r='9.5' fill='none' stroke-width='3' stroke-linecap='round'%3E%3CanimateTransform attributeName='transform' type='rotate' from='0 12 12' to='360 12 12' dur='2s' repeatCount='indefinite'/%3E%3Canimate attributeName='stroke-dasharray' values='0,150;42,150;42,150' keyTimes='0;0.475;1' dur='1.5s' repeatCount='indefinite'/%3E%3Canimate attributeName='stroke-dashoffset' values='0;-16;-59' keyTimes='0;0.475;1' dur='1.5s' repeatCount='indefinite'/%3E%3C/circle%3E%3C/g%3E%3C/svg%3E"); | ||
| @media (prefers-reduced-motion: reduce) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change all these to use the general pattern of only applying the motion transition when prefers-reduced-motion: no-preference.
Set the circle static svg on general .loading class because you use the same image for multiple loaders (which ones?)
Use the animated image-mask if prefers-reduced-motion: no-preference
You can see what you did now here: https://play.tailwindcss.com/cRTFEUk0SK
|
Thanks for the review. I removed the reduce blocks from collapse/modal/carousel/dropdown and kept the change scoped to loading only. loading.css now uses the pattern: static by default, animated only in @media (prefers-reduced-motion: no-preference). |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Please be respectful. If PR is not helpful it will be closed. If it is helpful it will be merged |
I am a real contributor, not an automated tool. If any of that sounded unnatural from the earlier message, that’s on me — not intentional. |
Summary
Changes
Testing
Notes