[Modal] aria-hidden should not be applied on non-portal modals

[pnarielwala-tc]

aria-hidden=true gets applied on top level container when a modal is opened. But if disablePortal is used, aria-hidden should not be set on that top level container as the modal will be created inline and most likely within that top level container

Running axe on the opened modal yields the following error:

"ARIA hidden element must not contain focusable elements (aria-hidden-focus)"

Fix all of the following: Focusable content should have tabindex='-1' or be removed from the DOM

You can find more information on this issue here: https://dequeuniversity.com/rules/axe/3.3/aria-hidden-focus?application=axeAPI

https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/dialog.html

The dialog element is not a descendant of any element that has aria-hidden set to true.

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

1. Modal with disablePortal prop is opened and aria-hidden is set to true on top level html node

Expected Behavior 🤔

1. Modal with disablePortal prop is opened and aria-hidden is not set to true on top level html node
2. Modal without disablePortal prop is opened and aria-hidden is set to true on top level html node

Steps to Reproduce 🕹

https://codesandbox.io/s/material-mui-demo-9oddt?fontsize=14&hidenavigation=1&theme=dark

Context 🔦

Your Environment 🌎

Tech	Version
Material-UI	v4.9.0
React	v16.12.0
Browser
TypeScript	v3.8.0-dev.20200125
etc.

[oliviertassinari]

I could have sworn that we had a 6 months old duplicate issue about this problem, but I can't find it, fare enough. Thanks for the report.

[dkadrios]

Has anyone found a suitable workaround for this? It's wreaking havoc with our accessibility efforts.

It doesn't matter if disablePortal is true or false, aria-hidden="true" gets applied regardless. Premptively setting aria-hidden="false" doesn't help either, it just gets overwritten.

I also have a second root container, dedicated to tooltips, that also is getting set to aria-hidden when a dialog is open. I'm assuming it is being applied to all root body components, not just the react container (?)

[dkadrios]

Super hacky but this seems to be working in the short-term:

useEffect(() => {
    if (open) {
      document
        .getElementById('react-container')
        .setAttribute('aria-hidden', 'false')
    }
  }, [open])

[eps1lon]

It doesn't matter if disablePortal is true or false, aria-hidden="true" gets applied regardless.

@dkadrios That doesn't sound right. Could you open a separate issue and fill out the template so that we can isolate the problem?

[kylewilt]

This appears to still be an issue on 4.11.0. I've traced it down to here: Modal.js Line 124

The isTopModal function returns false and open is true. It seems that the call to add the current modal to the ModalManager instance doesn't happen early enough.

In my case I'm rendering a

element with open={true} all the time as well as disablePortal being set to true.

Is there a particular reason why this hasn't been addressed other than priority and/or resources?

[oliviertassinari]

@kylewilt What's your use case for using disablePortal?

[kylewilt]

@oliviertassinari if we don’t use disablePortal accessibility tools start reporting a lot of problems for screen reader support. Having the dialog appear in the dom outside of the root react node causes you to get errors like missing a “main” element and heading order problems etc. it also tends to complain about hidden input elements.

There’s really no sane work around in that situation because adding the missing nodes to the dialog creates new errors about repeating main nodes etc. it’s basically problematic to put it outside the rest of the tree that the react components for the page live in.

My “insane” workaround at the moment is to use a web pack plugin to replace the modalmanger.JS import with my own which is a verbatim copy of the original with the ariaHidden function basically being a noop.

[mattdemichele-pearson]

@oliviertassinari Just to chime in on a use case for the disablePortal - I am developing a react app that gets loaded and initialized into a separate page that has existing non-react content, including CSS. I use disablePortal to keep the menus self-contained within my app so that I can ensure proper CSS theming.

This really shouldn't be causing aria-hidden=true to be set to the entire root element. I hope a bug fix can be found

[franciscopuyo]

The aria-hidden=true makes the modal not navigable by a screen reader and also the code not testable by using getByRole in react-testing-library

[chittolina]

any updates on this one?

[joshvito]

