[react-aria-components] Popover not compatible with native popover API that became widely available in December.

[mmorris8]

Provide a general summary of the issue here

There's a newly available popover API. Yay!

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/popover

The React Aria Popover control doesn't work with it. If you put a Popover element inside a native element popover (say, as part of a popover form dialog) it won't work.

🤔 Expected Behavior?

Popover was developed as a component during a time when there was no equivalent API in the browsers. Now that one has been added any alternate implementation should be made compatible with it.

😯 Current Behavior

React Aria Popover uses z-index to get its popover to the top of the stack. This isn't compatible with the new popover API which uses a special top layer in the browser to place popovers, dialogs and any other elements that use the top layer in front of those that do not. As a result React Aria components which leverage popovers like the ComboBox nested inside popover elements will cease to function. While it is possible to nest them in over Aria popovers, the native browser API is easier to work with and hooks into CSS to allow fine control of the animation of the popover.

💁 Possible Solution

Feature detection can be used to detect the presence of the popover API, and the Aria Popover can leverage the API when it is present and use the existing code for older browsers. However, this could present backwards compatibility challenges.

The popover API also introduces new attributes for buttons - popovertarget and popovertargetaction. These need to be relayed to the interior button element.

🔦 Context

I have a facet list with combo boxes. On mobile I want to slide in the facet list from the left since there isn't enough room to keep it continually visible. I composed the desktop view first.

🖥️ Steps to Reproduce

<button popovertarget="my-popover">Open Popover</button>

<div popover="auto" id="my-popover">Greetings, one and all!
<ComboBox>
  <Button>Click to open Popover</Button>
  <Popover>Alas, I cannot work in this context.</Popover>
</ComboBox>
</div>

Version

react-aria 3.34.3

What browsers are you seeing the problem on?

Other

If other, please specify.

All browsers that support the popover api

What operating system are you using?

Mac OSX

🧢 Your Company/Team

Covenant Health

🕷 Tracking Issue

No response

[mmorris8]

Ok, going to show my hack to get this to mostly work as it illustrates the issue. I'm going to prune irrelevant code to make this clearer, so no guarantees that this can be pasted in and work.

export default function MenuFacet({label, virtual, optionLabels, ...props}) {
  // This is an Algolia search facet, so the list box items are provided 
  by the useMenu state hook of that package
  const { items, refine } =  useMenu(props)

  // We need a react Aria id to associate button to target
  const id = useId()

  // Again, Algolia React Instant Search tracks what's selected
  const selectedItem = items.filter(item => item.isRefined)?.[0]?.label

  const onSelectionChange = value => {
    document.getElementById(id).hidePopover()
   // ... Some refinement of the search occurs next, not relevant here.
  }

  return virtual ? null : (
    <ComboBox 
      onSelectionChange={onSelectionChange}
      isDisabled={items.length === 0}
      selectedKey={selectedItem ? selectedItem : ''}
      {...props}
    >
      <Label>{label}</Label>
      <div>
        <Input />
        {/* Wrapping a button in a button - disgusting and I'm all ears to any 
        better way. We need to open the popover without a re-render which messes up the popover state */}
        <button popovertarget={id}><Button /></button>
      </div>
      {/* React Aria needs this element to track the popover state even though 
      we aren't putting anything in it - it's going to be below the top layer and 
      anything in it would be concealed by the blur mask on that layer in my CSS */}
      <Popover />
     {/* The real popover is here */}
      <div popover="auto" id={id}>
        <ListBox items={items} selectionMode="single">
          {item => <ListBoxItem id={item.label}>{
            optionLabels ? optionLabels[item.label] : item.label
          }</ListBoxItem>}
        </ListBox>
      </div>
    </ComboBox>
  )
}

This mostly works. I'm going to have to calculate my list box's position on my own either in JS or with CSS. I would use anchor positioning, but Firefox and Safari aren't ready for that feature.

This "bug" might be needed to be done as a feature, because the CSS markup is likely going to be different. In that case a new element would be needed to do an opt in, or wait until a major version to implement this as despite breaking changes. But I'm not entirely sure breaking changes can't be avoided.

