ui-layout
ui-layout copied to clipboard
angular-ui
Optional animations
This pull request adds an attribute animate that needs to be set to "true" in order for the animations to be activated.
This can be used as a work-around for issue #161.
I was thinking about something like this, but left that out, because I couldn't see a need to dynamically dis/enable animations. I suppose an application will either animate or not. To disable animation, just declare empty animate-{row,column} classes. But I guess there could be a use case for that.
I suppose it might be worth trying to turn the animation off on drag start and then turn it back on...
Awesome, this looks great! Thanks for the tests.
I do think that it'd be better to have animations on by default instead of off - that way you can still disable and it's not a major change.
It occurred to me, that the purpose of the animations is really to animate the collapsing/toggling. So I would suggest that we add the classes before toggling a container and remove it afterwards. What do you think.
Hm, that'd be interesting to see, mind putting together that in a Plunkr?
@SomeKittens I'd like to do that later this week, if time permits. But will wait for @widmoser 's reaction since s/he seems to be very active at the moment.
@petrsimon, @SomeKittens I turned on animations by default in the PR. I like the idea to limit animations to collapsing/toggling as it might also fix the issues reported in #161, but I think it would be better to create a separate PR for this. I can look into it at some point, but at the moment #166 has more priority for me.
Something I was also thinking about is to have a look at whether it would be possible to simplify the collapse/toggle implementation and after that do everything related to it (persisting state, etc.), because right now I am not very confident in changing something in relation to it because I don't understand it fully. I'll probably have to examine the code in more detail :)
@SomeKittens is there something else that needs to be done before merging this PR?
@widmoser Hi, sorry. Work got busy and then it got worse. Trying to catch up now.
This PR has been pending for so long. Dear Dev team, could you please consider review the change and merge into upstream?
