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

ng-show sometimes does not remove ng-hide class after expression becames truthy #13380

Closed
tymekg opened this issue Nov 25, 2015 · 11 comments
Closed

Comments

@tymekg
Copy link

tymekg commented Nov 25, 2015

I've noticed that in my application ng-show sometimes leaves ng-hide class even after ng-show expression evaluates to true.

This behavior is not consistent across browsers: In places where I see issues on Chrome it looks fine on FF, while in other place the situation is opposite.

Even if I retry the same scenario on a single browser, the defect does not occur every time, so this must be some non-deterministic timing issue.

Scenario & recreation

I've tried to isolate the problem and stripped my app to this simple plunker:
http://plnkr.co/edit/pNUXB6A0gCyfdej8k5Xu?p=preview
I am able to recreate the issue there with my FF ESR 38.4.0, but not every time.
I'm clicking the button and if the modal appears with both lines of text I need to click Stop/Run in Plunker and try again, for several times. My teammate who is using exactly the same FF version cannot recreate it at all.

Some more observations:

  • Manually changing scope.loadedData (the watched expression there) to a falsy value and then back to the original value causes the div to show up.
  • Calling scope.$digest() manually does not help
  • replacing ng-show with additional condition in ngClass ('ng-hide': !loadedData) does not help either
  • using ng-if instead of ng-show works fine
  • debugging reveals that ng-show directive's watch is triggered properly. It calls $animate to remove ng-hide class, but it doesn't take effect.
  • ng-show alone works fine, the issue occurs on the element which has both ng-show and ng-class directives

Affected versions

This is a regression introduced in Angular 1.4.0 (recreated this defect on 1.4.0, 1.4.7, 1.4.8 and 1.5.0-beta.2, but It did not occur on 1.3.20)

@frfancha
Copy link

I observe the same, especially: ng-show alone works fine, the issue occurs on the element which has both ng-show and ng-class directives
A by-pass could then be to let ng-show alone on a div containing the div with ng-class.

@Narretz
Copy link
Contributor

Narretz commented Nov 26, 2015

Interesting bug. It ultimately seems to be caused by an interplay of the modal directive and the way ngAnimate handles consecutive animations on the same element. In my tests, the problem will only occur the first time you open the modal. The request for the modal template (details.html) seems to cause a timing issue in some cases, but not always, which causes ngAnimate to join the addClass and removeClass animations, because the addClass animation is still cached on the object, even though it has been executed already.

As a workaround, you can try to put your modal templates into the $templateCache when your application runs. There are grunt plugins for that for example: https://github.com/karlgoldstein/grunt-html2js

@tymekg
Copy link
Author

tymekg commented Nov 30, 2015

I confirm that I was able to recreate in plunker only on the first time. Further attempts of opening modal always work as expected.

On the other hand, in my application where I first noticed this issue it works differently. I can open one of my modals multiple times (without reloading the page) and the issue still occurs.
Maybe I have removed something relevant to the case when isolating the problem into plunker.

Thanks for suggesting grunt-html2js, I'll give it a try.
I don't expect it to fix the issue in my original case (as noted above, it is not only a matter of first modal appearance), but it looks like a reasonable performance improvement anyway.

@Narretz
Copy link
Contributor

Narretz commented Nov 30, 2015

It would still be interesting if you could create a repro where it fails every time, since we have to create a test for this problem, and it's difficult if there's no 100% repro.

Do you make a network request for the template every time the modal opens or just the first time? Maybe there's more going on in your application at the moment the modal opens?

@tymekg
Copy link
Author

tymekg commented Nov 30, 2015

I don't do anything explicitly in my app to fetch the template every time, so the default angular behavior works there - template is fetched only on the first time and cached afterwards. I've just double checked that in Firebug console.

I do however fetch some additional data via $http request to my REST service in the beginning of my modal controller. The fetched data is then assigned to scope variables which are used in modal template. One of those variables is used in ng-show condition, which is supposed display part of the modal only after data has been fetched.

In the provided plunker I tried to simulate that $http request with a $timeout which assigns value to $scope.loadedData.

@tymekg
Copy link
Author

tymekg commented Nov 30, 2015

I made a small update to the plunker: changed delay in $timeout from 50 ms to 0ms. Now it is 100% recreatable:
http://plnkr.co/edit/vWqR5G1AHE38s4D1rI7Z?p=preview
This time the first appearance of modal works fine (scope variable is updated before template gets fetched) but every further attempt is affected by the defect.

Note: this is still recreatable in FF only (I'm using 38.4.0 ESR). In my Chrome and IE the plunker works as expected.

Narretz added a commit to Narretz/angular.js that referenced this issue Nov 30, 2015
…ined animations

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

Fixes angular#13380
@tymekg
Copy link
Author

tymekg commented Nov 30, 2015

I have patched angular-animate.js locally with commit 5713231 and it also fixes the issue in my application. 👍

@Narretz
Copy link
Contributor

Narretz commented Nov 30, 2015

Good stuff! We'll have to double-check if that isn't breaking other stuff. I totally read over the fact that this is mainly a FF issue. Seems to be an implementation issue with how a requestAnimationFrame is triggered.

@tymekg
Copy link
Author

tymekg commented Dec 1, 2015

It's not only a FF issue. It seems to be very tricky with browsers.

The original issue found in my app was observable in Chrome only. Then I tried to isolate a test case and created the plunker example.. and to my surprise I was able to recreate the issue with plunker on FF only.

So it seems that the issue affects different browsers, depending on case. However the root cause seems to be the same, because your fix also fixes the issue I observed in my app on Chrome.

@Narretz Narretz modified the milestones: 1.4.9, 1.5.0-rc.0 Dec 2, 2015
Narretz added a commit to Narretz/angular.js that referenced this issue Dec 2, 2015
…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
…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
…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
…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
@petebacondarwin petebacondarwin modified the milestones: 1.5.0-rc.1, 1.5.0-rc.0 Dec 9, 2015
Narretz added a commit to Narretz/angular.js that referenced this issue Dec 9, 2015
…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
…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
…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 added a commit to Narretz/angular.js that referenced this issue Jan 5, 2016
…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
…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
…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
@jakub-g
Copy link

jakub-g commented Feb 8, 2016

I think I experienced exactly this bug using the following setup:

ionic 1.2.1-nightly-1867 (ionic 1.2.1 stable bower name)
angular 1.4.3
angular-animate 1.4.3

it's 100% reproducible in Chrome/Android and Chrome/Windows in a a timeout-scenario but not reproducible at all without timeout.

Using ng-if instead seems to solve the bug.

@Narretz
Copy link
Contributor

Narretz commented Feb 8, 2016

@jakub-g This should be fixed in latest 1.4 (1.4.9). You are using an outdated version. If the problem persists in the new version, please open a new issue.

@angular angular locked and limited conversation to collaborators Feb 8, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.