Dragging onto underlay closes modal

[mjlangan]

I put a breakpoint in checkUnderlayClick and I understand why it acts the way it does, but it caught me by surprise the first time my modal disappeared because I was dragging my mouse around inside it. I'm not sure if this is an intended behavior or not, so feel free to educate me if it is :)

This behavior can be reproduced in Chrome & Safari on the demo modals configured to close on underlay clicks. Simply click within the modal contents, and drag the mouse out of the modal and onto the underlay.

[davidtheclark]

@mjlangan Huh, that's strange. So in React's synthetic event, the e.target element depends on where you mouseup rather than where you mousedown? That doesn't seem right ... I wonder if this is some known problem ...

[davidtheclark]

Or maybe (probably) it's doing exactly what it's supposed to ... in which case I'm not sure what the better strategy is here. Any suggestions?

[mjlangan]

@davidtheclark I found that instead of:

underlayProps.onClick = this.checkUnderlayClick;

If the code is this:

underlayProps.onMouseDown = this.checkUnderlayClick;

The modal now only closes when I click outside the modal, and ignores drags. I'm not sure what implications this has on touch users, though.

[davidtheclark]

I'm not sure what implications this has on touch users, though.

Yeah, that's the problem. Right now you can use this modal in such a way that it vertically overflows the viewport and you can scroll with touch by swiping either inside or outside the modal — so we don't want to change things such that a swipe outside closes. So then you're stuck with the complexity of determining whether a touch is a click-like tap or a swipe.

I guess we could use something like https://github.com/davidtheclark/teeny-tap, but it would have to use mousedown instead of click and ensure that on touchscreens that event target you get is from whatever touchstart targeted. Tricky business.

[mjlangan]

I cloned the repo and tried the demo with my onMouseDown change. When I test "mobile" using Chrome's dev tools, the scrolling behavior you describe still works:

Demonstration in RecordIt (it doesn't show the touch cursor so it looks like I have a normal pointer). First scrolling using the mousewheel and then using swipes. http://recordit.co/vRSfjgWmbO

Are there other ways I can test this to further verify that it still works as you would expect with onMouseDown?

EDIT: Tried it on my Galaxy S8, it also works as you describe (I can scroll the modal by either siding my finger on the modal itself or the underlay)

[davidtheclark]

Huh, I'm glad I'm wrong! I tried this out on my own iPhone and bunch of mobile browsers with Browserstack — and found no problems. onMouseDown seems to work 🌮

I am having a hard time time finding any documented explanation of this — like, does no mousedown event fire if you touch-drag, only if you tap? I would love to read about why this works. But in the meanwhile I think I've checked in a sufficient number of browsers that I'm about as confident it will work ok as the other strategy — so let's release it.

[mjlangan]

@davidtheclark I approved the PR, but don't have permission to merge it in for you. Looking forward to pulling in the update in my project :)

[davidtheclark]

Thanks @mjlangan. I'll try to get out a release tonight.