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.91k stars 832 forks source link

Dialog: Allow bypassing focus lock for some elements #1859

Open brandon-pereira opened 1 year ago

brandon-pereira commented 1 year ago

Feature request

I am trying to use the Dialog component, however I noticed a bug where I am loading Google Maps Place Autocomplete service, and I'm unable to select places because they are rendered in a portal outside the dialog.

This issue also appears to exist if you use a third party <select> library instead of Radix. The reason this is the case for me is because I was attempting to migrate from Reach UI incrementally (modals first, then dropdowns, etc).

Overview

To solve this, I think the dialog component should select a shards or whitelist prop much like React Focus Lock to allow including external elements in the focus lock logic.

Examples in other libraries

https://www.npmjs.com/package/react-focus-lock

Who does this impact? Who is this for?

Probably more advanced use cases, or anyone dealing with third party libraries or dependencies.

Additional context

None

benoitgrelard commented 1 year ago

Hey @brandon-pereira,

Are you able to provide a sandbox demonstrating your issue so we can use this as a baseline?

nimeshvaghasiya commented 1 year ago

@brandon-pereira I'm having same issue, this is because when you use dialog, it sets style = "pointer-events: none;" to body element. this does fix it but It removes backdrop and I need the backdrop while showing radix ui dialog.

I have used modal={false} and it allows me to select place but dialog close suddenly when I select place over google autocomplete dropdown.

@benoitgrelard I think it should be improved but couldn't able to provide sandbox. same issue is appear on this sandbox but with mui + radix ui dialog.

If you click on Movie dropdown and select 'chocolate' dialog is just disappear.

brandon-pereira commented 1 year ago

@nimeshvaghasiya thanks for setting up the code sandbox, I didn't have the time to set one up.

The reason it closes is because they are adding a focus guard to the dialog, but don't expose the controls to add external elements to ignore.

tarmooo commented 1 year ago

Same problem here too, modal=false removes backdrop but would be nice to still have backdrop. My issue is that i can't use https://github.com/flatpickr/flatpickr inside dialog.

max-programming commented 1 year ago

What do you think is the best possible solution right now to get it working?

We can probably add a custom backdrop ourselves.

joaom00 commented 1 year ago

The onInteractOutside prop can be used to check which elements are allowed and use event.preventDefault() to not close

max-programming commented 1 year ago

The onInteractOutside prop can be used to check which elements are allowed and use event.preventDefault() to not close

Doesn't work in my case. I think my usecase is different. I am using the google places API autocomplete inside the dialog. That gets displayed on top but seems like it's focus-locked somehow so I can't click and select any place.

I can still use the keyboard but clicking is still necessary. How can we make it work with that?

Right now I set modal to false and it works but without a backdrop and the modal closes immediately as @nimeshvaghasiya described

tarmooo commented 1 year ago

The onInteractOutside prop can be used to check which elements are allowed and use event.preventDefault() to not close

Doesn't work in my case. I think my usecase is different. I am using the google places API autocomplete inside the dialog. That gets displayed on top but seems like it's focus-locked somehow so I can't click and select any place.

I can still use the keyboard but clicking is still necessary. How can we make it work with that?

Right now I set modal to false and it works but without a backdrop and the modal closes immediately as @nimeshvaghasiya described

const SheetPortal = () => (
    <SheetPrimitive.Portal>
        <div className="myBackdropClassName">{children}</div>
    </SheetPrimitive.Portal>
);

something like that should add your backdrop back when modal=false just add a class to that div that has background with opacity

max-programming commented 1 year ago

@tarmooo Yeah adding a sheet might

The onInteractOutside prop can be used to check which elements are allowed and use event.preventDefault() to not close

Doesn't work in my case. I think my usecase is different. I am using the google places API autocomplete inside the dialog. That gets displayed on top but seems like it's focus-locked somehow so I can't click and select any place. I can still use the keyboard but clicking is still necessary. How can we make it work with that? Right now I set modal to false and it works but without a backdrop and the modal closes immediately as @nimeshvaghasiya described