[mmorris8]

Updated the description. I'm using flex blox to position the list box directly off the list with flex-end or flex-start as appropriate. It's working very well.

[mmorris8]

Got this mostly working. Getting this error though -

ariaHideOutside.ts:112  Uncaught TypeError: Cannot read properties of null (reading 'contains')
    at ariaHideOutside.ts:112:64
    at Array.some (<anonymous>)
    at MutationObserver.<anonymous> (ariaHideOutside.ts:112:46)

I'm guessing this is React Aria trying to enforce focus on the now missing React Popover. Whatever it is trylng to do the Popover API is handling.

[mmorris8]

Still could use some help with the above. I found this:

One significant difference between the WAI-ARIA 1.2 ComboBox spec and our ComboBox implementation was the usage of React Portals for the listbox. In the example the listbox is a sibling to the combobox, allowing for easy navigation between the two elements when using a screen reader. The same couldn't be said for our listbox since it is portalled to the end of the document, meaning there could be any number of elements that a screen reader user would have to navigate through just to move from the ComboBox input to the listbox itself. For touch screen reader users, it would be particularly problematic since they may not have access to a physical keyboard and thus can't use the arrow keys to navigate through options.

This put us in a bind because removing the portal strategy wasn't really an option. If we didn't portal the popover out to the end of the document, we'd run into the risk of the listbox being clipped if the combobox's parent had overflow: hidden or overflow: scroll set. Therefore, we had to figure out a way to make the input and listbox the only elements accessible to screen readers while the popover is open so that a screen reader user wouldn't accidentally leave the ComboBox context.

The solution that we came up with was to crawl the DOM and apply aria-hidden to every element that wasn't the input or listbox. To crawl the DOM we used a TreeWalker, setting up a node filter to determine if a node should be left visible, hidden, or skipped in the case where its parent was already hidden. In addition, we watch for any changes in the DOM while the listbox is open via a MutationObserver, hiding those new elements if need be. When the popover closes, every node that we modified is reverted back to its previous state. Check out our ariaHideOutside function in @react-aria/overlays if you'd like to learn more.

That explains what the function is trying to do. One problem - it's entirely unnecessary with the native PopOver API. I'm thinking I'll need to build my own ComboBox unless I can disable this portal behavior.

[mmorris8]

Well, this is what I have.

import React, { useRef } from "react"
import { useId } from "react-aria"
import { Button, ListBox, ListBoxItem } from "react-aria-components"

export default function CvComboBox({
    label, 
    items, 
    selectedKey, 
    inputValue,
    optionLabels
  }) {

  const togglePopUp = () => {
    const popOver = popoverRef.current
    const bounds = inputRef.current.getBoundingClientRect()

    if (bounds.top > window.innerHeight / 2) {
      popOver.style.top = `32px`
      popOver.style.bottom = 'auto'
      popOver.style.height = `${ Math.floor(bounds.top - 32)}px`
      popOver.classList.add('above')
    } else {
      popOver.style.top = 'auto'
      popOver.style.bottom = `32px`
      popOver.style.height = `${ Math.floor(window.innerHeight - bounds.bottom - 32) }px`
      popOver.classList.remove('above')
    }

    popOver.style.left = `${bounds.left}px`
    popOver.style.width = `${bounds.width}px`
    popOver.togglePopover()
  }

  const id = useId()
  const buttonRef = useRef(null)
  const inputRef = useRef(null)
  const listBoxRef = useRef(null)
  const popoverRef = useRef(null)

  return (
    <div className="CvComboBox">
      <label id={`${id}-lb`} htmlFor={`${id}-in`}>{label}</label>
      <div>
        <input id={`${id}-in`} ref={inputRef} defaultValue={inputValue}/>
        <Button buttonRef={buttonRef} onPress={togglePopUp} />
      </div>
      <div ref={popoverRef} popover="auto" id={id} className="menu-popover">
        <div>
            <ListBox  
              items={items}
              listBoxRef={listBoxRef} 
              selectionMode="single"
              selectionBehavior="replace"
              selectedKeys={selectedKey}
              aria-labelledby={`${id}-lb`}
              aria-label="Suggestions"
            >
              {item => <ListBoxItem id={item.label}>{
                optionLabels ? optionLabels[item.label] : item.label
              }</ListBoxItem>}
            </ListBox>
        </div>
      </div>
    </div>
  )
}

