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

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

Open
natlus opened this issue Oct 28, 2024 · 7 comments
Open

Comments

@natlus
Copy link

natlus commented Oct 28, 2024

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
Copy link

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
Copy link

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
Copy link

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
Copy link

alexdemers commented Nov 5, 2024

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
Copy link

aparent-emgs commented Nov 5, 2024

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
Copy link

aparent-emgs commented Nov 8, 2024

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
Copy link

aparent-emgs commented Nov 14, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants