233 migrate to react spring

[idoberko2]

Related Issue:

233

Summary: Refactored the following components to use react-spring animations library instead of react-motion:

• BigClickableButton
• MainPane
• Modal
• ProgressBar
• Sidebar
• Toggle (not to CSS animations, only to react-spring)
• TwoPaneModal

Notes: I did not remove the react-motion dependency from package.json since it's being used also in CircularOutline. If you wish to have CircularOutline refactored as well, please let me know.

[AWolf81]

Sorry, for the long time waiting for the review.

I've compared the animations with master and noticed two points:

• CreateNewProject opening animation: You've added the folding animation. In master it was just a slide animation before. I think the new animation is OK (maybe a bit slower would look better) but for some people this could be a bit too much. For me it's OK if it would be slightly slower.
• Show modal animation: On master it's sliding in a bit from bottom up and in your branch it's not sliding. I like the effect on master.

Overall your code looks great just found the two differences.

For the outline animation, I think we can address this separately and I think we should create an issue for it. I'm also not sure if @joshwcomeau worked on it. I think he wanted to improve it.

Comparison of the animations master vs. migration branch

Master - CreateNewProject open animation Migrate_to_react_spring - CreateNewProject open animation

Master - Modal open animation Migrate_to_react_spring - Modal open animation

[idoberko2]

Hi @AWolf81, The Modal was probably broken by me, I will investigate it and push a fix soon. About the added animation in TwoPaneModal (create new project): actually it wasn't added by me, as you can see the code exists already in master: https://github.com/joshwcomeau/guppy/blob/ce86fe0293c56bc2b0ee98df3972112625cb2d5a/src/components/TwoPaneModal/TwoPaneModal.js#L97-L101 My guess is that something prevented it from working when using Motion and switching libraries made it working somehow. I can try to remove it if you believe it should be removed, I actually thought it was nice. What do you say? Should I remove it?

[AWolf81]

@idoberko2 ah, OK, I would remove the folding animation from opening to be consistent to the look of the modal. I also like the folding animation but I think that's to heavy for an opening animation.

But please check that the folding happens at the last form step. So it closes the TwoPaneModal and displays the build pane with a folding animation.

[idoberko2]

@AWolf81, thanks for that form note, that helped me find the bug I introduced. I fixed the issues in both components (Modal and TwoPaneModal). Please note that after my fix to TwoPaneModal the state variable isBeingDismissed was no longer needed, so I removed it and the logic related to its update.

[AWolf81]

@idoberko2 thanks, Modal & TwoPanelModal is working.

Can you please have a look at the Toggle button? I'm not sure if this is also on master but I think that's new. It looks like the shape of the toggle handle gets elliptic during animation. See screenrecording below - it's more visible in Guppy then in the Storybook. Update OK, I've checked on another branch. It's also getting elliptical during transition but the performance is a bit better so I haven't noticed that before. Duration for the toggle animation is around 0.7s (I'll check that in your branch as well).

Please also check if adding native to toggle button will improve performance. Performance pretty choppy on my side (see screenshot below) - not sure if this is related to my machine. In the profile there are two click events.

Performance profile

[idoberko2]

Hey @AWolf81, sorry for the time it took, I tried several solutions until I came to this result. The Toggle now looks good on my machine, better than than what's found in master branch. Please let me know how it looks on your end and if something requires fixing.

[AWolf81]

Hi @idoberko2, no problem and thanks for your update. I think it's better now. But I'm not sure if it's related to my machine because master is also not smoothly animating. It's still not running smoothly for me. Sometimes it gets skewed and then it jumps to round at the end position. In the screenrecording I think it's not that clearly visible but I think it could be related to minimizing Guppy and opening it later - seems to be worse sometimes.

@melanieseltzer, would be great if you could have a look at the toggle button and if the animation is working similar to master? It's difficult for me to test.

If it's just on my machine I think we could leave the animation as it is now.

[melanieseltzer]

I poked around in the code and the skew is coming from this line. Set stretchAddition to 0 and it's not skewed anymore. It looks like master has a slight skew though. If we bump the value down to 0.4 it looks better and more subtle to me, @AWolf81 could you try that? If that looks better for you then @idoberko2 can change the value.

Performance-wise, I'm not noticing a major lag like you are, @AWolf81. It's behaving like master for me but others might want to chime in.

However, I do notice that upon loading Guppy, the toggle is starting on the right and shifting to the left. It's subtle but here's a screen cap:

I think it could be prevented by removing the 'from' prop? Since react-spring documentation says it only plays a role when the component first renders...

from={{
  translate: isToggled ? 0 : 100,
  stretch: isToggled ? -stretchAddition : stretchAddition,
}}

I think that's what's causing it to animate right > left on mount, because isToggled is false on mount, so translate animation will be 100 > 0.

Or is there another way to turn off animation on mount?

[idoberko2]

Hey @melanieseltzer! You're totally right about the from prop, I missed that one. Regarding the skew/stretch effect - it took me a while to perform this refactor, it was a complicated piece of code in master that took care of this effect (renderBall). I think that the value I set for the stretchAddition makes the behavior as similar as possible to what's currently on master but I also agree that 0.4 looks better. I have this changes locally (removing from and set stretchAddition to 0.4), waiting to hear @AWolf81's thoughts about the 0.4 before I push them. Thanks!

[AWolf81]

Good catch @melanieseltzer. I haven't noticed that mount behaviour before but removing from fixes the animation on mount problem.

I think 0.4 is also OK but I can not judge it in Guppy. I've created a Codesandbox with the ToggleButton and the toggling speed is really fast there. Maybe there is something else in Guppy that's also animating/blocking and causing the problem.

I'd like to have the toggling snappy maybe we could debounce the launchDevServer task. But that's a separate issue.

So I think animations are OK after the requested changes from Melanie.

[j-f1]

0.4s is what Apple tends to use for animations IIRC.