Overlay for Dialog/SideSheet responds to mouseup even if mousedown has different target

[jasonbarry]

If you click inside a Dialog or SideSheet component, drag your mouse outside of the element to the overlay, and release the mouse, the dialog / sidesheet dismisses.

Steps to repro:

1. Go to https://evergreen.segment.com/components/dialog
2. Click the first Show Dialog button
3. Click inside the dialog, and while holding down the click, drag your cursor outside of the dialog to the overlay
4. Release the click
5. Notice that the dialog is dismissed

I'd expect the dialog not to respond to the dismissal because the click didn't start on the overlay.

[colinlohner]

Hey @jasonbarry Thanks for bringing this to our attention. I will start to look into this a little more today

[treygriffith]

I looked into this a little bit, and I think I know the cause of the issue (although not the solution).

Overlay is listening for onClick events from the Box component that makes up the overlay and its children. https://github.com/segmentio/evergreen/blob/e1050a724413b344f3d1412199a277bdc2243821/src/overlay/src/Overlay.js#L258

Of course, these onClick events will fire for any click inside the overlay (including inside the children, e.g. Dialog/SideSheet) which is undesirable. So the handleBackdropClick handler checks that the currentTarget (the element where the handler is registered) is the same as the target (the element that fired the onClick event) https://github.com/segmentio/evergreen/blob/e1050a724413b344f3d1412199a277bdc2243821/src/overlay/src/Overlay.js#L229

This ensures that clicks inside of the children do not close the overlay. However, there is a problem with this approach: onClick is only fired for mousedown+mouseup where both events fire in the same element. But since Box encompasses both the backdrop and the children (e.g. SideSheet/Dialog), any click inside any part of the overlay will result in an onClick event.

So I think what we're seeing is that some browsers (Chrome) use the mouseup element as the target, thus defeating the check target === currentTarget in cases where the mouseup is on the backdrop, even if the mousedown is in the children.