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

v1.0.0-beta.14: potential backdrop visible issue for popup #2815

Closed
dreamershl opened this issue Dec 27, 2014 · 1 comment
Closed

v1.0.0-beta.14: potential backdrop visible issue for popup #2815

dreamershl opened this issue Dec 27, 2014 · 1 comment

Comments

@dreamershl
Copy link

my app will track all the popup instance by itself. When the new popup is shown, the previous one will be destroyed by calling "popup.close();". But sometime the backdrop will be still visible with the class like this "< div class="backdrop visible" > < / div > " And the counter of the backdrop is like the following. it is -2!!!

image

There are 2 issues:

  1. Backdrop should protect the counts from mismatched calling. the count should be never < 0. The class 'visible' will only removed after 400ms delay. any change on the count will cancel the removal.
  2. showPopup() is using the closure "previousPopup" in the timer function. It potentially will cause some false value issue. check the following comments ( start with //!) for detail

To make my faced issue clear, in my code, after calling the "alert()", I try to close all the previous popup while the new instance believe it is not the first one and schedule "stackPushDelay", 75, delay to unshift itself into the stack and show up. Within this delay, the previous popup close() will trigger the " $ionicBackdrop.release();" . if dismiss this new instance within 400ms too, this new instance close function will also trigger "$ionicBackdrop.release()" again, Because of the mentioned 1st issue, the counts will be modified without check, this cause the backdrop delayed class removal failure.

     function showPopup(options) {
var popupPromise = $ionicPopup._createPopup(options);
var previousPopup = popupStack[0];

     //! let's assume the value of preivousPopup is true here. So the hide will be called.
if (previousPopup) {
  previousPopup.hide();
}

var resultPromise = $timeout(angular.noop, previousPopup ? config.stackPushDelay : 0)
.then(function() { return popupPromise; })
.then(function(popup) {
 //! here is the closure from the above. the value will be always same as the first time access. not the actual popupStack first item
  if (!previousPopup) {
    //Add popup-open & backdrop if this is first popup
    $ionicBody.addClass('popup-open');
    $ionicBackdrop.retain();
    //only show the backdrop on the first popup
    $ionicPopup._backButtonActionDone = $ionicPlatform.registerBackButtonAction(
      onHardwareBackButton,
      PLATFORM_BACK_BUTTON_PRIORITY_POPUP
    );
  }
  popupStack.unshift(popup);
  popup.show();

  //DEPRECATED: notify the promise with an object with a close method
  popup.responseDeferred.notify({
    close: resultPromise.close
  });

  return popup.responseDeferred.promise.then(function(result) {
    var index = popupStack.indexOf(popup);
    //! this will change the popupStack length. But this change will have no effect on the above closure flag 'preivousPopup'
    if (index !== -1) {
      popupStack.splice(index, 1);
    }
    popup.remove();

    var previousPopup = popupStack[0];
    if (previousPopup) {
      previousPopup.show();
    } else {
      //Remove popup-open & backdrop if this is last popup
      $timeout(function() {
        // wait to remove this due to a 300ms delay native
        // click which would trigging whatever was underneath this
        $ionicBody.removeClass('popup-open');
      }, 400);
      $timeout(function() {
        $ionicBackdrop.release();
      }, config.stackPushDelay || 0);
      ($ionicPopup._backButtonActionDone || angular.noop)();
    }
    return result;
  });
});

function close(result) {
  popupPromise.then(function(popup) {
    if (!popup.removed) {
      popup.responseDeferred.resolve(result);
    }
  });
}
resultPromise.close = close;

return resultPromise;
 }
@dreamershl
Copy link
Author

Just 5 cents thoughts, why don't just adopt the smart pointer reference count concept for the backdrop. anyone who need it, just increase 1. once no need it, just release 1. once it is 0, just hide it from the screen. So no need check whether it is the first popup instance or not.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants