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

Looses reference on state change - angular ui-router #25

Closed
mufsir opened this issue Mar 23, 2016 · 10 comments
Closed

Looses reference on state change - angular ui-router #25

mufsir opened this issue Mar 23, 2016 · 10 comments

Comments

@mufsir
Copy link

mufsir commented Mar 23, 2016

When state change happens, I'm not able to close the existing Growl notification using $growlNotification.remove();. It only closes when time-out occurs.

@jvandemo
Copy link
Owner

@mufsir — Just to make sure I understand correctly:

  • you are using ui-router (because you mention a state change)
  • you are triggering a notification
  • clicking the close link in the notification still works here
  • then the state changes while the notification is still visible
  • clicking the close link in the notification does no longer work
  • the notification disappears after timeout

Is that correct? Thanks!

@mufsir
Copy link
Author

mufsir commented Mar 24, 2016

@jvandemo : Yes!

@jvandemo
Copy link
Owner

@mufsir — Do you by any chance have a plunk for your scenario? If not, would you be interested in creating one? Thanks!

@tyriker
Copy link

tyriker commented Sep 21, 2016

https://plnkr.co/edit/2uPR4AFwJ90idMxMhYRY?p=preview

Go to Route 2, a couple growls will show. Go to Route 1 and they are no longer dismissible.

The TTL is 60 seconds. They'll disappear after the TTL.

@tyriker
Copy link

tyriker commented Sep 21, 2016

The controller $growlNotification may have to move up to the growl-notifications directive? Then there'd have to be some communication between the 2 when adding a new growl...

The problem ultimately looks like the controller goes away when you change to a different state. At least when you change states at the same level (sibling or higher). I'd guess a nested state change would continue to work.

@jvandemo
Copy link
Owner

Okay—I see it.

UI-router destroys the scope (which exposes the controller) when leaving the state and thus the click handler is not executed as expected.

I think it should be possible to create a directive e.g. close-growl-on-click to close the notification when the scope is already destroyed so we can:

<growl-notification>
  <button close-growl-on-click>Close me</button>
</growl-notification>

I will experiment with demo code to see if we can make this happen.

Thanks for reporting! 👍

@jvandemo
Copy link
Owner

I have a working directive right here:
https://plnkr.co/edit/4j9pivI7bBuCVy8QQ2zK?p=preview

@tyriker
Copy link

tyriker commented Sep 29, 2016

This worked for the Plunker, but not in practice. There were 2 issues I faced and resolved.

First, the directive was only removing itself from the DOM, not the entire <growl-notification> element. But, easy fix... just call growlNotificationController.remove() instead and let it do its thing.

Second, since the event handler was running "outside" of Angular, the removal wasn't happening until the next $digest cycle happened. FIxed by wrapping the growlNotificaitonController.remove call in $scope.$apply(...). I don't know how/why it works in the Plunker.

Here's the code I ended up with:

.directive('growlCloseButton', function () {
  return {
    require: '^^growlNotification',
    restrict: 'E',
    link: function ($scope, element, attributes, growlNotificationController) {
      var handler = function () {
        $scope.$apply(growlNotificationController.remove());
      };

      element.on('click', handler);

      $scope.$on('$destroy', function () {
        if (growlNotificationController.timer && growlNotificationController.timer.then) {
          growlNotificationController.timer.then(function () {
            element.off('click', handler);
          });
        }
      });
    },
    template: '<button type="button" class="close pull-right"><span>x</span></button>'
  };
});

Awesome work BTW!

@jvandemo
Copy link
Owner

@tyriker — Awesome, thank you, great feedback!

I will run some additional tests to make sure everything works as expected before adding the directive to the core library.

Thanks again — great work!

@jvandemo
Copy link
Owner

I have added a note to the README file with a link to this thread in case someone needs this behavior. No one else has requested this feature before or since, so I'm going to leave it out of the core library for now.

If anyone feels it should be added in, please add a comment to this thread.

@tyriker—Thank you for your input and feedback!

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

3 participants