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

Hidden animated panels are visible on page load #12825

Open
Morhpeuss opened this issue Sep 11, 2015 · 20 comments
Open

Hidden animated panels are visible on page load #12825

Morhpeuss opened this issue Sep 11, 2015 · 20 comments

Comments

@Morhpeuss
Copy link

Hi,

I have hidden floating panel.
HTML:
<div class="modal animated left-modal " data-ng-hide="!showFormCreator" ng-cloak data-ng-controller="formCreatorController" data-ng-include="'/app/views/form-creator.html'"></div>

CSS:

.left-modal.ng-hide-add {      
     -webkit-animation:  fadeOutRight  0.5s ;   
    -moz-animation:   fadeOutRight 0.5s;
    -ms-animation:   fadeOutRight 0.5s;    
    animation:   fadeOutRight 0.5s;     
}

.left-modal.ng-hide-remove{
    -webkit-animation:  fadeInRight  .5s ;   
    -moz-animation:   fadeInRight .5s;
    -ms-animation:   fadeInRight .5s;    
    animation:   fadeInRight .5s;   
}

In angular 1.3.13 works all fine.

After update to 1.4.4 and now to 1.4.5 all hidden animated panels are visible on page load for few seconds.

After page load is all fine.

Thanks for help.

Josef

@Morhpeuss Morhpeuss changed the title Hidden animated panels visible on page load Hidden animated panels are visible on page load Sep 11, 2015
@Narretz
Copy link
Contributor

Narretz commented Sep 11, 2015

Can you please post a demo via plnkr.co or similar that shows the problem?

@Morhpeuss
Copy link
Author

Demo: http://plnkr.co/edit/xWkbSWkKu2fMfUhRdvsk . When the application starts, you will see the hidden animated panel.

@gkalpak
Copy link
Member

gkalpak commented Dec 7, 2015

A little more info:

This seems to have been introduced in 1.4.0-rc.0 (aka the big $animate refactoring).
I suspect it is the last breaking change from here, namely, the addClass animation is cancelled while an enter animation is in progress.

What I haven't figured out yet, is what causes the enter animation (but it is happening as you can see here).

@gkalpak
Copy link
Member

gkalpak commented Dec 7, 2015

Aha...it's the ngInclude, of course.

