search

andrewMacmurray / elm-simple-animation

stateless animation utils for Elm
https://package.elm-lang.org/packages/andrewMacmurray/elm-simple-animation/latest/
MIT License
50 stars 3 forks source link

Add 'Animation.empty' definition for convenience #14

Closed 13r0ck closed 3 years ago

13r0ck commented 3 years ago

Maybe I am going at this the wrong way, but I thought that adding Animation.empty would make it easier to match either no animation or some animation depending on the model.

My thinking is 

expandFade =
    Animation.fromTo
        { duration = 2000
        , options = [ Animation.loop ]
        }
        [ P.opacity 1, P.scale 1 ]
        [ P.opacity 0, P.scale 2 ]

animatedEl
    ( if model.needsAnimation then
          expandFade
      else
          Animation.empty
    )

If I am going at this the wrong way then I can close this PR, also not sure if Animation.empty or Animation.none makse more sense?

andrewMacmurray commented 3 years ago

Thanks for this @13r0ck, in my own projects I've been pushing the "empty" bit closer to the html / view part - so something more like:

if shouldAnimate then
   Animated.div expandFade [] [ element ]
else
   element

I'd rather not "force" clients down either decision in favour of keeping the api small.

Why don't you open an issue and see if we can get some feedback from people? If more people like it we can definitely include it.

13r0ck commented 3 years ago

Created #15 as requested. Thanks!

andrewMacmurray commented 3 years ago

Gonna close this for now until we get some more feedback on issue #15