const SheetPortal = () => (
    <SheetPrimitive.Portal>
        <div className="myBackdropClassName">{children}</div>
    </SheetPrimitive.Portal>
);

something like that should add your backdrop back when modal=false just add a class to that div that has background with opacity

Still it does not solve the problem. The main issue I think is about the focus guard/focus lock that is on the dialog as @brandon-pereira said

ramonmalcolm10 commented 1 year ago

I found a temporary solution that track whether the user is typing or not by using onFocus and onBlur to prevent closing of the modal.

const Modal: React.FC<Props> = ({
    isOpen,
    onClose,
    title,
    description,
    children,
    disable,
    isModal = true,
}) => {
    const handleClose = () => {
        if (!disable) {
            onClose();
        }
    };

    return (
        <>
            {isOpen && !isModal && (
                <div className="fixed inset-0 z-50 bg-background/80 backdrop-blur-sm data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0" />
            )}
            <Dialog open={isOpen} onOpenChange={handleClose} modal={isModal}>
                <DialogContent>
                    <DialogHeader>
                        <DialogTitle>{title}</DialogTitle>
                        <DialogDescription>{description}</DialogDescription>
                    </DialogHeader>
                    <div>{children}</div>
                </DialogContent>
            </Dialog>
        </>
    );
};
BenLebich commented 1 year ago

For anyone who's using react-google-autocomplete + radix dialog here was my solution:

Add this to the Content component:

onInteractOutside={(e) => {
  const classes: Array<Array<string>> = [];

  e.composedPath().forEach((el: any) => {
      if (el.classList) {
          classes.push(Array.from(el.classList));
      }
  });
  // pac = classes naming for https://www.npmjs.com/package/react-google-autocomplete
  // primary use case is the "AddContact" modal
  if (classes.join("-").includes("pac-container")) {
      e.preventDefault();
  }
}}

And anywhere you're rending the autocomplete input, just add this:

useEffect(() => {
        // Disable Radix ui dialog pointer events lockout
        setTimeout(() => (document.body.style.pointerEvents = ""), 0)    
})
MarcusChrist commented 1 year ago

For anyone who's using react-google-autocomplete + radix dialog here was my solution:

Add this to the Content component:

onInteractOutside={(e) => {
  const classes: Array<Array<string>> = [];

  e.composedPath().forEach((el: any) => {
      if (el.classList) {
          classes.push(Array.from(el.classList));
      }
  });
  // pac = classes naming for https://www.npmjs.com/package/react-google-autocomplete
  // primary use case is the "AddContact" modal
  if (classes.join("-").includes("pac-container")) {
      e.preventDefault();
  }
}}

And anywhere you're rending the autocomplete input, just add this:

useEffect(() => {
        // Disable Radix ui dialog pointer events lockout
        setTimeout(() => (document.body.style.pointerEvents = ""), 0)    
})

Worked like a charm

Archiyopp commented 1 year ago

Had a similar problem with a flatpicker calendar, I had to add a pointers events auto class to the calendar, and create a ref for the overlay element

onInteractOutside={(e) => {
  if (e.target !== overlayRef.current) {
    // avoid closing while interacting with timepicker calendar
    e.preventDefault();
  }
}}
kaydenthomson commented 10 months ago

For anyone who's using react-google-autocomplete + radix dialog here was my solution:

Add this to the Content component:

onInteractOutside={(e) => {
  const classes: Array<Array<string>> = [];

  e.composedPath().forEach((el: any) => {
      if (el.classList) {
          classes.push(Array.from(el.classList));
      }
  });
  // pac = classes naming for https://www.npmjs.com/package/react-google-autocomplete
  // primary use case is the "AddContact" modal
  if (classes.join("-").includes("pac-container")) {
      e.preventDefault();
  }
}}

And anywhere you're rending the autocomplete input, just add this:

useEffect(() => {
        // Disable Radix ui dialog pointer events lockout
        setTimeout(() => (document.body.style.pointerEvents = ""), 0)    
})

@BenLebich's solution also works great if your using the Google's SDK directly.

I made some minor modifications - but the results are the same.

onInteractOutside={(e) => {
  const hasPacContainer = e.composedPath().some((el: EventTarget) => {
    if ("classList" in el) {
      return Array.from((el as Element).classList).includes("pac-container")
    }
    return false
  })

  if (hasPacContainer) {
    e.preventDefault()
  }
}}
elikeyz commented 9 months ago

For anyone who's using react-google-autocomplete + radix dialog here was my solution: Add this to the Content component:

onInteractOutside={(e) => {
  const classes: Array<Array<string>> = [];

  e.composedPath().forEach((el: any) => {
      if (el.classList) {
          classes.push(Array.from(el.classList));
      }
  });
  // pac = classes naming for https://www.npmjs.com/package/react-google-autocomplete
  // primary use case is the "AddContact" modal
  if (classes.join("-").includes("pac-container")) {
      e.preventDefault();
  }
}}

And anywhere you're rending the autocomplete input, just add this:

useEffect(() => {
        // Disable Radix ui dialog pointer events lockout
        setTimeout(() => (document.body.style.pointerEvents = ""), 0)    
})

@BenLebich's solution also works great if your using the Google's SDK directly.

I made some minor modifications - but the results are the same.

onInteractOutside={(e) => {
  const hasPacContainer = e.composedPath().some((el: EventTarget) => {
    if ("classList" in el) {
      return Array.from((el as Element).classList).includes("pac-container")
    }
    return false
  })

  if (hasPacContainer) {
    e.preventDefault()
  }
}}

I have been racking my brain all day over this issue till I stumbled on this solution, works like a charm. Nice work, guys.

mr-chandan commented 7 months ago

I had the same problem here is how i fixed it

useEffect(() => {
  // Disable Radix ui dialog pointer events lockout
  setTimeout(() => (document.body.style.pointerEvents = ""), 0);
 });
                onInteractOutside={(e) => {
                  const classes: string[] = [];
                    //add all the classes to the array
                  e.composedPath().forEach((el: EventTarget | HTMLElement) => {
                    if (
                      (el as HTMLElement).classList &&
                      (el as HTMLElement).classList.length > 0
                    ) {
                      console.log(el);
                      classes.push(
                        ...Array.from((el as HTMLElement).classList)
                      );
                    }
                  });

                  // Check if any of the classes contain the specified class names
                  const classNamesToCheck: string[] = [
                    "tox-menu-nav__js",
                    "tox-swatches__row",
                    "tox-dialog"
                  ];
                  if (
                    classNamesToCheck.some((className) =>
                      classes.includes(className)
                    )
                  ) {
                    e.preventDefault();
                  }
                }}
lumenwrites commented 4 months ago

Ugh, took me a long time to track down and fix this bug. This is what worked for me (similar to the solutions above, but without the need for setTimeout):

const ModalComponent = ({
  open = false,
  onOpenChange,
  size = 'narrow',
  className,
  closeOnOutsideClick = true,
  children,
}: ModalProps) => {
  // Workaround to disable Radix ui dialog pointer events lockout, which prevents
  // other elements opened inside the portal from receiving pointer events.
  // https://github.com/radix-ui/primitives/issues/1859#issuecomment-1771356816
  const [hasRendered, setHasRendered] = useState(false)
  useEffect(() => {
    // Force an update to re-enable pointer events in a separate render cycle
    setHasRendered(open)
  }, [open])
  useEffect(() => {
    document.body.style.pointerEvents = ''
  }, [hasRendered])

  return (
    <DialogPrimitive.Root open={open} onOpenChange={onOpenChange}>
      <DialogContent
        size={size}
        className={className}
        closeOnOutsideClick={closeOnOutsideClick}
      >
        {children}
      </DialogContent>
    </DialogPrimitive.Root>
  )
}
ModalComponent.displayName = 'Modal'

There really needs to be a proper fix for this.

Takhine commented 2 weeks ago

My use-case was to enable animation so I needed to forceMount the Dialog, but it kept stealing the focus on re-render from other inputs

I was able to fix this by doing the following:

<RadixDialog.Content onOpenAutoFocus={(e) => e.preventDefault()}>