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

ngAnimate doesn't remove class which is animating #13339

Closed
maxaon opened this issue Nov 18, 2015 · 3 comments
Closed

ngAnimate doesn't remove class which is animating #13339

maxaon opened this issue Nov 18, 2015 · 3 comments
Assignees
@Narretz
Labels
component: ngAnimate frequency: low severity: broken expected use type: bug
Milestone
1.5.0-rc.1

Comments

@maxaon
Copy link

maxaon commented Nov 18, 2015

Removing class which is currently animating doesn't work.

Angular Version: since 1.4.5 till 1.5.0-build.4389
Browser: Any
Reproducable: almost always (sometimes doesn't reproduce under debugging)
Operating system: Any

Detail
For the example of bug see plunker
By normal it should not have class pending at the end.

What happens:

  1. Adding class pending
  2. NgModel adds class ng-valid-required (it's joining animation and applies preparation class)
  3. Timeout is fullfilled
  4. Trying to remove class.
    Inside ngAnimations it checks that element has animations, it cannot be cancelled and skipped, so it's try to join animation. Because element is in running state it tries to normalize options. Function normalize options cannot find class pending and remove it from options. Remove animation exits as invalid.
  5. Class pending added to dom node
@Narretz
Copy link
Contributor

Narretz commented Nov 19, 2015

Very interesting. Is the use of attrs.$addClass necessary? You could otherwise simply use element.addClass and the bug does not appear: http://plnkr.co/edit/k8KBsZ3hlXgC0QsL2eAB?p=preview

@Narretz Narretz added this to the Backlog milestone Nov 19, 2015
@maxaon
Copy link
Author

maxaon commented Nov 20, 2015

Yes, as a temporary solution, we do not use animations, but nevertheless adding animation in this situation is problematic.

@Narretz
Copy link
Contributor

Narretz commented Dec 2, 2015

This will be fixed by #13414, see http://plnkr.co/edit/ldW3OBDahDbiK5C2bnzP?p=preview

@Narretz Narretz modified the milestones: 1.5.0-rc.0, Backlog Dec 2, 2015
Narretz added a commit to Narretz/angular.js that referenced this issue Dec 2, 2015
@Narretz
fix(ngAnimate): don't normalize classes on follow-up animations to jo…
5328ccd 
…ined animations

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
matsko added a commit to matsko/angular.js that referenced this issue Dec 9, 2015
@matsko
fix(ngAnimate): don't normalize classes on follow-up animations to jo…
4cfbae9 
…ined animations

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
matsko pushed a commit to matsko/angular.js that referenced this issue Dec 9, 2015
@Narretz @matsko
fix(ngAnimate): don't normalize classes on follow-up animations to jo…
31927ef 
…ined animations

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
@matsko matsko mentioned this issue Dec 9, 2015
Closed
matsko pushed a commit to matsko/angular.js that referenced this issue Dec 9, 2015
@Narretz @matsko
fix(ngAnimate): don't normalize classes on follow-up animations to jo…
dd45a48 
…ined animations

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
@petebacondarwin petebacondarwin modified the milestones: 1.5.0-rc.0, 1.5.0-rc.1 Dec 9, 2015
Narretz added a commit to Narretz/angular.js that referenced this issue Dec 9, 2015
@Narretz
fix(ngAnimate): don't normalize classes on follow-up animations to jo…
b07d30b 
…ined animations

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
@Narretz Narretz self-assigned this Dec 9, 2015
Narretz added a commit to Narretz/angular.js that referenced this issue Jan 5, 2016
@Narretz
fix(ngAnimate): don't normalize classes on follow-up animations to jo…
83832bf 
…ined animations

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
Narretz added a commit to Narretz/angular.js that referenced this issue Jan 5, 2016
@Narretz
fix(ngAnimate): allow removing classes that are added by a running an…
ff19137 
…imation

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
@Narretz Narretz mentioned this issue Jan 5, 2016
Closed
Narretz added a commit to Narretz/angular.js that referenced this issue Jan 5, 2016
@Narretz
fix(ngAnimate): allow removing classes that are added by a running an…
4c6567e 
…imation

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
Closes angular#13472
Closes angular#13678
Narretz added a commit to Narretz/angular.js that referenced this issue Jan 5, 2016
@Narretz
fix(ngAnimate): allow removing classes that are added by a running an…
d24910b 
…imation

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
Closes angular#13472
Closes angular#13678
@Narretz Narretz closed this as completed in 776972e Jan 5, 2016
Narretz added a commit that referenced this issue Jan 6, 2016
@Narretz
fix(ngAnimate): allow removing classes that are added by a running an…
6c4581f 
…imation

This allows follow-up animations to remove a class that is currently
being added.

Fixes #13339
Fixes #13380
Closes #13414
Closes #13472
Closes #13678
kkirsche added a commit to kkirsche/kibana that referenced this issue Feb 21, 2016
@kkirsche
Update AngularJS
d64590c 
<a name="1.4.9"></a>
# 1.4.9 implicit-superannuation (2016-01-21)

## Bug Fixes

- **Animation**
  - ensure that animate promises resolve when the document is hidden
  ([9a60408c](angular/angular.js@9a60408))
  - do not trigger animations if the document is hidden
  ([09f6061a](angular/angular.js@09f6061),
   [elastic#12842](angular/angular.js#12842), [elastic#13776](angular/angular.js#13776))
  - only copy over the animation options once
  ([2fc954d3](angular/angular.js@2fc954d),
   [elastic#13722](angular/angular.js#13722), [elastic#13578](angular/angular.js#13578))
  - allow event listeners on document in IE
  ([5ba4419e](angular/angular.js@5ba4419),
   [elastic#13548](angular/angular.js#13548), [elastic#13696](angular/angular.js#13696))
  - allow removing classes that are added by a running animation
  ([6c4581fc](angular/angular.js@6c4581f),
   [elastic#13339](angular/angular.js#13339), [elastic#13380](angular/angular.js#13380), [elastic#13414](angular/angular.js#13414), [elastic#13472](angular/angular.js#13472), [elastic#13678](angular/angular.js#13678))
  - do not use `event.timeStamp` anymore for time tracking
  ([620a20d1](angular/angular.js@620a20d),
   [elastic#13494](angular/angular.js#13494), [elastic#13495](angular/angular.js#13495))
  - ignore children without animation data when closing them
  ([be01cebf](angular/angular.js@be01ceb),
   [elastic#11992](angular/angular.js#11992), [elastic#13424](angular/angular.js#13424))
  - do not alter the provided options data
  ([7a81e6fe](angular/angular.js@7a81e6f),
   [elastic#13040](angular/angular.js#13040), [elastic#13175](angular/angular.js#13175))
  - correctly handle `$animate.pin()` host elements
  ([a985adfd](angular/angular.js@a985adf),
   [elastic#13783](angular/angular.js#13783))
  - allow animations when pinned element is parent element
  ([4cb8ac61](angular/angular.js@4cb8ac6),
   [elastic#13466](angular/angular.js#13466))
  - allow enabled children to animate on disabled parents
  ([6d85f24e](angular/angular.js@6d85f24),
   [elastic#13179](angular/angular.js#13179), [elastic#13695](angular/angular.js#13695))
  - correctly access `minErr`
  ([0c1b54f0](angular/angular.js@0c1b54f))
  - ensure animate runner is the same with and without animations
  ([937942f5](angular/angular.js@937942f),
   [elastic#13205](angular/angular.js#13205), [elastic#13347](angular/angular.js#13347))
  - remove animation end event listeners on close
  ([d9157849](angular/angular.js@d915784),
   [elastic#13672](angular/angular.js#13672))
  - consider options.delay value for closing timeout
  ([592bf516](angular/angular.js@592bf51),
   [elastic#13355](angular/angular.js#13355), [elastic#13363](angular/angular.js#13363))
- **$controller:** allow identifiers containing `$`
  ([2563ff7b](angular/angular.js@2563ff7),
   [elastic#13736](angular/angular.js#13736))
- **$http:** throw if url passed is not a string
  ([c5bf9dae](angular/angular.js@c5bf9da),
   [elastic#12925](angular/angular.js#12925), [elastic#13444](angular/angular.js#13444))
- **$parse:** handle interceptors with `undefined` expressions
  ([7bb2414b](angular/angular.js@7bb2414))
- **$resource:** don't allow using promises as `timeout` and log a warning
  ([47486524](angular/angular.js@4748652))
- **formatNumber:** cope with large and small number corner cases
  ([9c49eb13](angular/angular.js@9c49eb1),
   [elastic#13394](angular/angular.js#13394), [elastic#8674](angular/angular.js#8674), [elastic#12709](angular/angular.js#12709), [elastic#8705](angular/angular.js#8705), [elastic#12707](angular/angular.js#12707), [elastic#10246](angular/angular.js#10246), [elastic#10252](angular/angular.js#10252))
- **input:**
  - fix URL validation being too strict
  ([6610ae81](angular/angular.js@6610ae8),
   [elastic#13528](angular/angular.js#13528), [elastic#13544](angular/angular.js#13544))
  - add missing chars to URL validation regex
  ([2995b54a](angular/angular.js@2995b54),
   [elastic#13379](angular/angular.js#13379), [elastic#13460](angular/angular.js#13460))
- **isArrayLike:** recognize empty instances of an Array subclass
  ([323f9ab7](angular/angular.js@323f9ab),
   [elastic#13560](angular/angular.js#13560), [elastic#13708](angular/angular.js#13708))
- **ngInclude:** do not compile template if original scope is destroyed
  ([9590bcf0](angular/angular.js@9590bcf))
- **ngOptions:**
  - don't skip `optgroup` elements with `value === ''`
  ([85e392f3](angular/angular.js@85e392f),
   [elastic#13487](angular/angular.js#13487), [elastic#13489](angular/angular.js#13489))
  - don't `$dirty` multiple select after compilation
  ([f163c905](angular/angular.js@f163c90),
   [elastic#13211](angular/angular.js#13211), [elastic#13326](angular/angular.js#13326))
- **select:** re-define `ngModelCtrl.$render` in the `select` directive's postLink function
  ([529b2507](angular/angular.js@529b250),
   [elastic#13583](angular/angular.js#13583), [elastic#13583](angular/angular.js#13583), [elastic#13663](angular/angular.js#13663))

## Minor Features

- **ngLocale:** add support for standalone months
  ([54c4041e](angular/angular.js@54c4041),
   [elastic#3744](angular/angular.js#3744), [elastic#10247](angular/angular.js#10247), [elastic#12642](angular/angular.js#12642), [elastic#12844](angular/angular.js#12844))
- **ngMock:** add support for `$animate.closeAndFlush()`
  ([512c0811](angular/angular.js@512c081))

## Performance Improvements

- **ngAnimate:** speed up `areAnimationsAllowed` check
  ([2d3303dd](angular/angular.js@2d3303d))

## Breaking Changes

While we do not deem the following to be a real breaking change we are highlighting it here in the
changelog to ensure that it does not surprise anyone.

- **$resource:** due to [47486524](angular/angular.js@4748652),

**Possible breaking change** for users who updated their code to provide a `timeout`
promise for a `$resource` request in version v1.4.8.

Up to v1.4.7 (included), using a promise as a timeout in `$resource`, would silently
fail (i.e. have no effect).

In v1.4.8, using a promise as timeout would have the (buggy) behaviour described
in angular/angular.js#12657 (comment).
(I.e. it will work as expected for the first time you resolve the promise and will
cancel all subsequent requests after that - one has to re-create the resource
class. This was not documented.)

With this change, using a promise as timeout in v1.4.9 onwards is not allowed.
It will log a warning and ignore the timeout value.

If you need support for cancellable `$resource` actions, you should upgrade to
version 1.5 or higher.

<a name="1.4.8"></a>
# 1.4.8 ice-manipulation (2015-11-19)

## Bug Fixes

- **$animate:** ensure leave animation calls `close` callback
  ([6bd6dbff](angular/angular.js@6bd6dbf),
   [elastic#12278](angular/angular.js#12278), [elastic#12096](angular/angular.js#12096), [elastic#13054](angular/angular.js#13054))
- **$cacheFactory:** check key exists before decreasing cache size count
  ([2a5a52a7](angular/angular.js@2a5a52a),
   [elastic#12321](angular/angular.js#12321), [elastic#12329](angular/angular.js#12329))
- **$compile:**
  - bind all directive controllers correctly when using `bindToController`
  ([5d8861fb](angular/angular.js@5d8861f),
   [elastic#11343](angular/angular.js#11343), [elastic#11345](angular/angular.js#11345))
  - evaluate against the correct scope with bindToController on new scope
  ([b9f7c453](angular/angular.js@b9f7c45),
   [elastic#13021](angular/angular.js#13021), [elastic#13025](angular/angular.js#13025))
  - fix scoping of transclusion directives inside replace directive
  ([74da0340](angular/angular.js@74da034),
   [elastic#12975](angular/angular.js#12975), [elastic#12936](angular/angular.js#12936), [elastic#13244](angular/angular.js#13244))
- **$http:** apply `transformResponse` even when `data` is empty
  ([c6909464](angular/angular.js@c690946),
   [elastic#12976](angular/angular.js#12976), [elastic#12979](angular/angular.js#12979))
- **$location:** ensure `$locationChangeSuccess` fires even if URL ends with `#`
  ([6f8ddb6d](angular/angular.js@6f8ddb6),
   [elastic#12175](angular/angular.js#12175), [elastic#13251](angular/angular.js#13251))
- **$parse:** evaluate once simple expressions only once
  ([e4036824](angular/angular.js@e403682),
   [elastic#12983](angular/angular.js#12983), [elastic#13002](angular/angular.js#13002))
- **$resource:** allow XHR request to be cancelled via a timeout promise
  ([7170f9d9](angular/angular.js@7170f9d),
   [elastic#12657](angular/angular.js#12657), [elastic#12675](angular/angular.js#12675), [elastic#10890](angular/angular.js#10890), [elastic#9332](angular/angular.js#9332))
- **$rootScope:** prevent IE9 memory leak when destroying scopes
  ([87b0055c](angular/angular.js@87b0055),
   [elastic#10706](angular/angular.js#10706), [elastic#11786](angular/angular.js#11786))
- **Angular.js:** fix `isArrayLike` for unusual cases
  ([70edec94](angular/angular.js@70edec9),
   [elastic#10186](angular/angular.js#10186), [elastic#8000](angular/angular.js#8000), [elastic#4855](angular/angular.js#4855), [elastic#4751](angular/angular.js#4751), [elastic#10272](angular/angular.js#10272))
- **isArrayLike:** handle jQuery objects of length 0
  ([d3da55c4](angular/angular.js@d3da55c))
- **jqLite:**
  - deregister special `mouseenter` / `mouseleave` events correctly
  ([22f66025](angular/angular.js@22f6602),
   [elastic#12795](angular/angular.js#12795), [elastic#12799](angular/angular.js#12799))
  - ensure mouseenter works with svg elements on IE
  ([c1f34e8e](angular/angular.js@c1f34e8),
   [elastic#10259](angular/angular.js#10259), [elastic#10276](angular/angular.js#10276))
- **limitTo:** start at 0 if `begin` is negative and exceeds input length
  ([4fc40bc9](angular/angular.js@4fc40bc),
   [elastic#12775](angular/angular.js#12775), [elastic#12781](angular/angular.js#12781))
- **merge:**
  - ensure that jqlite->jqlite and DOM->DOM
  ([2f8db1bf](angular/angular.js@2f8db1b))
  - clone elements instead of treating them like simple objects
  ([838cf4be](angular/angular.js@838cf4b),
   [elastic#12286](angular/angular.js#12286))
- **ngAria:** don't add tabindex to radio and checkbox inputs
  ([59f1f4e1](angular/angular.js@59f1f4e),
   [elastic#12492](angular/angular.js#12492), [elastic#13095](angular/angular.js#13095))
- **ngInput:** change URL_REGEXP to better match RFC3987
  ([cb51116d](angular/angular.js@cb51116),
   [elastic#11341](angular/angular.js#11341), [elastic#11381](angular/angular.js#11381))
- **ngMock:** reset cache before every test
  ([91b7cd9b](angular/angular.js@91b7cd9),
   [elastic#13013](angular/angular.js#13013))
- **ngOptions:**
  - skip comments and empty options when looking for options
  ([0f58334b](angular/angular.js@0f58334),
   [elastic#12190](angular/angular.js#12190), [elastic#13029](angular/angular.js#13029), [elastic#13033](angular/angular.js#13033))
  - override select option registration to allow compilation of empty option
  ([7b2ecf42](angular/angular.js@7b2ecf4),
   [elastic#11685](angular/angular.js#11685), [elastic#12972](angular/angular.js#12972), [elastic#12968](angular/angular.js#12968), [elastic#13012](angular/angular.js#13012))

## Performance Improvements

- **$compile:** use static jquery data method to avoid creating new instances
  ([55ad192e](angular/angular.js@55ad192))
- **copy:**
  - avoid regex in `isTypedArray`
  ([19fab4a1](angular/angular.js@19fab4a))
  - only validate/clear if the user specifies a destination
  ([d1293540](angular/angular.js@d129354),
   [elastic#12068](angular/angular.js#12068))
- **merge:** remove unnecessary wrapping of jqLite element
  ([ce6a96b0](angular/angular.js@ce6a96b),
   [elastic#13236](angular/angular.js#13236))

## Breaking Changes

Signed-off-by: Kevin Kirsche <[email protected]>
@kkirsche kkirsche mentioned this issue Feb 21, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Narretz Narretz

Labels
component: ngAnimate frequency: low severity: broken expected use type: bug
Projects
None yet
Milestone
1.5.0-rc.1
Development

No branches or pull requests
3 participants
@petebacondarwin @Narretz @maxaon