angular-toastr icon indicating copy to clipboard operation
angular-toastr copied to clipboard

Angular 1.4 new ngAnimate breaks toastr?

Open samypr100 opened this issue 9 years ago • 20 comments

I'll put a plunker soon (if I can replicate in a simple environment), but I updated some of my projects that use toastr to Angular 1.4 latest and I'm getting a bunch of errors if multiple toastrs are called at the same time during the resolve function of angular ui router (could this be due to the ngAnimate changes in 1.4?). Commenting out possible multiple calls to toastr fixed the issue.

Sample Error:

TypeError: Cannot read property 'end' of undefined
    at queueAnimation (angular-animate.js:2338)
    at Object.push (angular-animate.js:2189)
    at Object.leave (angular.js:5257)
    at remove (angular-toastr.tpls.js:70)
    at Object.clear (angular-toastr.tpls.js:39)
    at routes.js:100
    at processQueue (angular.js:14745)
    at angular.js:14761
    at Scope.$eval (angular.js:15989)
    at Scope.$digest (angular.js:15800)

In toastr using line #

        $animate.leave(toast.el).then(function() {

samypr100 avatar Oct 30 '15 22:10 samypr100

I cannot possibly know until we see that plunker.

Foxandxss avatar Oct 30 '15 22:10 Foxandxss

@Foxandxss

Man!, had to get very creative to do this plunker. This is actually my first shot at a plunker for this problem. http://plnkr.co/edit/gnRptpYM5vx3y73EYSZD?p=preview

You'll eventually see an throw in the console (this throw is dangerous since it destroys routing, at least in ui-router). This probably has to do with angular ngAnimate changes on 1.4.7 that can't run multiple animations on parallel or something like that.

samypr100 avatar Oct 30 '15 23:10 samypr100

That Plunker is absolutely perfect mate. Will give it a shot this weekend.

Foxandxss avatar Oct 30 '15 23:10 Foxandxss

@Foxandxss

I saw Angular JS team response. :) Should I wait for your 2.0 release then or is there something I can do on my end meanwhile?

samypr100 avatar Nov 25 '15 03:11 samypr100

The problem is that I don't have any good workaround apart from not using .clear in a way that removes toasts that just pop up.

To fix that (in the way that the angular team proposes) I need to rewrite the whole thing (which I want to do for 2.0.0). I hope they fix it anyway.

Foxandxss avatar Nov 25 '15 12:11 Foxandxss

Will this work with 1.4 branches before 1.4.7?

mikepc avatar Dec 07 '15 20:12 mikepc

So far there is no official solution. I will need to rewrite some stuff. El 7/12/2015 21:47, "mikepc" [email protected] escribió:

Will this work with 1.4 branches before 1.4.7?

— Reply to this email directly or view it on GitHub https://github.com/Foxandxss/angular-toastr/issues/136#issuecomment-162657091 .

Foxandxss avatar Dec 07 '15 20:12 Foxandxss

My hats off to you for an awesome piece of software though, man. Can't wait to implement it. I was playing around with the plunkr and it looks like 1.4 works fine as long as you don't wrap it in $interval or possibly $timeout. I plan on implementing it for flash messages basically so I don't believe I will be impacted. Really looking forward to rocking it.

mikepc avatar Dec 07 '15 21:12 mikepc

The problem is that they rewrote the entire $animation module for 1.4 and they added a new cool feature to manage running animations to make it more performance. The problem is that it is not working correctly in some cases and this is one of them. They will technically fix it in a new version, but that is out of my reach.

Foxandxss avatar Dec 07 '15 21:12 Foxandxss

Just wondering if you have managed to make any headway with this; I'm not entirely sure what your plan is. Do plan on fixing this in the near future for current deployments? We have moved to ng 1.4.3 and our angular-toastr messages aren't dismissing correctly anymore and I'm wondering if we need to look at a different solution to waiting for your fix. Your solution is great and I'd love it if your answer is "it'll be done by x", but totally appreciate that this may not be the top thing on your list...

I'm also not too clear what you are referring to when you mention "2.0.0" in this thread; do you mean a version 2 of angular-toastr or a version of angular-toastr for ng 2? If you mean the latter, will this also be fixed for those of us deploying ng 1.4.x?!

luke-adams avatar Jan 12 '16 16:01 luke-adams

Hello,

It is the same issue as this one? If so, Angular hasn't provide a solution for me yet, so I am waiting on that. There is no workaround I can think of because it is angular the one that is processing the animations. The only possible workaround is trying to avoid closing a toastr immediately after open.

About toastr 2. It is a complete rework I have in mind (new tools, written in es6, new code, new features, etc etc). I am still working on the tooling. I am also a maintainer of ui-bootstrap and we recently released 1.0.0 and that took lot of time. I hope I can dedicate time to this toastr 2 now.

Toastr for ng2 will come for sure too, but that is another history.

Foxandxss avatar Jan 12 '16 16:01 Foxandxss

Cool; thanks so much for your reply.

In fact, I just did some more digging and while I thought our issue was the same as this one it transpires that the code that was causing the issue was using deprecated .success() & .error() rather than .then(). With what I have just discovered it was our use of that deprecated code causing an issue that looked a lot like this!

But, it's good to have a clearer idea of your roadmap; thanks again for replying, being quick and being awesome. Really appreciate all you are doing. :+1:

:-)

luke-adams avatar Jan 12 '16 16:01 luke-adams

@luke-adams Just in case: You can still use .success() & .error() if you set to true $httpProvider.useLegacyPromiseExtensions

samypr100 avatar Jan 13 '16 02:01 samypr100

thanks @samy100

luke-adams avatar Jan 13 '16 13:01 luke-adams

FWIW, I ran into this issue myself and ended up working around it by disabling the ng-enter transition animation for the toasts (Plunker). Not ideal, but at least I'm not left with toasts stuck open.

SamHurne avatar May 06 '16 14:05 SamHurne

Figured out a different workaround, not using ngAnimate though, just some custom animations and classes.

/* toastr.config*/
onShown(toast) {
  toast.el
  .addClass("toasty-show");
}
/*toasty.css*/
@keyframes fadein {
  0% { opacity: 0; }
  100% { opacity: 1; }
}
.toast {
  display: none !important;
}
.toast.toasty-show {
  display: block !important;
}
.toasty-show {
  animation: fadein 0.3s ease;
}

So by default, all .toast elements are hidden via display, and shown when .toasty-show is appended. This is because there's a jarring flash when the toast appears and is then immediately set to opacity: 0. I haven't figured out the fadeout yet, since the element is removed immediately after the onHidden event, but I guess this is another alternative to not having animations at all =)

I'm using the latest angular-toastr, angular 1.5.5, and chrome. If I'm missing something and there's a way to postpone the removal of the toast element, let me know and I'll add the couple lines for a full workaround.

Edit: Checked the source, and it looks like there's no good way to hook in and stall before the toast gets removed without $animate.leave(), so without source changes (passing a promise to onHidden maybe?), the leave animation won't work without ngAnimate.

ansballard avatar May 06 '16 21:05 ansballard

Hi, I know this is a old bug, but is resolved in version 2.1.1?

jmosul avatar Sep 09 '16 09:09 jmosul

This is a very complicated bug to replicate and the root cause is in reality different little issues.

I think I was able to remove one of the biggest blockers (replace: true) so hopefully that would help greatly with the issue.

Foxandxss avatar Sep 09 '16 11:09 Foxandxss

I went back to the project where I could replicate the issue. I updated to [email protected], [email protected], and it looks like the transitions are working correctly for me. Can't guarantee everything is resolved since my code has changed quite a bit since, but with all the latest libs it's working for me. Thanks guys!

ansballard avatar Sep 20 '16 14:09 ansballard

I have done new tests with the [email protected] + [email protected] and it worked correctly for me.

Thanks!

caiquecaleiro avatar Sep 28 '16 13:09 caiquecaleiro