I've had to abandon the use of ComboBox itself because of how tightly it is coupled to the Popover element. This shows promises, but Aria behavior implementation isn't my strong suit so I would appreciate any input on what I'm missing here, if anything.

[devongovett]

Is there a reason you must use native popover rather than the React Aria Popover component? I know it's shiny and new, and maybe we'll eventually support it, but is there anything wrong with using the existing Popover that our ComboBox is designed to work with in the meantime?

One problem - it's entirely unnecessary with the native PopOver API.

Potentially? This would require testing on various screen readers to confirm. The native popover is not modal (meaning it does not hide elements outside it), so it may have the same problem.

[mmorris8]

I had a working fly in menu using native popover in place before the decision to use React Aria was made. I put in the combobox and got it to work up until I noticed the Aria HidePopover. I set it aside to work on other problems and yesterday the HidePopover bug was the only one I had left to do, so time to attack it. And it seems I have to build my own combobox for now. If anything I'm at least uncovering the problems that lie ahead for this.

[mmorris8]

One other thing - if I could tell ariaHideOutside the area that it shouldn't be masking somehow that would save me a lot of trouble. That's the only thing I need in order to get the existing combo box to work - I need to inform that function of what my popover contains somehow - but it isn't documented anywhere I can find.

[mmorris8]

I tried putting in my own Popover object hoping it would be able to get the popover properties being passed down from Combobox but that doesn't work. So the internal components do not work like their custom cousins that resort to the use functions like useComboBox - interesting. They are hard wired together.

I'm just trying to figure this out.

• Tried using FocusScope Manager to be rid of the bug. Doesn't work, though intuitively it should since that's what the docs say Popover and Overlay use. Lot's of undocumented magic going on here.

[mmorris8]

Ok, I fixed this by just flat out hacking the offending function.

/**
 * Hides all elements in the DOM outside the given targets from screen readers using aria-hidden,
 * and returns a function to revert these changes. In addition, changes to the DOM are watched
 * and new elements outside the targets are automatically hidden.
 * @param targets - The elements that should remain visible.
 * @param root - Nothing will be hidden above this element.
 * @returns - A function to restore all hidden elements.
 */
export function ariaHideOutside(targets: Element[], root = document.body) {
  // This function is incompatible with the new popover api. Disable if
  // any are present with an immediate return.
  if (document.querySelector('body:has([popover])')) return

  let visibleNodes = new Set<Element>(targets);
  let hiddenNodes = new Set<Element>();

I really dislike applying flat out hacks like this, but I'm out of other more "go with the flow" solutions.

Looks like implementing the new API is going to be a bear (Or not - I don't know much about the underpinnings of the framework, but I learned a bit). In the meanwhile could the code above be applied to get out of the way of programmers who do want to use the new API?

[mmorris8]

The above did not sit well with me. I have solved it after a week of probing. I feel like a complete and total idiot at the simplicity of the fix - nest the popover.

<ComboBox
      onOpenChange={(isOpen) => {
        // We have to open the native popover after it is rendered. By calling setTimeout without
        // a timer we defer this call to the end of the event stack - without this wrapping we get an
        // error.  The popoverToggle method is the same one seen above as we have to recalculate
        // our position once we are promoted to the top layer.
        setTimeout(() => popoverToggle(isOpen, popoverRef.current, wrapRef.current))
      }}    
    >
      <Label>{label}</Label>
      <div ref={wrapRef}>
        <Input />
        <Button />
      </div>
      <Popover>
        <div popover="manual" ref={popoverRef} anchor="providerFacets" className="menu-popover">
          <div> 
            <ListBox 
              items={items} 
              selectionMode="single" 
            >
              {item => <ListBoxItem id={item.label}>{
                optionLabels ? optionLabels[item.label] : item.label
              }</ListBoxItem>}
            </ListBox>
          </div>
        </div>
      </Popover>
    </ComboBox>