Fix modal outside click bug when start inside and drag to outside

[hamitaksln]

Currently, on modal if you start mouse down inside modal and mouse down outside it closes modal and it's super annoying. I've tried to solve it with use-click-outside hook and seems it works just fine.

[rtivital]

We've migrated from use-click-outside hook to overlay events because it does not work correctly with elements that are rendered within portal, for example <Menu withinPortal />

[hamitaksln]

Ah, I didn't know that. However there should be a way to solve the issue. It's very annoying to have closing modal on selecting text and move mouse down outside.

[rtivital]

There is not a good way to support all use cases, current implementation provides good balance between UX and DX

[Keilo75]

Hi, I'm experiencing the same problem - would it be possible to somehow choose the implementation using a prop, for example?
I've got a color picker inside a modal and to be honest, the UX is terrible. A drag-based input doesn't work well with the current way and it leads to a very frustrating experience.
I haven't looked deep into the code but I would really appreciate if there was a way to switch between overlay events / use-outside-click.

[rtivital]

@Keilo75 I do not really know what solution would really satisfy people, see this PR a reference – #2669

[jvdsande]

The use case described in this PR make sense (i.e. a click starting inside the modal, and finishing outside, should not close the modal). But I agree that the solution for now causes other issues while trying to solve this one.

One way this could be solved, even though it can look a bit hacky, is by following the mouse actions of the user through a mutable ref.

The gist of it would be:

• When the user mouse-downs on the modal, set a ref to true
• When the user mouse-ups on the overlay, set the ref to false with a slight delay
• On click on the overlay, only trigger the close call if ref is false

This works because in the case of Modal, mouseup will always fire on the overlay, even when the click was contained inside the modal content.

The slight delay for setting ref to false is because mouseup will actually fire before click, so we need to skip a frame. A simple setTimeout(() => ref.current = false) will suffice.

[jvdsande]

@rtivital , let me know if you are open to my suggestion above 🙂 we have some users complaining about this issue, so we would really enjoy a fix!

@hamitaksln , you can play around with my suggestion and update this PR if you'd like, otherwise I can also work on a separate PR if you prefer

[Keilo75]

@jvdsande

I suggested a workaround like this aswell but from my understanding, something like this would not work because if you were to have children rendered in a portal in that modal, the mouse down event would not fire.
I did some research and came up with a solution in #2896.

[hamitaksln]

@jvdsande

I suggested a workaround like this aswell but from my understanding, something like this would not work because if you were to have children rendered in a portal in that modal, the mouse down event would not fire.
I did some research and came up with a solution in #2896.

I've tried your PR and everything seems to be working fine.

[rtivital]

@jvdsande Haven't tried it, but it seems that won't work for elements within portal, as they are not a part of Modal dom tree. @Keilo75 has sent a PR #2896, it should work fine with portals

[jvdsande]

Yep, I saw it afterwards and it's indeed a better implementation 👍