[WC-3406]: Fix combobox menu jumping

[yordan-st]

Pull request type

Bug fix (non-breaking change which fixes an issue)

Also includes:

• Dependency change (@floating-ui/react added to combobox-web)
• Test related change (new unit + E2E coverage for menu positioning)

---

Description

The Combobox dropdown menu jumped around while open. Near the bottom of the viewport
it flickered between rendering above and below the input, and changing the number of
visible options (e.g. by typing to filter) made it flip placement mid-interaction.

Root cause: menu positioning was hand-rolled in hooks/useMenuStyle.ts.
getMenuPosition() chose top-vs-bottom placement from the measured menu height, and that
same menuHeight was a dependency of the positioning effect — so placing the menu changed
its height, which re-ran the effect, which recomputed placement, which changed the height
again.

Fix: replace the hand-rolled logic with floating-ui via a new useFloatingMenu hook:

• strategy: "fixed", placement: "bottom-start", whileElementsMounted: autoUpdate
• middleware: offset(4) → flip({ crossAxis: false, fallbackStrategy: "bestFit", padding: 8 })
  → size({ padding: 8 }) — the size middleware sets the menu width to the input width and
  caps maxHeight to min(availableHeight, 320)
• when space is tight the menu now shrinks and scrolls within the viewport instead of
  overflowing or flipping
• alwaysOpen (keepMenuOpen) mode is unchanged — it renders inline (position: relative)
  and does not use floating positioning

Also: deleted useMenuStyle.ts, reworked the menu SCSS so the wrapper is a flex column and
the option list fills + scrolls inside the (possibly shrunk) wrapper, and made the
Single/Multi selection wiring consistent.

Known limitations (out of scope, documented in the OpenSpec change):

• No React portal — with position: fixed, a transformed/filter/contain ancestor can
  still mis-anchor the menu (same as the old code, not a regression).
• No shift middleware (menu width is pinned to the anchor, so horizontal overflow can't
  occur in practice).

What should be covered while testing?

Open the Combobox in the New Country popup (or any combobox), then:

