Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jasonbarry opened this issue Sep 3, 2020 · 2 comments · May be fixed by #1665
Open

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

jasonbarry opened this issue Sep 3, 2020 · 2 comments · May be fixed by #1665
Labels
bug help-wanted An issue that needs additional investigation or community feedback to resolve

Comments

@jasonbarry
Copy link
Contributor

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 colinlohner self-assigned this Sep 3, 2020
@colinlohner
Copy link
Contributor

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

@treygriffith
Copy link
Contributor

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.

onClick={handleBackdropClick}

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)

if (event.target !== event.currentTarget || !shouldCloseOnClick) {

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.

@brandongregoryscott brandongregoryscott added bug help-wanted An issue that needs additional investigation or community feedback to resolve labels Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help-wanted An issue that needs additional investigation or community feedback to resolve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants