Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Angular 1.4 new ngAnimate breaks toastr? #136

Closed
samypr100 opened this issue Oct 30, 2015 · 20 comments
Closed

Angular 1.4 new ngAnimate breaks toastr? #136

samypr100 opened this issue Oct 30, 2015 · 20 comments

Comments

@samypr100
Copy link

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() {
@Foxandxss
Copy link
Owner

I cannot possibly know until we see that plunker.

@samypr100
Copy link
Author

@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.

@Foxandxss
Copy link
Owner

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

@samypr100
Copy link
Author

@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?

@Foxandxss
Copy link
Owner

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.

@mikepc
Copy link

mikepc commented Dec 7, 2015

Will this work with 1.4 branches before 1.4.7?

@Foxandxss
Copy link
Owner

So far there is no official solution. I will need to rewrite some stuff.
El 7/12/2015 21:47, "mikepc" notifications@github.com escribió:

Will this work with 1.4 branches before 1.4.7?


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

@mikepc
Copy link

mikepc commented Dec 7, 2015

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.

@Foxandxss
Copy link
Owner

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.

@luke-adams
Copy link

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?!

@Foxandxss
Copy link
Owner

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.

@luke-adams
Copy link

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. 👍

:-)

@samypr100
Copy link
Author

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

@luke-adams
Copy link

thanks @Samy100

@SamHurne
Copy link

SamHurne commented May 6, 2016

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.

@ansballard
Copy link

ansballard commented May 6, 2016

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.

@jmosul
Copy link

jmosul commented Sep 9, 2016

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

@Foxandxss
Copy link
Owner

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.

@ansballard
Copy link

I went back to the project where I could replicate the issue. I updated to angular-toastr@2.1.1, angular-animate@1.5.8, 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!

@caiquecaleiro
Copy link

I have done new tests with the angular-toastr@2.1.1 + angular-animate@1.5.8 and it worked correctly for me.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants