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

Chore/remove velocity #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Chore/remove velocity #1

wants to merge 2 commits into from

Conversation

runspired
Copy link
Collaborator

This PR removes velocity usage. For the most part, this is a seamless removal

  • Remove velocity transitions
  • Remove velocity import and version test
  • Remove velocity based animation primitives
  • Alter Growable Mixin to not require velocity

TODO:

  • Amend Demo Site (Demos should stay either with liquid-fire proper or with liquid-fire-velocity).
  • Fix any tests that expected velocity

Questions:

  • Is registering an animate function with a specific API on the liquid-fire-transitions service the way to approach liquid-fire-core's need for an animate function within Growable (@ef4 states no, changeWidth changeHeight api or similar would make more sense)
  • Where does the velocity based demo site belong (I believe it belongs with liquid-fire proper)
  • Do we remove explode (this is not velocity backed, so where does it belong here?) (@ef4 states it belongs here)

…-user supplied animation function. We will need to flesh this out a bit more
@runspired
Copy link
Collaborator Author

Related PR to remove core modules in liquid-fire-velocity: ember-animation/liquid-fire-velocity#1

@ef4
Copy link
Contributor

ef4 commented Jan 15, 2016

Is registering an animate function with a specific API on the liquid-fire-transitions service the way to approach liquid-fire-core's need for an animate function within Growable?

animate is probably too fat and nebulous of an API. It's a very thin wrapper over velocity and can go in liquid-fire-velocity.

We can make something more specific to support growable. Maybe each animation library needs to implement changeWidth and changeHeight transitions that we can use internally.

@runspired
Copy link
Collaborator Author

@ef4 that makes a lot of sense to me, would they be required to be registered on liquid-fire-transitions ?

Or would they be expected to be in a certain place in app/utils which could then be imported by the service or an initializer or some such?

@ef4
Copy link
Contributor

ef4 commented Jan 15, 2016

They can be regular transitions, no need for a special API.

@runspired
Copy link
Collaborator Author

doh

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.

2 participants