We are also experiencing screen reader issues because of this problem!

[cordeliadillon]

We're also running into this issue, particularly with the Menu component, which uses a Popover, which uses a Modal.

It'd be great to have a prop like isModal that controls whether aria-hidden is applied to the other page elements. This could default to true for Modals but could be overridden for Menu and Popover. There are valid reasons for having the underlying page be screen reader accessible when a Menu or Popover is open.

[darkgrayknight]

It would be nice if some of these Modal based components could be non-modal (like the isModal idea noted above). We are needing a floating window/toolbox that can be dragged around and the user able to navigate both controls in the Dialog and on the page and for it all to be accessible.

[darkgrayknight]

This appears to still be an issue on 4.11.0. I've traced it down to here: Modal.js Line 124

The isTopModal function returns false and open is true. It seems that the call to add the current modal to the ModalManager instance doesn't happen early enough.

In my case I'm rendering a element with open={true} all the time as well as disablePortal being set to true.

Is there a particular reason why this hasn't been addressed other than priority and/or resources?

This is still an issue in 5.8.0, code is located here mui-base/src/ModalUnstyled/ModalUnstlyed.js

[alexkyle98]

Also experiencing this issue when testing a Popover component in testing library which uses the disablePortal prop. A fix would be great.

[badalsaibo]

screen.getByRole('listbox') doesn't work with disabePortal props enabled. To get by, I had to explicitly use data-testid to get the component and assert test.

  <Select
      MenuProps={{
        disablePortal: true,
        MenuListProps: {
          'data-testid': 'select-options',
        },
      }}
>

screen.getByTestId('select-options')

[blingerson]

This is also an issue with Drawers (or any other modal) where the container property is used. One example is a contained drawer in a sidenav. When the page loads aria-hidden is added to the root container element of the entire page when the item is opened even though the whole page is still accessible since the drawer is contained. It seems maybe aria-hidden on the container element would make more sense here.

[Greg-NetDuma]

If you have time you can try my fix #34165, after some generic testing it seems to work but I've not tested nested modals that deeply.

[eliw00d]

@kylewilt What's your use case for using disablePortal?

Just to add another use case, I ran into this after running into the Dialog > Select enforce focus issue and using the following workaround:

https://github.com/mui/material-ui/issues/10341#issuecomment-770784016

Using disablePortal fixed the Dialog > Select enforce focus issue but broke role selectors in Playwright due to aria-hidden being true all the time.

[darkgrayknight]

We did implement a workaround with useEffect: useEffect(() => { if (open) { let root = document.getElementById("root"); root?.setAttribute("aria-hidden", "false"); } }, [open]); basically setting aria-hidden to false.

[pietmichal]

Any updates?

[pietmichal]

That's how I've fixed it. Drop the component anywhere in the tree.

import React from "react";

export function RootAriaLabelMuiBugFix(props: { rootSelector: string }): null {
  const root = document.querySelector(props.rootSelector);

  React.useEffect(() => {
    if (!root) {
      console.error("RootAriaLabelMuiBugFix couldn't find the element.");
      return;
    }

    const observer = new MutationObserver(() => {
      if (root.getAttribute("aria-hidden")) {
        root.removeAttribute("aria-hidden");
      }
    });

    observer.observe(root, {
      attributeFilter: ["aria-hidden"],
    });

    return () => {
      observer.disconnect();
    };
  }, [root]);

  return null;
}

Example usage: <RootAriaLabelMuiBugFix rootSelector="#root" />

[garyb1]

Any updates on this?

Another use case is using speech input software like voice control on MacOS, no commands will be picked up. Saying "show numbers" or "click item" will not do anything when a parentNode is aria-hidden and therefore not exposed in the a11y tree.

[maximshipko]

For the workaround, render Modal into the specific container:

<div ref={containerRef}>
  <Modal container={() => containerRef.current} open disablePortal>
    ...
  </Modal>
</div>

[jordanarldt]

@maximshipko That workaround is a bit helpful, but still doesn't work if you have other siblings in the container, or if the container is nested unfortunately.