Skip to content

[Bug]: headless Popover/Menu/Listbox pin the popover attribute after the props spread, so only one surface can be open per page #36647

Description

Component

Popover

(also affects Menu and Dropdown/Listbox — same defect, see below.)

Package version

@fluentui/react-headless-components-preview 0.2.5

React version

18.3.1

Environment

node v22.12.0
@fluentui/react-headless-components-preview 0.2.5
Chrome 147 (native popover / top layer)

Current Behavior

Three hooks in the headless package set the native popover attribute on their surface slot, and
all three set it on the wrong side of the ...props spread — so a consumer's own popover
value can never win.

Site File Shape today
PopoverSurface components/Popover/PopoverSurface/usePopoverSurface.ts:40-41 popover pinned after ...props
MenuPopover components/Menu/MenuPopover/useMenuPopover.ts spreads popover: 'auto' over the base state's root
Listbox components/Dropdown/Listbox/useListbox.ts assigns state.root.popover = 'auto' after the hook returns

popover="auto" is the browser's light-dismiss mode: opening one auto popover closes every other
open auto popover
. Because the value cannot be overridden, an application that wants two surfaces
open at once — a popover inside a popover, a persistent side surface, anything anchored that should
not dismiss its sibling — has no way to reach popover="manual".

Measured on an 8-cell probe (eight surfaces asked to open simultaneously):

  • before: openCount 1/8, popover attribute reads auto on all eight;
  • after moving the default ahead of the spread: openCount 8/8, attribute reads manual on all
    eight.

Menu and the shared Listbox measured the same way at 1/3 → 3/3 open surfaces.

Expected Behavior

popover is a normal DOM attribute on a public slot. The package's own convention for slot
defaults — slot.always(..., { defaultProps }) — already expresses "a default the consumer may
override", and is used elsewhere in the same files. These three sites should use it, so the value
merges before the consumer's props rather than after.

Reproduction

// Two popovers asked to stay open together.
<Popover open>
  <PopoverTrigger>{/* … */}</PopoverTrigger>
  <PopoverSurface popover="manual">first</PopoverSurface>
</Popover>
<Popover open>
  <PopoverTrigger>{/* … */}</PopoverTrigger>
  <PopoverSurface popover="manual">second</PopoverSurface>
</Popover>

Inspect either surface: the rendered attribute is popover="auto", not "manual", and only one
surface is visible. Happy to provide a StackBlitz on request.

Steps to reproduce

  1. Render two PopoverSurfaces with popover="manual", both open.
  2. Inspect the rendered elements — both carry popover="auto".
  3. Count visible surfaces: one, not two.

Discovery context

Found during pixel-level verification of a styling layer built over
@fluentui/react-headless-components-preview. Verifying an anchored component means opening several
instances in one document at once, which is what surfaced this at the PopoverSurface site; the other
two sites were then found by auditing every place in the package that sets the same attribute,
rather than by waiting to trip over them.

One half of the original finding was withdrawn on evidence, and is recorded here so it is not
re-derived.
An internal review folded the surface's id into this as "the same defect class on a
public prop". It is not. The surface id is pinned deliberately: Popover.types.ts:78-82
documents <Popover id> as the surface's id channel (usePopover.ts:77 resolves
props.id ?? generatedSurfaceId) and usePopoverTrigger.ts:80 points aria-details at that same
value. Letting a surface-level id win would leave the trigger referencing an element that no
longer exists — a dangling IDREF, i.e. an accessibility regression. PopoverSurface.test.tsx
already guards it, and applying the drafted id change fails that test (measured). The same
reasoning retires an equivalent claim about role. Neither should be changed.

Proposed fix

At all three sites, move the attribute into the surface slot's defaultProps — the position
slot.always already provides for defaults — so it merges ahead of ...props:

// useMenuPopover.ts / useListbox.ts, after the fix
root: { popover: 'auto', ...baseState.root },

Note that neither the Menu nor the Listbox site carried anything but the attribute: the listbox's
id and aria-labelledby are owned by useListboxSlot and were not touched, so the id lesson
above does not recur there.

Verified alongside: no visual movement in a 41-scene pixel-diff sweep; Menu light dismiss and the
closed listbox's display: none unchanged. A regression test was added at each of the three sites
and each was verified to fail without the fix.

A working implementation is included in PR #[WINDMOD-PR-NUMBER] (commits 01ccede78a and
40bca2b821); happy to split it into a standalone PR against this issue.

Suggested severity

High - No workaround (a consumer cannot reach popover="manual" at all through the public prop).

Are you willing to submit a PR to fix?

yes

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions