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

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

Open
mmorris8 opened this issue Sep 23, 2024 · 3 comments

Comments

@mmorris8
Copy link

mmorris8 commented Sep 23, 2024

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 mmorris8 changed the title Popover not compatible with native popover API that became widely available in December. [react-aria-components] Popover not compatible with native popover API that became widely available in December. Sep 24, 2024
@mmorris8
Copy link
Author

mmorris8 commented Sep 24, 2024

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

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

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.

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

1 participant