$animateCss used on a directive that uses a templateUrl breaks #14074
Comments
|
I also see this error in my own project return (c.$$ngAnimateParentKey || (c.$$ngAnimateParentKey = ++K)) + "-" + a.getAttribute("class") + "-" + b |
|
Thanks for reporting. Can you please create a plnkr that repdroduces the error you mentioned? |
|
There is a plnkr included in the original story, here it is again. Let me know if this one doesn't work. |
|
@CodySchaaf You said you have an error in your project. I was specifically asking for a repro for that error, because your posted plnkr doesn't throw an error. |
|
Also, as far as I can see, the behavior in 1.4.9 and 1.5.0 is the same: no animation with templateUrl. |
|
So I doubt this worked in 1.4.x, because this inherently happens because with templateUrl, it takes at least a few milliseconds until the template is available and the direcitve is linked. But the animation runs before that happens, so there's nothing to animate. This is a known limitation. You can fix this by putting the template in the cache: https://plnkr.co/edit/o7SyQNY5oMZxAKVh7Eeg?p=preview |
|
Oh that makes sense. Sorry. I will try to get a working repro for that. I suspect it has to do with angular attempting to preform some clean up action on the element that expects it to have a parentNode since c is Looks like you are correct with 1.4.9 having similar behavior in the plnkr, the issue in my application was not present in 1.4.9 though so I will dig deeper. Thanks for your help. |
|
Sounds like there might be a different issue then, thanks for your help. It definitely worked in app before the upgrade, so maybe it is just timing changes that have surfaced this issue now with 1.5. I'll report back if I can get a plnkr that is broken in 1.4.9 but working in 1.5.0. Probably wouldn't be an issue in production since the templates are cached there, but we don't currently cache them in development (might be a good time to get the environments in sync). |
|
1.5.0 includes lazy compilation, which can cause subtle issues. It's only active when the directive uses transclusion, though (and should not cause differences between template / templateUrl).. |
|
@Narretz I was able to make a plnkr that works on 1.4.9 but is broken on 1.5.0 1.5.0 Just click ready and you'll see it change color and scroll (if screen is less than 1000px tall) on 1.4.9 but not on 1.5.0 You'll also see the template is loaded before you hit ready on 1.4.9 but not until after 1.5.0 (view the network requests in the console). This is probably because of the lazy compile, although there is no transclusion taking place, so I'm not sure why. |
|
Also here is an example with the error, you need to have debugging enabled through the $logProvider, as well as an animation stagger. |
|
For the first problem - |
|
@dcherman Here's another issue I'd like your opinion on. https://plnkr.co/edit/uP3HxlLpn9Gyzs3mFKLT?p=preview In this case the lazy compilation introduces a timing problem that breaks enter animations. There's an element with ngIf that has another directive that has a templateUrl. |
|
@Narretz That timing bug always existed actually. If you invert the condition and have I'm still thinking if there's anything that we can do in order to avoid this in the general case since causing eager compilation won't actually fix the bug here. |
|
I was able to repro pretty consistently yesterday in chrome, but I might have been doing something weird with the debugger. It seems to be less consistent today though. If you bump up the stagger index, and stagger, you can get a more consistent repro though, although not perfect. |
|
@CodySchaaf Ok, now the error pops up more often. I can't reproduce it in Canary or FF though, so it might be a Chrome bug that is going to be fixed in the future. @dcherman Oh, yeah that makes sense. I guess this limitation now causes issues more often because of lazy compilation. Would be cool to fix this directly in the compiler, although I can't say I see how. |
|
@Narretz I've got some stuff to ship this week so I doubt I'll really be able to do much looking into this (unless I somehow find free time after work) until at least next week. My initial curiosity on this is that we're replacing an element that has already clearly been cloned via transclusion at this point, and does that really need to happen? It seems like this element is inappropriately being replaced, so that's where I was going to start looking, starting with just seeing which (if any) unit tests break with that code removed for this specific scenario before really considering how to refactor the condition for replacement and what side effects that might have. |
|
@dcherman No probelm. It's not high priority. I was wondering about the replacement issue too.Maybe we can optimize this. |
|
Since this prevents me from being able to upgrade from 1.4.* do you think I should focus on finding a work around, or is there a chance it will we looked at in the near-ish future? I know it probably isn't a high priority, so just wanted to get a time frame. Thanks for all you guys do! |
|
@CodySchaaf I'll probably won't be able to look into it for some time. It's also pretty complex, so I doubt there's an easy solution, unfortunately. |
|
I've also been running into this problem when upgrading from 1.2.x (yes, I know) to 1.5.2. https://plnkr.co/edit/okAirW34LQ8uCYzR7Ijf?p=preview When you click 'Add message' the first one is not animated as it's not transcluded yet. All after that are fine. In this screenshot you can clearly see that the element is not yet transcluded:
This issue is the root cause I guess: #13884 I think there should at least be something about this in the release notes if this is not going to be fixed. Maybe even a note in the animation docs although it's not exclusive to that. |
For transcluded directives, the transclude function can be lazily compiled most of the time since the contents will not be needed until the `transclude` function was actually invoked. For example, the `transclude` function that is passed to `ng-if` or `ng-switch-when` does not need to be invoked until the condition that it's bound to has been matched. For complex trees or switch statements, this can represent significant performance gains since compilation of branches is deferred, and that compilation may never actually happen if it isn't needed. There are two instances where compilation will not be lazy; when we scan ahead in the array of directives to be processed and find at least two of the following: * A directive that is transcluded and does not allow multiple transclusion * A directive that has templateUrl and replace: true * A directive that has a template and replace: true In both of those cases, we will need to continue eager compilation in order to generate the multiple transclusion exception at the correct time.
|
Alright, after reading the docs here: https://docs.angularjs.org/api/ng/service/$compile#-templateurl- I do think this can be deemed expected behaviour. You guys could consider to try to wait for the compile to finish somewhere in the angular-animate code. |
|
@michi88, your issue is the same as #15510 (and might be slightly different (or a subset) of that from the OP). This comment applies to you too. tl;dr I just found this issue and I believe I have accidentally addressed the problem caused by replacing the node during lazy compilation in #15514 (as far as That said, I think there might be another (possibly similar) bug related to staggering (also mentioned in #14124). I'll try to look into it in the following days 😃 |
Previously, `$animate` would decide whether an animation should be cancelled based on some assumption that didn't hold in specific cases (e.g. when animating transcluded clones with `templateUrl` directives on them for the first time). As a result, the entering elements would not be animated in such cases. This affected commonly used, structural built-in directives (`ngIf`, `ngRepeat`, `ngSwitch` etc). This commit fixes it by avoiding invalid assumptions (i.e. by taking into account the transformations that take place while compiling such elements). Partly addresses angular#14074 and angular#14124. Fixes angular#15510
Previously, `$animate` would decide whether an animation should be cancelled based on some assumption that didn't hold in specific cases (e.g. when animating transcluded clones with `templateUrl` directives on them for the first time). As a result, the entering elements would not be animated in such cases. This affected commonly used, structural built-in directives (`ngIf`, `ngRepeat`, `ngSwitch` etc). This commit fixes it by avoiding invalid assumptions (i.e. by taking into account the transformations that take place while compiling such elements). Partly addresses angular#14074 and angular#14124. Fixes angular#15510
Previously, `$animate` would decide whether an animation should be cancelled based on some assumption that didn't hold in specific cases (e.g. when animating transcluded clones with `templateUrl` directives on them for the first time). As a result, the entering elements would not be animated in such cases. This affected commonly used, structural built-in directives (`ngIf`, `ngRepeat`, `ngSwitch` etc). This commit fixes it by avoiding invalid assumptions (i.e. by taking into account the transformations that take place while compiling such elements). Partly addresses angular#14074 and angular#14124. Fixes angular#15510
The issue is actually related to This one is quite complicated to solve (without animations falling apart) and I also had a hard time writing a failing unit test. If someone could write a failing unit test, I could give it a go. The preconditions for the bug are:
Due to (4), this issue only affects entering animations, since in all other cases the template would be cached (as part of having compiled the element). The easiest workaround is to make sure templates are cached. This can be done:
|
Previously, `$animate` would decide whether an animation should be cancelled based on some assumption that didn't hold in specific cases (e.g. when animating transcluded clones with `templateUrl` directives on them for the first time). As a result, the entering elements would not be animated in such cases. This affected commonly used, structural built-in directives (`ngIf`, `ngRepeat`, `ngSwitch` etc). This commit fixes it by avoiding invalid assumptions (i.e. by taking into account the transformations that take place while compiling such elements). Partly addresses angular#14074 and angular#14124. Fixes angular#15510
Previously, `$animate` would decide whether an animation should be cancelled based on some assumption that didn't hold in specific cases (e.g. when animating transcluded clones with `templateUrl` directives on them for the first time). As a result, the entering elements would not be animated in such cases. This affected commonly used, structural built-in directives (`ngIf`, `ngRepeat`, `ngSwitch` etc). This commit fixes it by avoiding invalid assumptions (i.e. by taking into account the transformations that take place while compiling such elements). Partly addresses #14074 and #14124. Fixes #15510 Closes #15514
Previously, `$animate` would decide whether an animation should be cancelled based on some assumption that didn't hold in specific cases (e.g. when animating transcluded clones with `templateUrl` directives on them for the first time). As a result, the entering elements would not be animated in such cases. This affected commonly used, structural built-in directives (`ngIf`, `ngRepeat`, `ngSwitch` etc). This commit fixes it by avoiding invalid assumptions (i.e. by taking into account the transformations that take place while compiling such elements). Partly addresses #14074 and #14124. Fixes #15510 Closes #15514
Previously, `$animate` would decide whether an animation should be cancelled based on some assumption that didn't hold in specific cases (e.g. when animating transcluded clones with `templateUrl` directives on them for the first time). As a result, the entering elements would not be animated in such cases. This affected commonly used, structural built-in directives (`ngIf`, `ngRepeat`, `ngSwitch` etc). This commit fixes it by avoiding invalid assumptions (i.e. by taking into account the transformations that take place while compiling such elements). Partly addresses angular#14074 and angular#14124. Fixes angular#15510 Closes angular#15514
|
User |
$animateCss ends up operating on an element that does not end up in the view after the directive is rendered.
As you can see in this plunker if you uncomment the template (and comment the templateUrl) the element is correctly styled and appears in the view. However when the directive is using tamplateUrl the element is not styled.
https://plnkr.co/edit/yO1AvvupoveRi3IWevVL?p=preview
This was not the case in 1.4.x, but is a new issue when upgrading to 1.5.
While the directive is using templateUrl you can also check the console and click reveal in elements pane to see that no element is revealed.
This issue means that you can not do any clean up in the done function or any further styling (my issue is I want to scroll the element into view once it is expanded since the element would be too short to scroll into view before. You can also see that '-active' and '-add-active' styles are not removed.
The text was updated successfully, but these errors were encountered: