Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(toast): replace cancel with hide on hideDelay #3558

Closed
wants to merge 1 commit into from
Closed

fix(toast): replace cancel with hide on hideDelay #3558

wants to merge 1 commit into from

Conversation

cebor
Copy link
Contributor

@cebor cebor commented Jul 2, 2015

This replaces the cancel() with hide() in a toast with the option hideDelay.

What it changes is, that the promise get resolved instead of rejected what makes more sense to me.

@ThomasBurleson
Copy link
Contributor

@cebor - plz update and dependent *.spec.js unit tests so all tests pass.

@ThomasBurleson ThomasBurleson added wip and removed wip labels Jul 2, 2015
@cebor
Copy link
Contributor Author

cebor commented Jul 3, 2015

@ThomasBurleson done!

@ThomasBurleson ThomasBurleson added this to the 0.10.1 milestone Jul 3, 2015
@ThomasBurleson ThomasBurleson self-assigned this Jul 3, 2015
@@ -386,7 +386,7 @@ function InterimElementProvider() {

function startHideTimeout() {
if (options.hideDelay) {
hideTimeout = $timeout(service.cancel, options.hideDelay) ;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rschmukler - this changes seems reasonable: hide === close.
As the author of interimElement.js, can you describe the difference [wrt interimElement] between cancel() and hide()).


Btw, a promise wrapper is also needed in hide() change is also needed:

function hide(response) {
  var interimElement = stack.shift();
  return $q.when(interimElement && interimElement.remove().then(function() {
    interimElement.deferred.resolve(response);
  }));
}

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

Successfully merging this pull request may close these issues.

3 participants