-
Notifications
You must be signed in to change notification settings - Fork 1k
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
USWDS - Core: Add transition utility with reduced motion preference handling #6268
base: develop
Are you sure you want to change the base?
Conversation
This addresses all
A couple ideas:
|
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.
@aduth thanks for submitting this! I've tested with both Reduce motion
settings and everything looks good. Minor suggestion for SASSDoc mixin.
I personally, prefer reducing motion.
I also want to make sure I'm understanding the second point in this comment correctly.
Do you mean inline prefers-reduced-motion
strictly for this single animation? If so, it seems fair to add it inline because:
- We're not using many animations and it seems unnecessary to create a mixin for this
- The animation itself is only found in this file
- We already have several other media queries in
usa-nav
, so wouldn't look out of place
packages/uswds-core/src/styles/mixins/utilities/_transition.scss
Outdated
Show resolved
Hide resolved
f374089
to
6ca2a6f
Compare
Yep, that's what I meant. I agree that -- at least in the short term -- adding the media query to the existing style is the most direct option. Maybe an extra mixin could be added in the future. Updated in 6ca2a6f. |
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.
This is great! This will benefit users and developers alike. Thanks @aduth
Left a couple comments for additional considerations but nothing blocking
@@ -42,6 +42,7 @@ | |||
@forward "text-indent"; | |||
@forward "text"; | |||
@forward "top"; | |||
@forward "transition"; |
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.
thought: We should consider adding guidance to our Utilities pages to document this new utility for users.
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.
Just to clarify, this would be a separate pull request in the uswds-site
repository, correct?
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.
@mahoneycm I can see that, but transitions in USWDS are new and aren't used often. There's not even a dedicated page on the website, so this might be a bigger level of effort than we expect.
I would wait until we a strategy for USWDS animations and transitions as a whole.
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.
/// @include u-transition('top'); | ||
/// @include u-transition('opacity', 'ease-in-out'); | ||
/// @include u-transition('all', cubic-bezier(0.17, 0.67, 0.83, 0.67), true); | ||
@mixin u-transition($prop, $easing: $project-easing, $essential: false) { |
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.
Praise: $essential
is a nice touch! Matches w3c guidance and allows this utility to be very resilient. Thanks!
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.
Thanks for the quick updates!
Summary
Animated transitions now respect the system's reduced motion preference. A new transition utility handles default behavior for easing and disabling inessential animation.
Breaking change
This is not a breaking change.
Related issue
Closes #5256
Preview link
In local Storybook, can be previewed at:
Problem statement
As a user who may be distracted or experience harm such as nausea due to animated content, I expect that the U.S. Web Design System will respect my preference to experience reduced motion where possible.
Related:
Solution
Adds a new utility
u-transition
with smart defaults to handle:$essential
argument (defaultfalse
) to control whether transition is disabled when reduced motion is preferredRelated:
Testing and review
See "Preview links" above.
Unless system preferences are configured to prefer reduced motion, there should be no changes in the appearance of animated transition effects.
Instructions for configuring reduced motion preference:
Before opening this PR, make sure you’ve done whichever of these applies to you:
git pull origin [base branch]
to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch isdevelop
).npm run prettier:sass
to format any Sass updates.npm test
and confirm that all tests pass.