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

[Dialog] Pointer down on Overlay scrollbar closes dialog (impossible to grab scroll) #1280

Open
lesha1201 opened this issue Mar 29, 2022 · 6 comments · May be fixed by #2252
Open

[Dialog] Pointer down on Overlay scrollbar closes dialog (impossible to grab scroll) #1280

lesha1201 opened this issue Mar 29, 2022 · 6 comments · May be fixed by #2252
Labels
Difficulty: Hard This issue is likely very difficult Package: react/alert-dialog Package: react/dialog Resolution: Needs Investigation This issue needs more investigation Topic: Accessibility This issue has to do with accessibility Type: Bug Confirmed bug

Comments

@lesha1201
Copy link
Contributor

Bug report

In Outer Scrollable story, Dialog Overlay has a scroll but Dialog is closed when clicking on Overlay's scrollbar which makes it impossible to scroll using left mouse button.

Current Behavior

Dialog is closed when clicking on Overlay's scrollbar.

Expected behavior

Dialog isn't closed when clicking on Overlay's scrollbar and it's possible to scroll by holding left mouse button.

Reproducible example

Components._.Dialog.-.Outer.Scrollable.Storybook.-.Google.Chrome.2022-03-29.12-07-54.mp4

Your environment

Software Name(s) Version
Radix Package(s) dialog, dismissable-layer 0.1.7
React n/a 17
Browser Chrome, Firefox 99.0.4844.84, 99.0b8
@jjenzz
Copy link
Contributor

jjenzz commented Mar 29, 2022

there is an onPointerDownOutside event on Dialog.Content that you can prevent if wish to prevent clicking outside from closing https://www.radix-ui.com/docs/primitives/components/dialog#content

@lesha1201
Copy link
Contributor Author

@jjenzz I don't want to prevent clicking outside from closing. I want to preserve it and make it possible to scroll using left mouse button.

@benoitgrelard benoitgrelard added Type: Bug Confirmed bug Resolution: Needs Investigation This issue needs more investigation Difficulty: Hard This issue is likely very difficult Resolution: Backlog Package: react/alert-dialog Package: react/dialog Topic: Accessibility This issue has to do with accessibility labels Apr 6, 2022
@benoitgrelard benoitgrelard changed the title [Dialog] Impossible to scroll with mouse button in scrollable Overlay [Dialog] Pointer down on Overlay scrollbar closes dialog (impossible to grab scroll) Apr 6, 2022
@benoitgrelard benoitgrelard changed the title [Dialog] Pointer down on Overlay scrollbar closes dialog (impossible to grab scroll) [Dialog] Pointer down on Overlay scrollbar closes dialog (impossible to grab scroll) Apr 6, 2022
@BrianHung
Copy link

headless seems to be running into the same problem 😅

https://twitter.com/adamwathan/status/1530666459545931776

@josias-r
Copy link

As @BrianHung already pointed out, it is not really impossible to identify a scrollbarclick by the event target or something.

As a workaround I am using the following handler on Dialog.Content:

onPointerDownOutside={(e) => {
  const currentTarget = e.currentTarget as HTMLElement;

  if (e.detail.originalEvent.offsetX > currentTarget.clientWidth) {
    e.preventDefault();
  }
}}

@benoitgrelard
Copy link
Collaborator

@josias-r,

This is really clever! I was wondering if we would be able to do it by location rather and that's exactly what you've done here.

I think we should be able to use something like this as a solution internally.
We'd have to ensure we cater for RTL, and to do this in only certain conditions are met, but this is a good start.

Thanks! 🙏

@gragland
Copy link

gragland commented Nov 17, 2022

This is my solution to prevent clicks on the 1password button from closing the dialog when clicked. The 1password extension absolutely positions this button over an input within the dialog. The dialog is positioned in the center of the viewport, so clientX/clientY is used instead of offsetX/offsetY.

 <Dialog.Content
  ref={contentRef}
  onPointerDownOutside={(e) => {
    if (!contentRef.current) return
    const contentRect = contentRef.current.getBoundingClientRect()
    // Detect if click actually happened within the bounds of content.
    // This can happen if click was on an absolutely positioned element overlapping content,
    // such as the 1password extension icon in the text input.
    const actuallyClickedInside =
      e.detail.originalEvent.clientX > contentRect.left &&
      e.detail.originalEvent.clientX < contentRect.left + contentRect.width &&
      e.detail.originalEvent.clientY > contentRect.top &&
      e.detail.originalEvent.clientY < contentRect.top + contentRect.height
    if (actuallyClickedInside) {
      e.preventDefault()
    }
  }}
> 
  content here 
</Dialog.Content>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: Hard This issue is likely very difficult Package: react/alert-dialog Package: react/dialog Resolution: Needs Investigation This issue needs more investigation Topic: Accessibility This issue has to do with accessibility Type: Bug Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants