vodkabears / Remodal

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

Why not remove the modifier on the wrapper when closing remodal? #231

Closed daolf closed 8 years ago

daolf commented 8 years ago

We can see that the modifier class is added to the wrapper but not removed on close:

Remodal.prototype.close = function(reason) {
    var remodal = this;
    debugger
    // Check if the animation was completed
    if (remodal.state === STATES.OPENING || remodal.state === STATES.CLOSING) {
      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.$wrapper.hide();
        unlockScreen();

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

      remodal);
  };`

The fix would simply be to replace

remodal.$wrapper.hide();

By

remodal.$wrapper.removeClass(remodal.settings.modifier);

I can submit a pull request if this is accepted.

PS: I noticed that the modifier is not removed from the $remodal el as well

vodkabears commented 8 years ago

Wrapper is a per-instance element. Bg, overlay are not.