radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.53k stars 792 forks source link

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

Open lesha1201 opened 2 years ago

lesha1201 commented 2 years ago

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

https://user-images.githubusercontent.com/10157660/160576823-63fba34c-4bb4-452e-a90b-7bbe4950c5ba.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 commented 2 years ago

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 commented 2 years ago

@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.

BrianHung commented 2 years ago

headless seems to be running into the same problem 😅

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

josias-r commented 2 years ago

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 commented 2 years ago

@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 commented 1 year ago

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>