angular-ui / ui-router

The de-facto solution to flexible routing with nested views in AngularJS
http://ui-router.github.io/
MIT License
13.53k stars 3k forks source link

Animations with Angular 1.2.0rc1 #320

Closed lanceschi closed 10 years ago

lanceschi commented 11 years ago

Hi,

it seems animations don't work at all with the new release of Angular 1.2.0rc1. Most probably a fix is needed.

Great job btw.

Cheers, Luca

bcronje commented 11 years ago

Yes it seems the ngAnimate API changed, see http://www.yearofmoo.com/2013/08/remastered-animation-in-angularjs-1-2.html for the rundown. Hopefully someone on the ui-router team will pick this up, as it should be a core functionality of any ui route implementation in my opinion.

timkindberg commented 11 years ago

Of course we will. As soon as its in a stable release.

lanceschi commented 11 years ago

Btw with Angular 1.2.0rc1 additional core modules now are to be specified: angular.module("unnamedApp", ['ngRoute', 'ngAnimate' ...

Cheers, Luca

nateabele commented 11 years ago

@timkindberg Maybe we can put this on a angular-1.2 branch or something.

timkindberg commented 11 years ago

Good idea

rlrosa commented 11 years ago

this is pretty nice http://www.yearofmoo.com/2013/08/remastered-animation-in-angularjs-1-2.html#how-to-make-animations-in-angularjs

nateabele commented 11 years ago

Okay, I've created an angular-1.2 branch which contains a refactored uiView directive and updated tests. However, 3 tests are still failing for reasons I haven't been able to figure out.

If you're using Angular 1.2 for development, I would encourage you to check out and build from the branch, help test the changes, and help debug the failing tests. Thanks.

bcronje commented 11 years ago

Nate, first bug:

The second parameter to $animate.leave() is a callback, you currently have it set to element.

Should be changed to

remove: function(element) { $animate.leave(element.contents()); },

Doesn't do anything for the failing tests though.

bcronje commented 11 years ago

Nate, I played around with this branch over the weekend and found a few issues. Then new enter(element, parent, after, done) animation method actually applies the animation classes to element and not parent, so with your current implementation you have to wrap your template code for the animations to work as intended which is not ideal.

I'll send a PR later today that might be a step in the right direction.

Question, is the ability to render the default content inside a ui-view that does not have a template a requirement? Or can any static content inside ui-view be discarded?

nateabele commented 11 years ago

Question, is the ability to render the default content inside a ui-view that does not have a template a requirement?

Yes. It should be present whenever there's nothing else to render.

bcronje commented 11 years ago

Nate, just send the PR. There are still 2 tests failing, but manually testing them produces the correct results, so not sure what's up with the unit tests. I also tested this in my own project and it seems to work fine.

Got inspiration from how ngView and angular-route-segment are handling the 1.2 animation changes.

Does this work for you?

Rodeoclash commented 11 years ago

I've also been playing around with this branch in conjunction with Angular 1.2 and it's been working great, looking forward to getting it merged in.

I have noticed that the $stateChangeStart event seems to fire multiple times. I haven't dug into this too much and it might be something else stupid that I'm doing, so I'll report back anything here. Just wanted to say the branch works great from POV.

ubenzer commented 11 years ago

@Rodeoclash it may be related to this issue i think: https://github.com/angular-ui/ui-router/issues/350 Check it out. :)

arush commented 11 years ago

Thanks for working on this guys, will test it on our dev too

Rodeoclash commented 11 years ago

@ubenzer hey, it was the most basic of mistakes on my end, assignment instead of conditional checking that was causing it. You might want to remove the possible link between them as I don't think their is one.

Otherwise zero problems with this branch, been working a treat.

devuo commented 11 years ago

I have also been using this branch, and so far, no problems at all.

arush commented 11 years ago

Hey guys, unfortunately I have found a bug when dynamically assigning the class with a scope variable (e.g. ng-class="this.variable") - one of the main reasons for the animations rewrite was to enable flexible class based animations like this.

Can't tell what is causing it, but I have successfully been able to reproduce it on multiple different apps.

Expected results (working with ng-view)

<div ng-view class="firstAnimationClass ng-leave" ng-class="this.variable"> <div ng-view class="firstAnimationClass ng-enter" ng-class="this.variable"> or <div ng-view class="secondAnimationClass ng-leave" ng-class="this.variable"> <div ng-view class="secondAnimationClass ng-enter" ng-class="this.variable">

(firstAnimationClass/secondAnimationClass is assigned by ng-class="this.variable")

No matter how the dynamic class changes, both the entering and leaving view will have the same animation class

Actual results (using ui-view)

<div ui-view class="firstAnimationClass ng-leave" ng-class="this.variable"> <div ui-view class="secondAnimationClass ng-enter" ng-class="this.variable">

as soon as ng-class changes, the animation classes are out of sync and never get back in sync

This situation should NEVER arise.


Code to reproduce

I have put together 2 fiddles that are identical, except the first uses angular 1.2 ng-view, and the second uses ui-router/angular-1.2

ng-view: http://jsfiddle.net/qZq8T/ ui-view: http://jsfiddle.net/qZq8T/1/

These should behave identically, it is the same code. But you will quickly see how animation classes get out of sync.

nateabele commented 11 years ago

@arush Awesome, thanks for putting this together. I'm actually heading to a conference right now, but I'll try and poke at it this weekend.

michalmikolajczyk commented 11 years ago

@arush, I have been trying to get animations to work with ui-view and angular-ui-router and couldn't, until I tried the code from your fiddle (lines 1-1494). angular-ui-router.js is the same in branches: master and angular-1.2.

anyway, with your version, it works. Is the code going to be pulled into one of the branches?

arush commented 11 years ago

Michal you have to grunt build from the angular-1.2 branch to generate the code in my fiddle.

Problem is there is still a bug with that code as I have described

— Sent from Mailbox for iPhone

On Sun, Oct 13, 2013 at 8:55 AM, Michał Mikołajczyk notifications@github.com wrote:

@arush, I have been trying to get animations to work with ui-view and angular-ui-router and couldn't, until I tried the code from your fiddle (lines 1-1494). angular-ui-router.js is the same in branches: master and angular-1.2.

anyway, with your version, it works. Is the code going to be pulled into one of the branches?

Reply to this email directly or view it on GitHub: https://github.com/angular-ui/ui-router/issues/320#issuecomment-26220482

michalmikolajczyk commented 11 years ago

never used grunt-build, I am using Yeoman with Grunt and Bower, all AngularJS source files I get from Bower.

Today I installed ui-router by including this in the bower.json file: "angular-ui-router": "https://github.com/angular-ui/ui-router.git#angular-1.2"

and that worked fine, the repo was installed correctly, but it seems that https://github.com/angular-ui/ui-router/blob/angular-1.2/release/angular-ui-router.js is the same as https://github.com/angular-ui/ui-router/blob/master/release/angular-ui-router.js

Anyway, I think I will skip ui-router for now. i need multiple views and I need control over which views get reloaded on state change. Ui-router only allows that with nested views. From what I found out and tried, changing state with multiple-views will cause reload to sibling (same level) views. In my app, I have two completely separate views and when I change state in one of them, I need to preserve the other one. I dont really care if I go from: myapp/#!/blog to myapp/#!/app or myapp/#!/blog to myapp/#!/blog?app=search_route I know I can preserve state data in a service, but I really need to preserve the views (like when a user scrolled down, I dont want the view to reload and start from top again, or cause page flicker).

I know about https://github.com/angular-ui/ui-router/issues/125 but that hasn't been resolved yet.

So... I started typing and in the meantime i found out about 'Sticky Views' in the Angular-routing project https://github.com/dotJEM/angular-routing/wiki/State-provider---views#sticky-views That seems to be exactly what I need. I am only worried that the project seems a bit young, not sure if it's ready for production.

Any advice? :)

zurie commented 11 years ago

@wiherek5 what you need to do is download the angular ui-router 1.2 angular branch.. seperately. then cd to that directory and do a NPM install and a bower install and a grunt build. (or i think grunt server) and that will actually build you a angular ui-router based on the branch. you can tell you are using the right one because when you open up the normal .js in the header you will see * @version v0.2.0 if you open up your BUILT ui-router from the branch you will see this instead. * @version v0.2.0-dev-2013-10-05 make sure your version has a dev writtin in the header otherwise you aren't using the new code.

dlukez commented 11 years ago

@arush I think I'm encountering a similar issue where other directives on my ui-view aren't updating (e.g. ng-class).

In the case of ng-class, when I inspect the call to $animate.addClass as a part of the ngClass change, it tells me that the target element doesn't have a parent so I assume it's linked to some other clone or template element (unsure) that isn't injected into the DOM.

I messed with the view directive code and managed to get the animations working for me, but broke like 8 tests at the same time. I'm still learning the ui-router stuff, so I only have a basic understanding of what's going on. I basically mimicked what's been done in the Angular 1.2 RC3 ng-view directive (not sure how much that has changed in the last few versions).

Your jsfiddle with my code: http://jsfiddle.net/dlukez/7RwNK/1/

Perhaps someone with a greater understanding can explain why those changes fix the animations and if necessary, factor in the appropriate changes.

My commit: dlukez/ui-router@652d2c3f0fc9b0090db1d5b8adcf21013d4a73f9

May try to have a look into why the tests break - any guidance would be welcome!

EDIT: The above commit was causing an error. This one is more up to date: dlukez/ui-router@a09ee6f39f04a9d5d3e3f66ea8ee1a715a445940

bnguyen82 commented 11 years ago

Awesome! I apply the new branch "angular-ui-router": "https://github.com/angular-ui/ui-router.git#angular-1.2" and my animation works again with angular 1.2-rc.3. However, I don't see $stateChangeStart firing multiple times as @Rodeoclash mentioned.

Thanks very much.

perqa commented 11 years ago

@bnguyen82, are you using the new (1.2), or older, syntax for your animations to work? Ref: http://www.yearofmoo.com/2013/08/remastered-animation-in-angularjs-1-2.html

arush commented 11 years ago

@dlukez interesting... I updated your fiddle with rc3 and looks to work. Are you still getting failed tests?

http://jsfiddle.net/7RwNK/4/

perqa commented 11 years ago

@arush & @dlukez That fiddle does not seem to adhere to angular 1.2 syntax, as described in http://www.yearofmoo.com/#how-to-make-animations-in-angularjs. I see for example an additional ng-class="slideDir" and some custom logic. Is that necessary with the angular 1.2 branch of ui-router?

Also, the direction of the transitions get reversed if you click around for a while.

dlukez commented 11 years ago

@perqa I think this is what @arush was hoping to achieve... The slide changes direction when you hit certain buttons and the direction is selected using the ng-class directive. This stuff isn't necessary to use the 1.2 branch of ui-router, unless you want other directives on your ui-view element.

@arush Yeah still getting a couple, but 2 from 7 is more hopeful. Having a look into them now. The other failed tests were caused by expecting a "leave" animation on first view load. Is this expected behaviour? Doesn't really seem logical to me...

arush commented 11 years ago

Yes, disagree with the need for a leave animation on first view load

— Sent from Mailbox for iPhone

On Sat, Oct 26, 2013 at 11:52 AM, Daniel Zimmermann notifications@github.com wrote:

@perqa I think this is what @arush was hoping to achieve... The slide changes direction when you hit certain buttons and the direction is selected using the ng-class directive. This stuff isn't necessary to use the 1.2 branch of ui-router, unless you want other directives on your ui-view element.

@arush Yeah still getting a couple, but 2 from 7 is more hopeful. Having a look into them now. The other failed tests were caused by expecting a "leave" animation on first view load. Is this expected behaviour? Doesn't really seem logical to me...

Reply to this email directly or view it on GitHub: https://github.com/angular-ui/ui-router/issues/320#issuecomment-27143882

Meligy commented 10 years ago

For lack of voting up issues in Github, is this expected any time soon? Thanks a lot.

ubenzer commented 10 years ago

Angular 1.2. stable is out. What is the development strategy now?

Will angular-1.2 branch be merged to master? What is the plan? @nateabele @timkindberg

nateabele commented 10 years ago

@ubenzer We'll release 0.3 (I'm just trying to get 2 more bugs fixed up), then angular-1.2 will be merged to master.