So, I am tempted to say that this works as expected (in the sense that it's a documented breaking change).
Still have to figure out the most appropriate work-around (suggestions welcome).

@gkalpak
Copy link
Member

gkalpak commented Dec 7, 2015

As is usually the case, moving the directive on a child element solves the issue. E.g.:

<!-- Change this: -->
<div ... ng-show="..." ng-include="...">Pane</div>

<!-- To that: -->
<div ... ng-show="..."><div ng-include="...">Pane></div></div>

Here is an updated plnkr.


So, like I said, I am inclined to calling this a works as expected.
WDYT, @Narretz, @matsko ?

@Narretz
Copy link
Contributor

Narretz commented Dec 7, 2015

Thanks for investigating @gkalpak!
I don't think it's a cancellation of the addClass animation because the ngInclude does not have actual animation code. But I haven't looked at what happens yet.

@gkalpak
Copy link
Member

gkalpak commented Dec 7, 2015

@Narretz, I'm not sure what you mean by "the ngInclude does not have actual animation code".
It does call $animate.enter() (which initiates a structural animation).

@Narretz
Copy link
Contributor

Narretz commented Dec 7, 2015

That's right, it initiates an animation but when no styles are found the
animation closes prematurely.
Am 07.12.2015 21:33 schrieb "Georgios Kalpakas" notifications@github.com:

@Narretz https://github.com/Narretz, I'm not sure what you mean by "the
ngInclude does not have actual animation code"
.
It does call $animate.enter() (which initiates a structural animation).


Reply to this email directly or view it on GitHub
#12825 (comment)
.

@gkalpak
Copy link
Member

gkalpak commented Dec 8, 2015

@Narretz, you are right; it's not enter cancelling addClass (they are merged actually). What makes the difference is that with ngInclude on the same element, the animations are enabled before ngShow tries to add the .ng-hide class for the first time (thus the animation).

Here's what is going on in both cases (with ngInclude on the same element and on a child element):

1. ngInclude on the same element as ngShow

<div ... ng-show="..." ng-include="...">...
  1. ngInclude transcludes the whole element (i.e. it removes it from the DOM and leaves a comment as a placeholder), before ngShow is compiled (since ngShow has a lower priority).
  2. ngInclude requests the template.
  3. By the time the template is back and the request promises are resolved, there have already been 2 $digests, thus animations are enabled (as per animateQueue.js#L113-L121).
  4. ngInclude creates a new element from the transcluded template element + the fetched HTML content, inserts it into the DOM and compiles/links it.
  5. ngShow on the newly created element, adds the .ng-hide class (using $animate.addClass()).
    Since animations are enabled, the user gets to see the element animating out of the view.

Compare this to:

2. ngInclude on a child element - ngShow on the parent

<div ... ng-show="..."><div ng-include="...">...
  1. ngInclude transcludes the child element (i.e. it removes it from the DOM and leaves a comment as a placeholder). Note that ngShow is not on the transcluded element, but on its parent.
  2. ngInclude requests the template.
  3. While the template is being fetched, ngShow's post-link function adds the .ng-hide class to the parent element (using $animate.addClass()).
  4. At this point, no request promises have been resolved yet, thus no two $digests have
    completed, which means animations are still disabled. With animations disabled, the .ng-hide class is added directly to the element's classes (without going through the ng-hide-animate/ng-hide-add/ng-hide-add-active hoops).
  5. By the time the template is back and ngInclude creates and inserts a new child element, the parent is already hidden (via .ng-hide).
  6. The user doesn't get to see the element animating out of the view.

That said, I still think this works as expected.
One could think of several workarounds, but having the directives on different elements is the simplest imo.

It remains to be investigated, why it did work prior to v1.4.0-rc.0 😃
Still looking into it.

@gkalpak
Copy link
Member

gkalpak commented Dec 8, 2015

Here is why it used to work prior to v1.4.0-rc.0:

Class-based animations were "blocked" on elements currently performing a structural animation (see animate.js#L517-L525). So, not allowing class-based animations while a structural animation (like enter) was in progress, meant that the .ng-hide class was added immediately (no ng-hide-animate/add/add-active ceremony) - effectively hiding the element as soon as it was inserted into the DOM by ngInclude.

After all, allowing class-based animations along with structural ones is a feature (not a bug) 😃

@gkalpak
Copy link
Member

gkalpak commented Dec 8, 2015

If anyone's interested, here are two demos:

@Narretz
Copy link
Contributor

Narretz commented Dec 8, 2015

Interesting. In this comment #11492 (comment), Matias said that animate would wait for the templates to download ...

@Narretz
Copy link
Contributor

Narretz commented Dec 8, 2015

As a workaround, you can put your templates into the templateCache when your module runs: http://plnkr.co/edit/Q6o13RNXjdRtmw8LW67W?p=preview

There are also build tools that do this automatically for you, e.g. https://github.com/karlgoldstein/grunt-html2js

@gkalpak
Copy link
Member

gkalpak commented Dec 8, 2015

In this comment #11492 (comment), Matias said that animate would wait for the templates to download ...

Matias is right (obviously 😃). It (sort of) does. That's why there is no animation when ngShow is applied before downloading the template (thus no flicker), but there is an animation when ngShow is applied after downoading the template.

@Narretz
Copy link
Contributor

Narretz commented Dec 9, 2015

The fact that it works (at least visually) when you add the template to the cache when the app runs, makes me think there is still a difference. Because template fetching is still async, even if the template is cached. It looks more like a timing issue.

@Narretz
Copy link
Contributor

Narretz commented Dec 9, 2015

There's definitely something strange going on: ng-animate is adding these bogus classes on the initial hide / enter animation: ng-ng-hide-add ng-ng-hide-add-active. structural animations get this ng prefix added. So it looks like the class based animation is mistaken for a structural animation. Not sure if that's the cause of this problem, but it's definitely a bug.

Forked example with classList log: http://plnkr.co/edit/FDEAbvvjVJzZccuZWMwY?p=preview

@Narretz Narretz self-assigned this Dec 9, 2015
matsko added a commit to matsko/angular.js that referenced this issue Dec 9, 2015
@gkalpak
Copy link
Member

gkalpak commented Dec 11, 2015

Yes, I've noticed it. This happens because the (non-structural) addClass('ng-hide') animation gets merged into the (structural) enter animation (which is already in progress) and in some sense "inherits" the isStructural: true. Thus there are also the ng-prefixed classes added (along with the expected ng-hide-* classes).

It would be nice to fix that, but I don't think it is causing any issues.

@Narretz
Copy link
Contributor

Narretz commented Dec 11, 2015

I've talked with @matsko about this and he said he would do some investigating. It's expected that the animations are merged, but the way they are merged doesn't look right.

The actual problem in the issue is caused by the way ngAnimate blocks animations on bootstrap. It waits two digest until animations are enabled. ngInclude calls $enter inside a watchActionFn, so it's happenind one digest later. We could delay animations further, but I'm not sure if that won't break some expectations. However, if you put it in the templateCache (best practice anyway) it doesn't animate, so it's probably more consistent to wait another digest.

@gkalpak
Copy link
Member

gkalpak commented Dec 11, 2015

The way $animate detects when the templates have been fetched and should enable animations is not 100% reliable. For example, because ngInclude calls $templateCache inside a watch-action callback and the src expression has to be evaluated first, $templateCache.totalPendingRequests is 0 when $animate checks, but not all (necessary) templates have been fetched.

That being said, I don't think it is reasonable to change how $animate detects "stability", because:

  1. It will break a lot of apps (and it will be very difficult for people to determine the source of the problem).
  2. I don't think there is a 100% reliable way to detect when the view is stable (and it heavily depends on the directives used and their arbitrary behaviors).

The current mechanism, tries to account for route and directive templates and it does it quite well.
It doesn't try to account for templates that directives might request asynchronously after evaluating an expression inside a $watch (which is what ngInclude does). Directives having that kind of behavior (and people using them) should be responsible for the side-effects of such behavior.

They can put them into wrappers, load templates into the $templateCache as a build step or whatever works for their usecase.

@petebacondarwin petebacondarwin modified the milestones: 1.4.9, 1.4.10 Jan 21, 2016
@petebacondarwin petebacondarwin modified the milestones: 1.4.10, 1.5.1 Jan 28, 2016
@Narretz Narretz modified the milestones: 1.5.1, 1.5.x Feb 15, 2016
@NeoGenet1c
Copy link

Quick (& dirty) workaround is to add ng-hide class to the element.

@Narretz Narretz modified the milestones: 1.5.x, 1.6.x Mar 31, 2017
@Narretz Narretz modified the milestones: 1.6.x, 1.7.x Apr 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants