vodkabears / Remodal

No longer actively maintained.
http://vodkabears.github.io/remodal/
MIT License
2.76k stars 774 forks source link

modal can't open again when modal closed twice #291

Open forjuan opened 6 years ago

forjuan commented 6 years ago

I should not close modal twice, but I really did. Then I found I can't open remodal again. Finally I found animationend event can't be triggered when wrapper element's value of display attribute is none, so the wrapper state will be closing and never change back. maybe can set hidden to the value of wrapper's visibility attribute to solve this problem.

` Remodal.prototype.close = function(reason) { var remodal = this;

  // Check if the animation was completed
if (remodal.state === STATES.OPENING || remodal.state === STATES.CLOSING || remodal.state === STATES.CLOSED) {
  return;
}

if (
  remodal.settings.hashTracking &&
  remodal.$modal.attr('data-' + PLUGIN_NAME + '-id') === location.hash.substr(1)
) {
  location.hash = '';
  $(window).scrollTop(scrollTop);
}

syncWithAnimation(
  function() {
    setState(remodal, STATES.CLOSING, false, reason);
  },

  function() {
    remodal.$bg.removeClass(remodal.settings.modifier);
    remodal.$overlay.removeClass(remodal.settings.modifier).hide();
   //====================here=================
   //remodal wraper hide()(display: none) when remodal first closed 
   // but it will never finish the css animation when element's value of display is none, it never trigger animationend event, the state will always be closing, so it won't work when we open the remodal next time
    remodal.$wrapper.hide();
    unlockScreen();

    setState(remodal, STATES.CLOSED, false, reason);
  },

  remodal);

};`

sukima commented 6 years ago

This is a very frustrating bug! It means that modal.close() is unsafe! Now we have to guard against it?! Especially since the modal can close itself which means if you code calls close from an action hook your screwed!

sukima commented 5 years ago

I use a work around to handle this use case. It uses a Confirmer object to handle the complexity as follows:

function openModal() {
  return new Confirmer(resolver => {
    let inst = $('[data-remodal-id=modal]').remodal();
    let promise = inst.open();
    resolver.dispose(() => {
      return promise.then(() => {
        let modalState = inst.getState();
        // https://github.com/vodkabears/Remodal/issues/291
        if (modalState !== 'opened') { return; }
        return inst.close();
      });
    });
  });
}