emilkowalski / vaul

A drawer component for React.
https://vaul.emilkowal.ski
MIT License
6.46k stars 215 forks source link

Focus is trapped inside drawers when always open even with modal=false #497

Open natlus opened 4 weeks ago

natlus commented 4 weeks ago

When a Drawer is always open and non-dismissible, for example when using snapPoints, tabbing into the drawer or nested drawers will trap the focus and you won't be able to tab back into the rest of the page. Even when using modal={false}.

Repro: https://codesandbox.io/p/devbox/85dwhz

Tabbing the page will result in focus on:

  1. Button outside drawer
  2. Handle in main drawer
  3. Handle in secondary drawer

Tabbing will now be stuck in a loop between main and secondary drawer. (This also happens with only 1 drawer, nested drawers only to debug my specific case.)

Is there a way to get around this behavior? Or is it an issue with Radix dialogs and/or vaul's handling of modal=false?

alexdemers commented 3 weeks ago

This has been bugging me for weeks. With my quick investigation, it looks like it's an issue with Radix and not this library.

alexdemers commented 3 weeks ago

Add this to your Component that renders your Drawer, and it will bypass the focus trap

// Hack to remove focus trap 
useLayoutEffect(() => {
    document.addEventListener('focusin', e => e.stopImmediatePropagation());
    document.addEventListener('focusout', e => e.stopImmediatePropagation());
}, []);
aparent-emgs commented 2 weeks ago

Add this to your Component that renders your Drawer, and it will bypass the focus trap

// Hack to remove focus trap 
useLayoutEffect(() => {
    document.addEventListener('focusin', e => e.stopImmediatePropagation());
    document.addEventListener('focusout', e => e.stopImmediatePropagation());
}, []);

Using the snippet of code inside my component that renders the Drawer does not fix the issue at all for me. The focus still gets trapped inside the drawer and I cannot focus on anything else in the page. Is this workaround missing a critical bit of information?

alexdemers commented 2 weeks ago

Add this to your Component that renders your Drawer, and it will bypass the focus trap

// Hack to remove focus trap 
useLayoutEffect(() => {
    document.addEventListener('focusin', e => e.stopImmediatePropagation());
    document.addEventListener('focusout', e => e.stopImmediatePropagation());
}, []);

Using the snippet of code inside my component that renders the Drawer does not fix the issue at all for me. The focus still gets trapped inside the drawer and I cannot focus on anything else in the page. Is this workaround missing a critical bit of information?

My fix was for focusing on input elements outside the drawer. Below is a fix for clicking on anything outside the drawer that you can try

useEffect(() => {
    if (isOpen) {
        // Pushing the change to the end of the call stack
        const timer = setTimeout(() => {
            document.body.style.pointerEvents = '';
        }, 0);

        return () => clearTimeout(timer);
    } else {
        document.body.style.pointerEvents = 'auto';
    }
}, [isOpen]);
aparent-emgs commented 2 weeks ago

I'm confused, the issue talks about a problem with not being able to use keyboard navigation to tab out of the Drawer's focus trap, not about clicking interactable elements outside of the Drawer's content. I have no problem interacting with the rest of the page, I simply cannot navigate outside of the drawer's inputs with keyboard navigation, and the first snippet that supposedly should be a workaround for this still does not fix that issue for me. Am I missing something?

aparent-emgs commented 2 weeks ago

If I understand the Radix component correctly, the issue here is that we never send the modal property to DialogPrimitive.Root. If this value is never set, it is by default true, and by default the Modal implementation of the DialogContent will be used, which sets the focus to always be trapped inside the dialog as long as it is opened:

How Vaul's Drawer implementation calls DialogPrimitive.Root on line 747:

    <DialogPrimitive.Root
      defaultOpen={defaultOpen}
      onOpenChange={(open) => {
        if (!dismissible && !open) return;
        if (open) {
          setHasBeenOpened(true);
        } else {
          closeDrawer(true);
        }

        setIsOpen(open);
      }}
      open={isOpen}
    >

The definition of Dialog (DialogPrimitive.Root) on line 57:

const Dialog: React.FC<DialogProps> = (props: ScopedProps<DialogProps>) => {
  const {
    __scopeDialog,
    children,
    open: openProp,
    defaultOpen,
    onOpenChange,
    modal = true,
  } = props;
  const triggerRef = React.useRef<HTMLButtonElement>(null);
  const contentRef = React.useRef<DialogContentElement>(null);
  const [open = false, setOpen] = useControllableState({
    prop: openProp,
    defaultProp: defaultOpen,
    onChange: onOpenChange,
  });

  return (
    <DialogProvider
      scope={__scopeDialog}
      triggerRef={triggerRef}
      contentRef={contentRef}
      contentId={useId()}
      titleId={useId()}
      descriptionId={useId()}
      open={open}
      onOpenChange={setOpen}
      onOpenToggle={React.useCallback(() => setOpen((prevOpen) => !prevOpen), [setOpen])}
      modal={modal}
    >
      {children}
    </DialogProvider>
  );
};

The definition of DialogContent on line 231

const DialogContent = React.forwardRef<DialogContentElement, DialogContentProps>(
  (props: ScopedProps<DialogContentProps>, forwardedRef) => {
    const portalContext = usePortalContext(CONTENT_NAME, props.__scopeDialog);
    const { forceMount = portalContext.forceMount, ...contentProps } = props;
    const context = useDialogContext(CONTENT_NAME, props.__scopeDialog);
    return (
      <Presence present={forceMount || context.open}>
        {context.modal ? (
          <DialogContentModal {...contentProps} ref={forwardedRef} />
        ) : (
          <DialogContentNonModal {...contentProps} ref={forwardedRef} />
        )}
      </Presence>
    );
  }
);

How trapFocus is set in DialogContentModal at line 274

trapFocus={context.open}

How trapFocus is set in DialogContentNonModal at line 311

trapFocus={false}

So clearly, with the way this is setup right now, it's kind of obvious why the focus is always trapped even when we set modal to false in Vaul's Drawer component. Is there a reason why we do not set modal on DialogPrimitive.Root? Feels like a given that we should be setting that property, so I assume there has to be a good reason as to why we don't...

aparent-emgs commented 1 week ago

So after searching a bit as far as I understand the modal property was removed in #424, but I'm not entirely sure why...

As it clearly breaks non-modal drawers functionality, could we see about adding it back? Or investigate a way to bypass this focus trap?