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

Add 'Animation.empty' definition for convenience #14

Closed
wants to merge 2 commits into from
Closed

Add 'Animation.empty' definition for convenience #14

wants to merge 2 commits into from

Conversation

13r0ck
Copy link
Contributor

@13r0ck 13r0ck commented Apr 12, 2021

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
Copy link
Owner

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
Copy link
Contributor Author

13r0ck commented Apr 13, 2021

Created #15 as requested. Thanks!

@andrewMacmurray
Copy link
Owner

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

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.

None yet

2 participants