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

fix(directive): releasing $timeout on remove() #27

Merged
merged 1 commit into from
Apr 27, 2016

Conversation

sfount
Copy link
Contributor

@sfount sfount commented Apr 27, 2016

This commit uses the Angular $timeout service to clear the timer (set by the growl directives link method) when the $growlNotification.remove() method is called. Ensuring this removal propagates and is respected by Angular's digest loop.

Solution:
Replace

this.timer.cancel()

with

$timeout.cancel(this.timer)

to ensure the timer is cleaned up within Angular.

Problem:
When testing an Angular application with Protractor there is a default configuration ignoreSynchronization that is set to false - this means that Protractor will watch Angular's services $http, $timeout etc. and wait until promises are resolved before continuing the test. In the current implementation the $timeout set in the directive

ctrl.timer = $timeout(//...

is never actually cleared when $growlNotification.remove() is called - this means end to end tests will wait until the ttl has been completed and the $timeout is cleaned up automatically (it is worth noting the call to $animate works fine so it actually looks like the notification is removed!).

This commit uses the Angular $timeout service to clear the timer (set by
the growl directives link method) when the `$growlNotification.remove()`
method is called. Ensuring this removal propagates and is respected by
Angular's digest loop.
@jvandemo jvandemo merged commit a642fce into jvandemo:master Apr 27, 2016
@jvandemo
Copy link
Owner

@sfount — Thank you, excellent work and great PR. Much appreciated! 👍

@jvandemo
Copy link
Owner

Merged and released as v2.4.0. Thank you @sfount!

@sfount
Copy link
Contributor Author

sfount commented Apr 28, 2016

Cheers @jvandemo. This is a great unopinionated library!

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

Successfully merging this pull request may close these issues.

None yet

2 participants