nenebale commented 10 years ago

What is the ETA for 0.3? I really need animations working on 1.2 and if it's going to take too long I'll try to hack something up until 0.3 is out. @nateabele

joseym commented 10 years ago

so I grabbed 573c3f94e7 and didn't have any luck getting animations to work with Angular 1.2.0 RC3. Will this also be fixed with 0.3?

nenebale commented 10 years ago

It worked for me we when I merged angular-1.2 and master on 1.2.0 stable.

juanpujol commented 10 years ago

+1 impatiently waiting for 0.3 =D

dgsunesen commented 10 years ago

+1 me to is waiting! :)

dgsunesen commented 10 years ago

@nenebale - how exactly did you get it to work?

nenebale commented 10 years ago

Just merge the current master branch and the angular-1.2 branch locally and build it with grunt:build. @dgsunesen

dgsunesen commented 10 years ago

I keep getting conflicts and build errors

nenebale commented 10 years ago

I had to solve some merge conflicts, but after that Grunt builded it like a charm!

It is important not to take the default grunt task but instead use grunt:build as it skips the (failing) tests.

cheahkhing commented 10 years ago

+1 also impatiently waiting for the animations fix for 1.2 :)

igorhart commented 10 years ago

+1 that would be perfect! 20 lis 2013 18:52 "CooKies(tm)" notifications@github.com napisa³(a):

+1 also impatiently waiting for the animations fix for 1.2 :)

Reply to this email directly or view it on GitHubhttps://github.com/angular-ui/ui-router/issues/320#issuecomment-28912001 .

dgsunesen commented 10 years ago

@nenebale - hey dude. Do you mind sharing your merged stuff? I can't seem to get it working :-/

renz45 commented 10 years ago

The 1.2 branch works for me (no need to do any merging), but you're going to have to use the new angular animation syntax. Here is the CSS I use to slide right and left on transition:

.slide-left.ng-enter,
.slide-left.ng-leave,
.slide-right.ng-enter,
.slide-right.ng-leave {
  -webkit-transition: 0.5s ease-in-out ;
  -moz-transition: 0.5s ease-in-out ;
  -o-transition: 0.5s ease-in-out ;
  transition: 0.5s ease-in-out ;
  position: absolute;
  width: 100%;
}
.slide-left.ng-enter {
  z-index: 100;
  left: 100%;
}
.slide-left.ng-enter.ng-enter-active {
  left: 0;
  right: 0;
}
.slide-left.ng-leave {
  z-index: 101;
  left: 0;
}
.slide-left.ng-leave.ng-leave-active {
  left: -100%;
}

.slide-right.ng-enter {
  z-index: 100;
  left: -100%;
}
.slide-right.ng-enter.ng-enter-active {
  left: 0;
}
.slide-right.ng-leave {
  z-index: 101;
  left: 0;
}
.slide-right.ng-leave.ng-leave-active {
  left: 100%;
}

I change the class based on what is clicked, but here is the html:

<div class='slide-right' ui-view ></div>
<div class='slide-left' ui-view ></div>
dgsunesen commented 10 years ago

@renz45 Shouldn't it automatically add the ng-enter/ng-leave classes once I load new content into the ui-view?

bcronje commented 10 years ago

@dgsunesen It automatically adds those classes, but you still need to define the CSS. Without the CSS defined nothing will happen.

renz45 commented 10 years ago

It does, you should only need the slide-left or slide-right class for it to be triggered. I'm sorry if the html part above was confusing, I meant that I only have one div:

<div class='slide-left' ui-view ></div>

and I switch the slide-left or slide-right depending on which button is clicked. You don't use ng-animate anymore, at least not for ui-router transitions. Ui-router looks at the css transition values to figure out delays and timing.

dgsunesen commented 10 years ago

Yeah.. That's what I thought as well.. Though it doesn't add the ng-enter/ng-leave classes to my ui-view dom element, when I change state :(

renz45 commented 10 years ago

Did you include the ngAnimate module into your project? With angular 1.2.1 animate is no longer part of the default framework. You need to include the module manually now.

angular.module('myApp', ['ui.router', 'ngAnimate'])

You can download the module from http://code.angularjs.org/1.2.1/