-
Notifications
You must be signed in to change notification settings - Fork 3k
Animation is now evaluated dynamically #96
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
Conversation
Hmm... core ng-view only creates the animator once on link, and this is also what the ng-animate guide says to do (http://www.yearofmoo.com/2013/04/animation-in-angularjs.html#using-animatings-in-your-own-directives), so at this point I'm not really convinced that this change is the right thing to do. |
@pennersr what is the use case, and what isn't happening the way you expected it to happen?... Considering that we follow the core teams recomendations, it would also be really interesting this is the same behavior in core, and if it should be something that should be posted to them instead. E.g. if your scenario is meant to work according to the core team, but actually doesn't then it could be a bug in either their directives as well or the animator implementation, and if it's the last being the case, that is why we really don't wan't to change things, as it would be a workaround to a bug, and then I would rather see the bug being fixed. |
My use case is as follows: I would like to use ui-router for the navigation of a mobile app, where the animation is dependent on the state the user is navigating to. For example, a dialog-like view needs to slide up, whereas normal views slide in from right to left. Furthermore, if my states are ordered I can easily detect whether or the user is moving forward (right-to-left animation), or going back (left-to-right animation). I can easily accomplish this by using a dynamic lookup for the ng-animate. For example, one could inspect the from/to state on Is there any other way to accomplish this? |
Sounds like a valid use case, but I think we need a more obvious way of doing this. I think $animator needs the ability to pass a user-defined 'context' object to the (JS) animation, for which we could pass the transition object with properties like 'from', 'to' etc identifying the states involved. This meta-animation could then pick the actual animation to perform -- assuming $animator was to provider some simple way of doing that. |
Is there somewhere where we can validate whether or not my patch is really off-limits? I've read http://www.yearofmoo.com/2013/04/animation-in-angularjs.html#using-animatings-in-your-own-directives, and while it lists some sample code I have difficulties interpreting that as the only, one true way of doing things. IMHO, not picking up the animation dynamically feels rather un-angularly. |
@pennersr if you watch the video with Msiko, I do indeed think your meant to be able to pick up these animations dynamically, that is why I am questioning if this is really an issue in the core instead. (If I remember correctly at least) If we fix it like in your pull request, it would only be fixed for ui-view, but still be broken for ng-view, ng-include and all the others, that is why it is very interesting to me to figure out weather the issue also exists in those or not. And if it is in reality a bug in the $animator or just in their directives as well, if their directives are the ones they wan't to fix, then we will to, but if they wan't to fix the animator for this, then doing this fix here, might even cause trouble. |
Indeed, which is why I submitted angular/angular.js#2480 |
Any progress on resolving this? Wouldn't it be good to patch this in for now, and then move forward with the angularjs ticket? It's not like this change requires a huge code fork; it's a small two line change. 🌴 I'd say it's good for ui-router to make opinionated decisions that contradict what angularjs core is doing wrong. It will encourage them to fix it. |
@ajoslin I agree that the dynamic evaluation behaviour is desireable, but from an API point of view having to recreate the animator object before each use is just a work-around: The two-step approach of getting an animator instance from $animate and then using it is clearly designed such that the animator does not need to do all the setup work all over again for each animation. So a proper fix for this issue can really only happen in $animator. If this issue is considered important enough we could include the workaround, and then make sure we go back to the recommended way of doing it once it's fixed in core. |
Does anyone have any sample code to get around this issue? Even if it's specific to ui-router only. I too need to animate right/left, left/right depending on what was clicked/touched. I would have thought you pass |
Starting since Angular 1.1.5, |
Awesome. Do you happen to know the commit that introduced it? We can use that as the reference implementation. If you don't know off-hand, no worries, I'll look it up when I get a chance. |
Looks like this is the commit that introduced it: angular/angular.js@fd21c75 |
Tested with Angular 1.1.5 on a |
Awesome. @pennersr Can you verify on your end? |
Confirmative, with 1.1.5 this works fine with the current ui-router. Closing... |
I was trying to modify the view animation dynamically, e.g. by using one of:
instead of the fixed:
While this works, the scope references are only evaluated once at link time, hence, changing those dynamically did not work.