fix(ngAnimate): class-based animations must set the addClass/removeClass class on the element #11853
Conversation
| @@ -103,6 +103,14 @@ describe("ngAnimate $$animateCssDriver", function() { | |||
| driver({ element: element }); | |||
| expect(capturedAnimation[1].applyClassesEarly).toBeFalsy(); | |||
| })); | |||
|
|
|||
| it("should not provide a method into the $animateCss animation if the animation is not structural", inject(function() { | |||
There was a problem hiding this comment.
"provide ... into" doesn't sound right somehow.
There was a problem hiding this comment.
it should not provide a method as an option value if the animation is not structural
matsko
force-pushed
the
fix_add_class_method_name
branch
from
b1a933e to
20b509d
Compare
…ass class on the element With the abstraction system that ngAnimate uses, $animateCss internally sets the provided `event` as a CSS class on the element. In this situation the `addClass` and `removeClass` events on the element as a CSS class. This should not happen with class-based animations and this feature is unnecessary and has now been removed. Closes angular#11810
matsko
force-pushed
the
fix_add_class_method_name
branch
from
20b509d to
5f0f82d
Compare
| @@ -104,6 +104,14 @@ describe("ngAnimate $$animateCssDriver", function() { | |||
| driver({ element: element }); | |||
| expect(capturedAnimation[1].applyClassesEarly).toBeFalsy(); | |||
| })); | |||
|
|
|||
| it("should not provide a `method` as an option value if the animation is not structural", inject(function() { | |||
There was a problem hiding this comment.
I think we could simplify this by changing it "should only provide ... if the animation is structural". What still confuses me, is that "method" doesn't really reflect what is set. It's really the event (name) that is set, right?
There was a problem hiding this comment.
I agree with @Narretz here. Let's change this to:
it("should only set the event name if the animation is structural", inject(function() {Also the previous it clause should probably change:
it("should signal $animateCss to apply the classes early when animation is structural", inject(function() {
|
I think the commit message ought to be
|
|
Apart from tweaks to test descriptions this LGTM |
|
Fixed up and merged as 3a3db69 |
With the abstraction system that ngAnimate uses, $animateCss internally
sets the provided
eventas a CSS class on the element. In thissituation the
addClassandremoveClassevents on the element as aCSS class. This should not happen with class-based animations and this
feature is unnecessary and has now been removed.
Closes #11810