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

aria-hidden error #517

Open
cabello opened this issue Nov 27, 2024 · 8 comments
Open

aria-hidden error #517

cabello opened this issue Nov 27, 2024 · 8 comments

Comments

@cabello
Copy link

cabello commented Nov 27, 2024

Hi there,

Minor bug report. Version: v1.1.0

It seems that keyboard navigation trigger errors when using Vaul drawers.

Steps to reproduce:

  • use tab to select the drawer trigger
  • press space to open the dialog
  • see the error showing up in the dev console

This happens with both asChild and having a button under it and without it, this should be enough to reproduce it:

<Drawer>
            <DrawerTrigger>Open</DrawerTrigger>
            <DrawerContent>Content</DrawerContent>
</Drawer>
Blocked aria-hidden on an element because its descendant retained focus. The focus must not be hidden from assistive technology users. Avoid using aria-hidden on a focused element or its ancestor. Consider using the inert attribute instead, which will also prevent focus. For more details, see the aria-hidden section of the WAI-ARIA specification at https://w3c.github.io/aria/#aria-hidden.
Element with focus: button
Ancestor with aria-hidden:

CleanShot 2024-11-27 at 09 13 22@2x

Also notice that it requires a few taps on Tab key in order to get the focus out of the trigger button that is currently aria-hidden and to the drawer and then inside the drawer elements.

@lucaswong98
Copy link

this happens for me too, and it happens even with mouse click on the trigger

@michelecocuccio
Copy link

Same here. It causes a bit of accessibility issue.

@MMTHD
Copy link

MMTHD commented Dec 18, 2024

Same here, the focus is behind the drawer instead of inside it.

@joserealdev
Copy link

Same here, any hints in order to make the error disappear?

@NicoToff
Copy link

NicoToff commented Dec 31, 2024

Same issue for me.

This is caused by the <Portal> element from @radix-ui/react-dialog this library uses.

radix assumes that the rest of the page should be "hidden" when a portaled dialog is shown. This is annoying, because I'm using the drawer in a non-modal way, so the rest of the page is still relevant and shouldn't be hidden.

If like me you are fine with vaul's Drawer staying in its spot in the DOM, you can simply remove the <Portal> component from your code and the drawer will work fine.

If you really need the drawer to be portaled, I think you could just portal it yourself by using createPortal: https://react.dev/reference/react-dom/createPortal

Alright, my workaround was only moving the problem. This is a really annoying issue, and I don't understand how we can hook into the radix Dialog to avoid the whole DOM becoming "aria-hidden".

Live Example

From the docs, open the "non-modal" drawer and see all DOM elements getting "aria-hidden": https://vaul.emilkowal.ski/other#non-modal

Note

This issue doesn't exist with Radix dialog alone. It doesn't add "aria-hidden" when using modal={false}

@NicoToff
Copy link

Related issues:
#519
#497

@NicoToff
Copy link

NicoToff commented Jan 2, 2025

Building upon the fix from alexdemers in #497, here is the hacky hook I'm using in my project to remove the unwanted aria-hidden:

/**
    @param active (Optional) If the Drawer is rendered conditionally, pass a boolean to trigger the fix when needed
 */
export const useFixVaulDrawer = (active = true) => {
  useLayoutEffect(() => {
    if (!active) return;
    const listener = (e: FocusEvent) => {
      e.stopImmediatePropagation();
    };
    document.addEventListener("focusin", listener);
    document.addEventListener("focusout", listener);
    window.requestAnimationFrame(() => {
      // radix also adds a "data-aria-hidden" attribute, so we select the element based on this
      const allHiddenNodes = document.querySelectorAll("[data-aria-hidden]");
      allHiddenNodes.forEach((node) => {
        if (!(node instanceof HTMLElement)) return;
        node.removeAttribute("data-aria-hidden");
        node.removeAttribute("aria-hidden");
      });
    });
    return () => {
      document.removeEventListener("focusin", listener);
      document.removeEventListener("focusout", listener);
    };
  }, [active]);
};

@david-bell-brown
Copy link

Looks like you can add autoFocus={true} to Drawer.Root to get around this problem and have focus properly switch to the Drawer.Content. This is happening because setting this to false (the default) prevents Radix's Dialog auto focus behaviour here.

I admittedly haven't done extensive testing on real devices, but I personally think the expected default behaviour of a component like this is to grab and trap focus on open. I'm also inclined to trust the Radix dev's decision to hide the rest of the page to screen readers when using this as a modal. Therefor, unless there's a good reason I'm not aware of, I think autoFocus's default should be changed to true, or even forced to true if modal = true. At the very least the API reference should be updated to include this.

@NicoToff I'd say the poor non-modal behaviour is a separate issue to this. The problem there is that setting modal={false} on Drawer.Root doesn't forward this property to Radix's Dialog.Root here, which would prevent the aria-hidden stuff. I haven't combed through those linked issues but I can imagine there's a lot of other problems with this use case because of this.

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

7 participants