feat: Make ComboBox optionally modal

[omarnyte]

Closes #4937

This MR optionally makes ComboBox modal by exposing an isNonModal attribute on both useComboBox and RAC's ComboBox. It does not implement that behavior on Spectrum's ComboBox.

Existing behavior (i.e. hiding outside elements with aria-hidden while the popover is open) is preserved as the default to respect the portal strategy outlined here. When isNonModal={true}, outside elements receive inert to prevent interactions whenever the popover is open.

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

useComboBox

Modal (new behavior)

1. Visit Modal story
2. Open popopver
3. Ensure that p tags above and below the ComboBox have inert attribute
4. Press either of the links. Ensure that
   a. The popover closes
   b. The browser does not navigate to the link
   c. The p tags no longer have the inert attribute

Non-Modal (existing behavior)

1. Visit NonModal story
2. Open popopver
3. Ensure that p tags above and below the ComboBox have aria-hidden attribute
4. Press either of the links. Ensure that
   a. The popover closes
   b. The browser navigates to the link

ComboBox (React Aria Components)

Modal (new behavior)

1. Visit ModalComboBox story
2. Open popopver
3. Ensure that p tags above and below the ComboBox have inert attribute
4. Press either of the links. Ensure that
   a. The popover closes
   b. The browser does not navigate to the link
   c. The p tags no longer have the inert attribute

Non-Modal (existing behavior)

1. Visit NonModalComboBox story
2. Open popopver
3. Ensure that p tags above and below the ComboBox have aria-hidden attribute
4. Press either of the links. Ensure that
   a. The popover closes
   b. The browser navigates to the link

🧢 Your Project:

Screenshots

Modal (new behavior)

Non-Modal (existing behavior)

[omarnyte]

I think the name isModal is more clear, but I used isNonModal to align with Popover's isNonModal prop.

[omarnyte]

true preserves existing behavior.

[omarnyte]

I didn't expose the modal behavior on Spectrum's ComboBox, so we need this type intersection here.

[snowystinger]

Hey, thanks for the PR. So we intentionally use aria-hidden instead of inert here so that users can tab out of a combobox to the next element (in a form for example) regardless of if the popover is open. I feel like this is a bit of a false positive from axe.

[omarnyte]

@snowystinger thanks for the context!

Are you open to a change that applies a data-a11y-ignore="aria-hidden-focus" attribute to those elements that get aria-hidden when the ComboBox popover is open? That would allow us to ignore false positives without turning off the rule entirely.

[snowystinger]

a change that applies a data-a11y-ignore="aria-hidden-focus"

I'm not sure what this would look like. Either it involves a lot of re-renders or it's a lot of searching the dom for inputs and anything else that could trigger it and then the cleanup.
Unfortunately, applying aria-hidden is already a pretty expensive (takes a long time, has caused noticeable delays in the past) bit of code to run. We'd be reluctant to increase how long it takes to run just for an AXE false positive.

Would it be possible to limit what you run AXE on? if you have a combobox open, you just need to run it on the input and the popover, nothing else actually matters

[omarnyte]

The logic to add/remove the data-a11y-ignore attribute can ride the existing logic to add/remove the aria-hidden attribute in the ariaHideOutside function. It would be a two-line change to apply that attribute in all cases. It would be a few more lines to apply it conditionally.

I didn't notice any significant performance difference between applying just aria-hidden and applying both aria-hidden and data-a11y-ignore in the Storybook story. The profiling logs are below (opening the popover; selecting the first item; closing the popover), but I do realize that a Storybook story is tiny compared to a full app.

profiling-dataAfter.json
profiling-data.Before.json

Would it be possible to limit what you run AXE on?

We can choose to turn off a rule entirely, or we can configure a CSS selector of elements to consider/ignore. We prefer not turning off rules unless absolutely necessary. We might be able to use a CSS selector that ignores any elements with aria-hidden if a ComboBox popover is open somewhere on the page. But I haven't looked closely enough at where that popover gets portalled to determine if it's feasible.

[snowystinger]

would data-a11y-disabled be applied to the element that has aria-hidden? or does it need to be on the actual child/descendent input's and tabIndex=-1's?

Would it be ok to only do it during dev, and not in production builds?

[omarnyte]

data-a11y-disabled="aria-hidden-focus" would be applied to the each element that currently gets aria-hidden. If I understand correctly, aria-hidden will make that element's children/descendants inaccessible to assistive devices, so only the elements that currently receive aria-hidden would need that data attribute.

Would it be ok to only do it during dev, and not in production builds?

In our case, we need the attribute in production builds as we run our tests in production/staging environments (both of which are non-development environments).

[snowystinger]

When you run axe, I'm assuming through Cypress, can you assert that there is a Combobox open? Or, I assume you know one is open because you opened it via the test. Then, when you run axe, you could use the configuration:

  "elementFilters": [
    {
      "selector": "[aria-hidden], [aria-hidden] *",
      "include": false
    }
  ]

If I'm missing the mark, maybe you can explain more about how you're running this.

[omarnyte]

Thanks for that suggestion!

For these specific checks, we're running Axe Watcher for Playwright Test in order to generate results for Axe Developer Hub. Those a11y checks run automatically on every Playwright test, so we can't control when the assertion runs. And I don't believe we can configure the rule to ignore certain selectors (I tried). We can only:

• enable/disable specific rules entirely
• include/exclude specific elements (using a CSS selector) in/from analysis

It would be great if we can exclude aria-hidden elements only when that data-a11y-ignore attribute is set so that no genuine aria-hidden-focus issues slip in. But if that's not feasible, I can reach out to Deque with a feature request to allow configuring a selector for a rule (which would be beneficial even outside the issue we're facing).

In any case, I appreciate your ideas to get our tests passing!