pmndrs / react-spring

✌️ A spring physics based React animation library
http://www.react-spring.dev/
MIT License
28.19k stars 1.19k forks source link

[bug]: Shorthands don't merge with transform values #1913

Open varunarora opened 2 years ago

varunarora commented 2 years ago

Which react-spring target are you using?

What version of react-spring are you using?

9.4.5

What's Wrong?

(You probably know this one already, so please ignore if it is on a to-do list already.)

If I want to go from one transform value to another, with the library, I can't use shorthands in the to value, if I am starting off with a from value (which cannot have shorthands).

So the only way you can really go if you have a from transform string is to another transform string.

Is this by design? Not sure this is a bug. But since I spent a bunch of time learning this lesson, thought I'd share it.

To Reproduce

import { useSpring } from 'react-spring'

const [props, api] = useSpring(() => {
  from: { transform: 'scale3d(0.1, 0.1, 0.1) translate3d(0, -1000px, 0)' }
  to: { scale3d: '0.475 0.475 0.475' }
})

return <animated.div style={{props}} />

Expected Behaviour

For the shorthands to merge with the initial transform value in coming up with a final value.

Link to repo

-

joshuaellis commented 2 years ago

This isn't a bug and if I'm honest I'm not sure I would want to add this as a feature either I think it would make the API confusing.

Is there a reason you want to do it this way? It feels over complicated and hard to read.

varunarora commented 2 years ago

Thank you, Joshua!

Yes, my "to reproduce" code does feel hard to read.

So the reason I ended up running into this is because I wanted to interpolate two different transform properties separately. The shorthand prop API showed that promise. I eventually used the to function this way: https://codesandbox.io/s/focused-estrela-yy950s?file=/src/App.js, but not before I tried this approach.

And the reason I tried this approach is because the shorthand props are celebrated as a new features for v9. And it didn't hit me that easily that they only work for to props. This can be confusing. So perhaps the real suggestion here is to bring them to the from props too? So that there is consistency?

joshuaellis commented 2 years ago

And the reason I tried this approach is because the shorthand props are celebrated as a new features for v9. And it didn't hit me that easily that they only work for to props. This can be confusing. So perhaps the real suggestion here is to bring them to the from props too? So that there is consistency?

I don't think this is true see this example – https://codesandbox.io/s/nice-brook-fsuqh3?file=/src/App.js

varunarora commented 2 years ago

Whoa! Cray cray. I'll try this with my transforms.

joshuaellis commented 2 years ago

If this solves your issue, please close the issue, else we can discuss further.

varunarora commented 2 years ago

I wouldn't say the bug is exactly I was describing it (because yes, you are right, it does capture shorthands in from), but have a look at what's happening here: https://codesandbox.io/s/agitated-fog-s4sio8?file=/src/App.js?

It gets messy when there are multiple things to be transformed. After the animation ends, the transform resets.

joshuaellis commented 2 years ago

Hm weird, it doesn't reset when the scale isn't changed. There must be an issue where it sees scale has been animated to it's default setting – 1, 1, 1 in this instance if it was only that property it could set transform: none but we have the translate transform that it also removes... Presumably a false setting.

varunarora commented 2 years ago

You are right, it's only doing it with 1, 1, 1. Lol my every guess to figure out what this bug truly was turned out to be wrong

joshuaellis commented 2 years ago

You are right, it's only doing it with 1, 1, 1. Lol my every guess to figure out what this bug truly was turned out to be wrong

At least you were making some suggestions! 😄