1. No jump — open it near the bottom of the viewport. The menu picks one placement
   (below by default, above only when there's genuinely no room) and stays there. It must
   NOT flicker between above and below.
2. No jump on filter — with the menu open near the bottom, type to change the number of
   matching options. The menu re-anchors smoothly and must NOT flip above↔below as the
   option count changes.
3. Shrink when tight — in a short viewport, the menu shrinks and the list scrolls
   internally; the menu bottom stays on screen (~8px from the edge) instead of overflowing.
4. Header/footer — with a menu header (select-all) or footer configured, both stay fully
   visible when the menu shrinks; only the option list scrolls.
5. Width — the menu width matches the input width.
6. alwaysOpen regression — a combobox with a footer / custom static content (always-open
   mode) still renders inline in the document flow, not floating.
7. General regressions — select, clear, multi-select boxes, lazy-load scrolling all behave
   as before.

[github-actions]

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge

---

What was reviewed

File	Change
src/hooks/useFloatingMenu.ts	New hook wrapping @floating-ui/react with offset/flip/size middleware
src/hooks/__mocks__/useFloatingMenu.ts	Jest manual mock returning stable refs and a marker style
src/hooks/useMenuStyle.ts	Deleted — hand-rolled positioning removed
src/components/ComboboxMenuWrapper.tsx	Now accepts floatingRef + floatingStyles; removed useMenuStyle call
src/components/SingleSelection/SingleSelection.tsx	Wires useFloatingMenu; threads refs/styles to menu
src/components/MultiSelection/MultiSelection.tsx	Wires useFloatingMenu with keepMenuOpen guard
src/components/SingleSelection/SingleSelectionMenu.tsx	Accepts and forwards floatingRef/floatingStyles
src/components/MultiSelection/MultiSelectionMenu.tsx	Accepts and forwards floatingRef/floatingStyles
src/components/__tests__/ComboboxMenuWrapper.spec.tsx	New unit tests: floating wiring and alwaysOpen bypass
src/__tests__/SingleSelection.spec.tsx	Adds mock + floating-wiring assertion
src/__tests__/MultiSelection.spec.tsx	Adds mock + floating-wiring assertion
src/__tests__/StaticSelection.spec.tsx	Adds useFloatingMenu mock
src/ui/Combobox.scss	Menu becomes display: flex; flex-direction: column; list uses flex: 1
e2e/Combobox.spec.js	Three new positioning tests (stable top, shrink, width match)
CHANGELOG.md	Fixed + Changed entries for the unreleased section
package.json	Adds @floating-ui/react ^0.26.27 dependency
openspec/changes/fix-combobox-menu-jump/	Design, proposal, tasks artifacts
e2e/…-snapshots/*.png	Updated baseline screenshots

Skipped (out of scope): pnpm-lock.yaml

---

Findings

⚠️ Low — useFloatingMenu mock uses useRef refs but real hook exposes callback refs

File: src/hooks/__mocks__/useFloatingMenu.ts lines 6–12
Note: The real useFloating returns refs.setReference and refs.setFloating as callback refs (functions), not RefObjects. The mock returns useRef objects instead. In the current tests this doesn't matter because the wiring assertions only check that the mock marker style is present, not that the ref is called. However, if a future test calls refs.setReference(element) expecting a function, it will throw. Consider typing the mock as Function or using jest.fn() stubs to closer match the real API:

export function useFloatingMenu(_open: boolean): any {
    return {
        refs: {
            setReference: jest.fn(),
            setFloating: jest.fn()
        },
        floatingStyles: { "--this-is-mocked-from-unit-tests": "true" },
        isPositioned: true
    };
}

---

⚠️ Low — E2E stability test samples positions without waiting for the visibility: hidden guard to clear on the first read

File: e2e/Combobox.spec.js lines 194–200
Note: The test correctly waits for visibility to not be "hidden" before sampling, which is a good web-first gate. However first is read immediately after that assertion while the five parallel readTop() samples form the stable set. If first itself were sampled before isPositioned flips (race between await expect resolving and the next line), it could produce a misleading baseline. This is very unlikely in practice — the await expect assertion ensures the property is already set — but making first part of the same Promise.all would remove any ambiguity:

const [first, ...samples] = await Promise.all([readTop(), readTop(), readTop(), readTop(), readTop(), readTop()]);
for (const top of samples) {
    expect(Math.abs(top - first)).toBeLessThanOrEqual(1);
}

---

⚠️ Low — visibility: hidden on closed menu conflicts with widget-combobox-menu-hidden (display: none)

File: src/hooks/useFloatingMenu.ts lines 41–44
Note: When open is false, floatingStyles includes visibility: hidden. ComboboxMenuWrapper also adds the CSS class widget-combobox-menu-hidden which sets display: none in the stylesheet. The two are compatible (both hide the element) but display: none already prevents the element from being focusable or measured — the extra visibility: hidden is redundant on the closed path. Only when open && !isPositioned (the first-frame flash prevention) does visibility: hidden carry its own weight. Not a bug, but slightly noisy in the DOM inspector.

---

Positives

• Root-cause analysis is spot-on: the old menuHeight → effect dependency → height change → re-run loop is exactly what caused the flicker, and replacing it with autoUpdate eliminates the feedback cycle entirely.
• alwaysOpen bypass is handled consistently across both SingleSelection and MultiSelection with useFloatingMenu(keepMenuOpen === true ? false : isOpen), fixing the asymmetry in the proof-of-concept branch.
• The size middleware's apply function correctly sets both width and maxHeight inline, letting floating-ui own the values that need to respond to scroll/resize — no stale closure issues.
• SCSS height model change (flex: 1; min-height: 0; max-height: 100% on the list) is the correct way to make header/footer share the capped height; hard-capping the list directly would clip the wrapper's chrome.
• Unit test separation is well-reasoned: mock + wiring tests in Jest, observable geometry (no-jump, shrink, width) in Playwright — correct given jsdom has no layout engine.
• visibility: hidden until isPositioned prevents the first-frame flash without causing layout shifts.
• All three selection variants (Single, Multi, Static) have the mock registered, so no spec can accidentally exercise real floating-ui geometry and flap.
• CHANGELOG entry is present in the Unreleased section with user-facing language, no implementation details.