ghosh / Micromodal

⭕ Tiny javascript library for creating accessible modal dialogs
https://micromodal.now.sh/
MIT License
3.54k stars 230 forks source link

don't close when clicked inside and let go of mouse button outside #349

Open piotrpog opened 4 years ago

piotrpog commented 4 years ago

That is something that is very annoying if modal contains ay kind of form. In that case, I often click the mouse on text input and let go of it when I move the cursor a bit - when the cursor is already outside modal. And this causes a modal to close.

DimaGashko commented 4 years ago

https://github.com/ghosh/Micromodal/issues/287 https://github.com/ghosh/Micromodal/issues/216

mrdarkside commented 4 years ago

Just remove data-micromodal-close trigger from overlay and add custom event to your modal using MicroModal.close('modal-id') excluding modal content part. Works fine for me.

modal.addEventListener("mousedown", (e) => {
      if (
        e.target.closest(".modal__overlay") && !e.target.closest(".modal__content") ) {
        MicroModal.close("modal-1");
      }
    });
DimaGashko commented 4 years ago

@mrdarkside That's almost the same what I did :)

Only one thing - your solution doesn't work on touch screens. Here's my approach:

(function initMicromodal() {
    document.body.addEventListener('mousedown', (e) => close(e.target));
    document.body.addEventListener('touchstart', (e) => close(e.target));

    /** @param {HTMLElement} target */
    function close(target) {
        if (!target.hasAttribute('data-close-parent-modal')) return;
        const modal = target.closest('.modal');
        if (modal) MicroModal.close(modal.id);
    }
}());
mrdarkside commented 4 years ago

What does this part mean /* @param {HTMLElement} target / ? Kinda comment?

DimaGashko commented 4 years ago

That's JsDoc comment. I use it just to autocomplete and intellisense.

image image image

mrdarkside commented 4 years ago

So we can add and destroy these events and lock scroll if needed with onShow() and OnClose() methods. Here's what I came to, any suggestions?

import MicroModal from "micromodal";

function scrollLock(bool = true) {
  // getting window scroll width value
  const scrollWidth = window.innerWidth - document.body.offsetWidth + "px";
  //all jumping elements aftef scroll lock (body, fixed elems etc)
  const jumpers = document.querySelectorAll("[data-jump-lock]");
  if (bool) {
    document.body.style.overflow = "hidden";
    jumpers.forEach((el) => {
      el.style.paddingRight = scrollWidth;
    });
  } else {
    document.body.style.overflow = "auto";
    jumpers.forEach((el) => {
      el.style.paddingRight = "0";
    });
  }
}

function onOverlayClose(target) {
  if (!target.hasAttribute("data-close-parent-modal")) return;
  const modal = target.closest(".modal");
  if (modal) MicroModal.close(modal.id);
}

MicroModal.init({
  onShow: () => {
    //adding avents and scroll lock
    document.body.addEventListener("mousedown", (e) =>
      onOverlayClose(e.target)
    );
    document.body.addEventListener("touchstart", (e) =>
      onOverlayClose(e.target)
    );
    scrollLock();
  },
  onClose: () => {
    //destroying events
    document.body.removeEventListener("mousedown", (e) =>
      onOverlayClose(e.target)
    );
    document.body.removeEventListener("touchstart", (e) =>
      onOverlayClose(e.target)
    );
    scrollLock(false);
  },
});
DimaGashko commented 4 years ago

I use Event delegation, so, you don't need to add/remove each time. I mean, I add that event to the body element, so it will be called each time you click on the page. Yes, even if all modal windows are closed, but is it bad? There's only one event and the handler just does nothing when the target is not a [data-close-parent-modal] element.

Or just loop through all [data-close-parent-modal] and add events directly (so, you don't need to remove them). (if you don't add modals dynamically)

p.s. in you case I wouldn't add those events to the body element:

MicroModal.init({
  onShow: (modal) => {
    modal.addEventListener("mousedown", (e) => onOverlayClose(e.target));
    . . .
  },
 . . .
});

image

But, if you add/remove those events with onShow() and onClose() methods you can add/remove them directly on an overlay:

onShow: (modal) => {
  const overlay = modal.querySelector('[data-close-parent-modal]');
  if (overlay) overlay.addEventListener('click', () => Micromodal.close(modal.id));
}
. . .

BUT! You missed one thing :)

console.log({ a: 5 } === { a: 5 }) // false

That's why your removeEventListeners just don't work at all

const handler = () => console.log('click')
$el.addEventListener('click', handler);
. . .
$el.removeEventListener('click', handler);
mrdarkside commented 4 years ago

Just checked I really add an additional eventListener each time I open a modal. Thanks!!!