Modal onBackgroundClick fires on mouse up not mouse down

[BrittanyDufort]

When you mouse down inside of the modal, move cursor outside of modal, then release the click (mouse up), onBackgroundClick fires (and the modal closes). Would be great if onBackgroundClick fired on mouse down on background.

Why have we run into this? We have color pickers in the modal and if selecting a color, when dragging the mouse, if the mouse goes outside the modal, the modal will close prematurely. Also happens when highlighting text in a textfield inside of a modal and dragging the mouse outside by accident.

[AlexanderRichey]

The event is passed into onBackgroundClick() (I realize now that this isn't documented). Have you tried conditionally closing the modal based on the event's properties?

[usualdesigner]

The event is passed into onBackgroundClick() (I realize now that this isn't documented). Have you tried conditionally closing the modal based on the event's properties?

Looks like the event is the same, so it does not work

[bnchrch]

Yeah same issue for us. The event doesn't differentiate its all type "click"

[bnchrch]

Would happily submit a PR changing this to onMouseDown if you'd like

[AlexanderRichey]

I don't think we should change the onBackgroundClick behavior. However, I suspect the current API can already accommodate your use-cases.

First, here's why I don't think the behavior of onBackgroundClick should be changed. In general, I'm very, very rarely in favor of changing existing, well-understood behavior of public APIs—that should only happen in a vanishingly small number of cases where the consequences of not changing the behavior would be grim. I don't think the current edge cases justify this kind of change. Another reason why I don't think changing the existing behavior is the right thing to do is that onMouseDown fires on right-clicks. It would be weird for the modal to close if a user right-clicks the background, I think.

The current API already lets you smuggle props to the background component, so I think you can already achieve the behavior you want by leveraging backgroundProps. Here's a sandbox that shows what I mean.

Another strategy that might help your use-cases that might be worth considering is to call event.stopPropagation() (or stopImmediatePropagation) in the appropriate place in your code to prevent the onBackgroundClick handler from being called in the first place in the problematic cases. But this might not work in all cases...

Let me know if either of these suggestions help you out or if you're still blocked.

[bnchrch]

@AlexanderRichey Appreciate the detailed response! Particularly the sandbox. My original request came out of trying to use onMouseDown in backgroundProps but being unable to decern between a background click and a modal click.

I didn't know I was able to do this

  const onMouseDown = (e) => {
    if (e.target === e.currentTarget) {
      toggleModal()
    }
  };

everything now works like a charm