Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat($animate): allow $animate to pass custom styles into animations #8974

Closed
wants to merge 3 commits into from

Conversation

matsko
Copy link
Contributor

@matsko matsko commented Sep 8, 2014

$animate now supports an optional parameter which provides CSS styling
which will be provided into the CSS-based animations as well as any
custom animation functions. Once the animation is complete then the
styles will be applied directly to the element. If no animation is
detected or the ngAnimate module is not active then the styles
will be applied immediately.

BREAKING CHANGE: staggering animations that use transitions will now
always block the transition from starting (via transition: 0s none)
up until the stagger step kicks in. The former behaviour was that the
block was removed as soon as the pending class was added. This fix
allows for styles to be applied in the pending class without causing
an animation to trigger prematurely.

---- DEMO -----
https://s3.amazonaws.com/angularjs-dev/ng-animate-piggyback/index.html

@IgorMinar
Copy link
Contributor

Should the mixed in style be removed once the animation is done?

I'm on cell so can't do proper review but from what I've see this looks good.

@matsko
Copy link
Contributor Author

matsko commented Sep 8, 2014

The styles should persist. If you look at the demo, you'll see that the goal is to place the box at the given coordinates. The animation that runs in between is just extra. If we removed the styles and if there is no animation or if ngAnimate is not activated then nothing would happen.

@tbosch
Copy link
Contributor

tbosch commented Sep 16, 2014

I don't like that the styles are added via addClass as a third parameter, and as @IgorMinar mentioned, that there is no way of removing them.

I would prefer an addStyles and removeStyles method. This shouldn't be a problem now as you collect all changes until the next turn of the digest loop, right?

@matsko matsko force-pushed the ng_animate_piggyback branch 3 times, most recently from 3ba919c to 3d88d51 Compare October 13, 2014 17:27
$animate now supports an optional parameter which provides CSS styling
which will be provided into the CSS-based animations as well as any
custom animation functions. Once the animation is complete then the
styles will be applied directly to the element. If no animation is
detected or the `ngAnimate` module is not active then the styles
will be applied immediately.

BREAKING CHANGE: staggering animations that use transitions will now
always block the transition from starting (via `transition: 0s none`)
up until the stagger step kicks in. The former behaviour was that the
block was removed as soon as the pending class was added. This fix
allows for styles to be applied in the pending class without causing
an animation to trigger prematurely.
@matsko matsko force-pushed the ng_animate_piggyback branch from 3d88d51 to a02f29c Compare October 13, 2014 17:28

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@matsko matsko force-pushed the ng_animate_piggyback branch from a02f29c to 09cc1dc Compare October 13, 2014 18:03
@IgorMinar
Copy link
Contributor

landed as e5f4d7b and 02be700

@IgorMinar IgorMinar closed this Oct 13, 2014
@caitp
Copy link
Contributor

caitp commented Oct 13, 2014

The problem with this, as has been mentioned previously, is that

  1. it adds totally unrelated functionality to a routine with a specific purpose, which is just not a very good API design (both for users, who will have a harder time finding out how to use this feature, and for maintenance, because it's more bits and pieces to reason about when fixing the inevitable bugs)
  2. it makes it harder to fix the "sometimes async, sometimes not" issue in setClass()

I think this could have been done better, this should not have shipped yet =\ now we can't even fix it without breaking changes u_u which means we can't really fix it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants