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

Reduced Motion Animations (for #192) #341

Draft
wants to merge 41 commits into
base: main
Choose a base branch
from

Conversation

saitheninja
Copy link

@saitheninja saitheninja commented Jan 27, 2023

Hi Adam.

I noticed that the old pull request (#194) for this issue (#192) has gotten a little stale so I took a crack at it in a new pull request. I did just enough to preview animations on the doc site in dev. I can fill in the rest of the packaging stuff whenever you need it.

The feedback you got on the previous pull request said that stepped movement felt a little strange, so instead of doing steps I made things move smaller distances, and slower. Spin is the exception. It's movement is stepped, because it makes me kind of sick even when it's slow, and it has to rotate all the way around so the distance is fixed.

I kept the timings the same for full motion and reduced motion one-shot animations so that they can be substituted easily, especially when combining animations. I slowed down the timing for the infinite animations because they still felt too fast to me, even when moving less.

The reduced motion slides don't move as far, which isn't exactly a drop-in replacement, but it can be combined with a fade to get the same effect as sliding from/to offscreen.

@stackblitz
Copy link

stackblitz bot commented Jan 27, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@argyleink
Copy link
Owner

very nice, this seems like a great angle to try out! "reduced" animations truly.

do you have vestibular motion sickness? i'm going to ping a few folks I know to see how it makes them feel. i'll get back to you once they've got back to me 👍🏻

@saitheninja
Copy link
Author

saitheninja commented Jan 29, 2023

do you have vestibular motion sickness?

Yeah, a mild form. I don't do well on boats specifically.

i'm going to ping a few folks I know to see how it makes them feel. i'll get back to you once they've got back to me 👍🏻

Awesome, thank you. I'm definitely interested to learn whether this helps other people with motion sickness.

@argyleink
Copy link
Owner

deploy preview available for testing here https://sensational-dasik-603d9d.netlify.app/#animations

@argyleink
Copy link
Owner

first report is that it's helpful but not bulletproof. the scale animations still triggered them. they suggested it's "reduced" effect is a fade out. pretty good suggestion. i wonder how many we can make fallback to a crossfade?

for readability, and to not conflate with any specific reduced-motion things
@saitheninja
Copy link
Author

sorry about the delay, i was working on some other stuff.

the scale animations still triggered them. they suggested it's "reduced" effect is a fade out.

maybe stepped animations are the way to go after all. i made some new animations that use stepped fades.
imo "scale fade" looks better than "scale less", so we probably don't need "scale less" and "scale less fade", but it's there to compare.

i wonder how many we can make fallback to a crossfade?

i added new animations for: scale, slide, spin, float, bounce, pulse, shake.
ping and blink already use fades.
shake-x and y have too much movement to fit the fades in. it was like being strobed, so i increased the duration.

@argyleink
Copy link
Owner

no worries on speed here, no rush, thanks for comin back and hackin.

there's definitely some cool results in here! the stepped fades are pretty sweet. they have an entry and exit cross fade animation, which we know is g2g, and then it shows the animation in the end state. seems to check a lot of boxes. there's still some finesse needed to get these fully ready, but it feels like progress.

disregarding the kinda akwards starts to some of the stepped crossfades, the spin is working really well. i like the scales too. the steps allow for the element to move somewhere while the cross fade takes the motion out of it and lets the brain kinda fill it in.

i agree the shakes are quite working tho.

i made this little playground to try a more isolated sandbox out, check it out here https://codepen.io/argyleink/pen/wvxVMJg

Video capture of the scale up animation vs the reduced, stepped and crossfaded, scale up:

Kapture.2023-02-10.at.10.23.07.mp4

Using the animations devtools you can replay the transition, see how it works. you can also click anywhere in the demo to replay the animation and see them side by side.

@saitheninja
Copy link
Author

hey Adam. thanks for the pen, that made it easier to mess with. i have probably spent too long playing with it. here's my pen.

shake and bounce are a little strobe-y, and i don't really have any good ideas for that.

@argyleink
Copy link
Owner

shake and bounce are strobe-y yep… but, there's a lot to like in this too I think! thanks for putting this together 🙂 lemme share it around and gather feedback 👍🏻

@argyleink
Copy link
Owner

lots of great feedback so far, this is definitely the most articulate 🙂

-- start quote --

Slides and scales look pretty good.
I am not sure about shake, ping, bounce and spin tho.

My thoughts on them 1 by 1:

Spin
I first wanted to write the following:
I am not sure if the cross fade is needed it feels a bit weird with it. I think a step animation should be enough if it's not too fast. I think the problem there had only been the speed. But that in general is a problem we cannot solve as it is within the hands of the user on how fast the animation is played in the end.

BUT after playing around with your demo for a while and putting content inside of the spining elemnt and as well removing the crossfade again I think this is a solid solution and looks way better than just a step animation if there is real content in place and / or is embedded inside of a page.

Ping
I am not sure if the animation is broken but I think the initial state should still be the small one and maybe crossfade into the bigger version.
I mean I am not sure if this would behave like people would expect it to.

Fiddled around a little bit. Came up with something like this.

@keyframes reduced-ping {
  0% {
    opacity: 1;
  }
  33% {
    transform: scale(1);
  }
  33.01% {
    transform: scale(2);
    opacity: 0;
  }
  60% {
    opacity: .5;
  }
  90%,
  100% {
    transform: scale(2);
    opacity: 0;
  }
}

So it adds two extra steps one that first shows the small one and fades it out and a second that fades in the large one but just until about 50% opactiy to then fade it out again. 🙂 Tbh. I don't know any other way to have a like an more or less instant transition between two states in an animation besides setting low third digit percentage differences.

Regarding the shake and bounce animations.

Shake
Imo we can leave the crossfade out. It is blinking a bit too much. It feels a bit like we need to put in a trigger warning with that and with that we would open up a whole new topic. I'd just use a step animation to the outer edges of the shake and then back to the middle. The animation overall just needs to many steps to see that it is shaking for using a crossfade .

Bounce
I don't think there is a way to properly simulate the bounce without any movement.

Generally speaking about shake and bounce I am not quite sure if it makes sense to try to simulate the for reduced motion users. As the overall positioning of the element doesn't change and the elements end and start points are equal I'd just suggest using something like blink to show some animation on the element.

I don't think we have to provide like an 1:1 experience for reduced motion for every animation. There are some cases that just can't be shown. In cases like this I always think of it like of the IE problem back in the days, we don't have to make sure everything looks the same on all devices. We just have to make sure no one gets explicitly excluded and has access to the same content with an experience suited to his / her individual settings and technological choice.
Even tho our problem isn't something like browser compatibility anymore I think this point a view can be applied to our specific case as well 🙂 People who have motion sickness never will change back to non reduced motion and think like Yo! These guys are withholding animations from us! 😄 I mean looking at the effort and thought process that has been put into this, every single person that suffers from motion sickness is probably thankful that we are thinking about them 🙂

@saitheninja
Copy link
Author

link to my pen for convenience: https://codepen.io/saitheninja/pen/XWPjWjR

i appreciate the feedback. it all makes a lot of sense.
i added no-blink and blink-only versions of shake and bounce. i could also it switch to the normal blink animation.


spin:

that in general is a problem we cannot solve as it is within the hands of the user on how fast the animation is played

my thoughts exactly.

ping:

I am not sure if the animation is broken but I think the initial state should still be the small one and maybe crossfade into the bigger version.

i couldn't figure out a way to smoothly transition the scale while already using a fade for the ping effect. i like the
solution they came up with. i copied it over.

shake:

The animation overall just needs to many steps to see that it is shaking for using a crossfade.

makes sense to me. removed the fades, and made a blink-only option.

bounce:

I don't think there is a way to properly simulate the bounce without any movement.

also makes sense. changed it to not match so closely. also made a blink-only option.


probably my most important take away:

We just have to make sure no one gets explicitly excluded and has access to the same content with an experience suited to his / her individual settings and technological choice.

@argyleink
Copy link
Owner

I really think we're onto an improvement here and want to keep moving it forward ❤️
You've really done some inspirational work here 🙂

What do you think of like, putting all "attention getters" into a category that gets reduced fallbacks that blink? This would mean that all the hard to reduce animations become basic attention getters, while all the animations we're confident do have nice crossfade reductions can continue to be as they are.

Categories like this:

Has reduced animation with reasonably similar effect

  • scale up
  • scale-down
  • slide-out-up
  • slide-out-down
  • slide-out-right
  • slide-out-left
  • slide-in-up
  • slide-in-down
  • slide-in-right
  • slide-in-left
  • blink

Has the same attention getting reduced animation, a slowish blink

  • shake-x
  • shake-y
  • spin
  • ping
  • float
  • bounce
  • pulse

This feels helpful for anyone who uses one of the attention getters, that when one of their users needs reduced motion, they get a reasonable attention getter (which was the goal of the CSS author anyway). This also means the CSS we ship isn't super bloaty for all the reduced motion fallbacks. 7 animations all get the same slow blink fallback 👍🏻

tldr;
we move forward with all the scales and slides and make all the others a blink attention getter. and ship it.

thoughts?

@Que-tin
Copy link
Contributor

Que-tin commented Apr 13, 2023

I'll quickly jump back into the loop, as this is finally close to shipping.

What do you think of like, putting all "attention getters" into a category that gets reduced fallbacks that blink?

Ship it! I think this is close to if not the most optimal solution for this use case, where we have reusable and freely configurable animations, there is.

This feels helpful for anyone who uses one of the attention getters, that when one of their users needs reduced motion, they get a reasonable attention getter

Do we put this somewhere inside the docs and maybe add smth. like a toggle on the top right of the animation sample section to give people an option to easily inspect those reduced animations?

7 animations all get the same slow blink fallback

We are just reusing the normal blink animation here right?


One more small side note, could we agree on naming / renaming those reduced animation with the following pattern ANIMATION_NAME-reduced, before merging? The suffix less or similar doesn't really feel consistent to what it really does.


@saitheninja, @argyleink Thanks for putting in the work & sorry for not finishing #194!

@argyleink
Copy link
Owner

Do we put this somewhere inside the docs and maybe add smth. like a toggle

I was thinking about this too, how to help folks test and understand what they're getting. A toggle sounds like more of a lift then I'm willing to do, so here's a couple alternative ideas:

  1. provide a codepen with them side by side, kinda like we have now
  2. add a chunk of text that explains it, like this
    Screenshot 2023-04-13 at 7 58 44 AM
  3. prompt them with some text that explains how to use devtools to emulate reduced motion so they can preview the effect

We are just reusing the normal blink animation here right?

yep, it's totally chill enough to just be reused 👍🏻

Thanks for putting in the work & sorry for not finishing

all good! this is a complex thing we're trying to innovate and be helpful with, iteration was a natural process 🙂

@Que-tin
Copy link
Contributor

Que-tin commented Apr 13, 2023

  1. provide a codepen with them side by side, kinda like we have now
  2. add a chunk of text that explains it, like this

How about both? Text + smth. like:
for a detailed overview of our reduced motion animations check here

Copy link
Owner

@argyleink argyleink left a comment

Choose a reason for hiding this comment

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

hey! glad to see commits in here 🙂
I see it's a draft too, so maybe I'm giving this a premature review 😅

for starters, it's super hard to tell what you're adding and what the linter/formatter changed.. can you remove all the changes that arent part of this reduced motion PR please? it will drastically help me review and merge with confidence.

looks like a big chunk of the work is done tho! when i set reduced motion in my browser and try the samples, most of them begin correctly but dont fade in at the end state. that's something you're hackin on? the slide-in's tho, they seem unfinished?

thanks for workin this stuff out, hope my early review didnt cramp your style. i'm certainly excited for all this! 🙂

@saitheninja
Copy link
Author

whoops! i forgot that pushing would trigger the CI. sorry, my bad.

hope my early review didnt cramp your style

no worries, just needed to clean up a bit. i implemented the suggestions from you and @Que-tin, and cleaned up the diff. the documentation might need some work. everything else is ready for review!

also i tested the new animations and it looks good on my side. i used the latest stable Chrome and Firefox on Linux. maybe @Que-tin could also test it out?


a summary:
the "attention getters" have too much movement to emulate. even a reduced version can still be disorientating when played too fast. since the user is in control of the speed, we switch it to the blink animation, because it achieves the same intented purpose: attention getter.

in/out that has similar reduced effect:

  • fade-{in,out} (no change)
  • fade-{in,out}-bloom (no change)
  • scale-{up,down}
  • slide-{in,out}-{up,down,left,right}

"attention getters" (start and end are the same) reduced to blink:

  • blink (no change)
  • shake-{x,y}
  • ping
  • float
  • pulse
  • spin
  • bounce

@Que-tin
Copy link
Contributor

Que-tin commented Apr 28, 2023

Just checking into it. There is actually a bit more to it than just changing the props.animations.css as this is a build artifact. I'll put something together and let you know how we'll proceed. I'll probably create a PR into your branch 👍.

Because we actually need to add smth. to the stylesheet generation functions and reuse that inside of the props.animations.js file.

@Que-tin Que-tin mentioned this pull request Apr 28, 2023
saitheninja and others added 3 commits April 29, 2023 13:39
[add] Dynamic reduced animations
[add] Local reference for the blink animation values
@saitheninja
Copy link
Author

awesome, thanks for your help @Que-tin. i see what i missed.

i think git is synced and merged properly now.

check if everything looks the same after running a npm run build

i did a npm run build && npm run serve and it seems to be working on my side.
slide-ins might be looking a little weird because they are playing pretty fast and overlapping. it's the same frames from the codepen though. i slowed it down to double check and it looks right.

@Que-tin
Copy link
Contributor

Que-tin commented Apr 29, 2023

From a pure build & technological stand point I'm fine with the PR.

We might have to look at the slide-in's inside of the docs again then. Looks pretty weird to me and I'm not sure if the current visualization is useful combined with the current animation speed.

Regarding that and the docs content itself @argyleink is the person to decide.

Update:
I might need to add a comment to the to-stylesheet.js regarding the .reduce I'm doing there, not sure about that tho. Two .filter's would've also done the trick, but I'm a fan of not looping the same element multiple times if I can just use a reduce. What do you guys think?

@argyleink
Copy link
Owner

i'll be takin a deep look and dive this week! thanks for hackin y'all 🤘🏻

build/to-stylesheet.js Outdated Show resolved Hide resolved
Copy link
Owner

@argyleink argyleink left a comment

Choose a reason for hiding this comment

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

i've made a suggestion here that fixes a couple things:

  1. groups all the keyframes under the same MQ
  2. removes the semicolon after ${val}, required after change Gen props from a javascript object #1 here

These animations aren't behaving properly though. Need fixed before this can merge:

  1. slide in down
  2. slide in up
  3. slide in right
  4. slide in left

All the animations:

  • to me it seems like they're abit too quick
  • the initial fade outs are great, but the fade in's feel abrupt

Lastly, the tests need updated. Looks like the number here needs updated to account for the additional keyframes.

All in all, nice work y'all, feels like we're gettin close to done!

[fix] Total props count test
@Que-tin
Copy link
Contributor

Que-tin commented May 8, 2023

Fixed the test and made the changes suggested to the to-stylesheet.js inside of there https://github.com/saitheninja/open-props/pull/2/files.

Regarding the animations, @saitheninja could you take a look at it?

@argyleink
Copy link
Owner

looks like there's a build issue too to fix https://github.com/argyleink/open-props/actions/runs/4936351522/jobs/8951530987?pr=341

@Que-tin
Copy link
Contributor

Que-tin commented May 18, 2023

looks like there's a build issue too to fix https://github.com/argyleink/open-props/actions/runs/4936351522/jobs/8951530987?pr=341

I think my pull request didn't get merged yet.

@saitheninja I think we've worked on the same stuff here, but there are a few problems inside your build adjustments. Let's work together on getting my PR merged inside of your branch so we can finish this of :)

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.

6 participants