Skip to content

feat(usePopover): add a pluggable positioning-adapter seam - #846

Merged
johnleider merged 8 commits into
vuetifyjs:devfrom
sridhar-3009:feat/popover-adapter-seam-733
Aug 20, 2026
Merged

feat(usePopover): add a pluggable positioning-adapter seam#846
johnleider merged 8 commits into
vuetifyjs:devfrom
sridhar-3009:feat/popover-adapter-seam-733

Conversation

@sridhar-3009

Copy link
Copy Markdown
Contributor

Addresses #733

Scope of this PR

This covers the checklist items that live in packages/0:

  • Define the abstract contract in usePopover/adapters/adapter.ts
  • V0PopoverAdapter reproducing current CSS anchor output byte-for-byte
  • Activator element registration (the missing handle) — attachAnchor()
  • Neutral placement descriptor + raw CSS passthrough
  • Thread the adapter option through PopoverRoot, TooltipRoot, SelectRoot, createCombobox
  • Docs page + a worked "bring your own engine" example that does not ship in the package
  • Snapshot-equivalent test proving the default path is unchanged (the full existing usePopover test suite passes unmodified against the new code path)

Non-goals from the issue are respected: no floating-ui dependency anywhere in packages/0, no bundled JS adapter, CSS anchor positioning stays the zero-cost default.

Naming

Went with PopoverAdapter per the convention the issue itself cites (.claude/rules/implementation.md's "composition noun matches the composable name verbatim"), over the more precise PositionAdapter the issue also raised as worth settling in review. Easy to rename before merge if you'd rather have PositionAdapter — it's a single identifier used consistently, no structural difference either way.

Design notes

Context shape. PopoverAdapterContext carries anchorName (the CSS custom-ident), anchorEl/contentEl (both element refs — the concrete blocker the issue called out, since usePopover previously only ever saw the content element), isOpen, placement, and positionTry. setup(context) returns the reactive style object for the content element — for V0PopoverAdapter that's the existing six CSS declarations; for a JS engine it'd be computed top/left/position.

Placement descriptor. derivePlacement() (adapters/placement.ts) does a best-effort parse of the positionArea CSS value into { side, align } — covers the common cases ('bottom', 'top span-left', logical block-start/block-end) and falls back to bottom/center for anything it doesn't recognize, with raw always carrying the original value verbatim. I didn't try to build a complete grammar for every position-area compound value — the issue asked for a "raw passthrough escape hatch so the CSS path loses no expressiveness," which raw provides regardless of what the normalized descriptor captures.

createCombobox's option is positionAdapter, not adapter — it already has an adapter?: ComboboxAdapter option for query filtering, so reusing the name would be ambiguous. Popover.Root/Tooltip.Root/Select.Root don't have that collision and use plain adapter.

Lifecycle. adapter.dispose?.() runs on onScopeDispose, with Vue 3.5's failSilently flag — usePopover() is routinely called outside an active effect scope (directly in tests, for instance), which would otherwise emit a spurious warning.

Test plan

  • The full pre-existing usePopover test suite (17 tests) passes unchanged against the new adapter-based contentStyles — this is the "default output identical to today" proof, since those tests assert exact CSS output
  • Added unit tests for derivePlacement(), V0PopoverAdapter, and the usePopover adapter seam (custom adapter receiving the right context, attachAnchor/attach populating anchorEl/contentEl, dispose lifecycle) — 100% line/branch coverage on all new files
  • Added two browser tests to Popover/index.browser.test.ts mounting the full Root+Activator+Content stack with a custom adapter, proving both DOM elements reach it and its styles are applied to (not merged with) the content element
  • pnpm vitest run --project v0:unit --project v0:browser → 6561 passed, 3 skipped (full suite, no regressions)
  • pnpm --filter=@vuetify/v0 typecheck clean
  • pnpm eslint clean on all changed files
  • pnpm vitest run packages/0/src/surface.test.ts — updated the frozen public-surface list for the 3 new exports (PopoverAdapter, V0PopoverAdapter, derivePlacement)
  • Added a changeset (minor bump for @vuetify/v0)

One environment caveat: I couldn't get a fully clean local pnpm run build:docs — this checkout has pre-existing case-collision issues on macOS's case-insensitive filesystem between intentionally-paired files like Equalizer.vue/equalizer.vue (unrelated to this PR, reproducible on a pristine dev checkout with zero changes). I reviewed the use-popover.md diff manually for syntax correctness against the page's existing conventions; CI's docs-check runs on a case-sensitive runner and will give the real signal.

usePopover had exactly one positioning implementation - CSS anchor
positioning, hard-coded - and no way to supply another. CSS anchor
positioning is unavailable in Firefox ESR and pre-26 Safari; when
unsupported, the emitted CSS properties are silently ignored and
content renders unanchored at position: fixed. There was also no
JS-positioning-library escape hatch, bundled or otherwise.

Adds a PopoverAdapter abstract class (adapters/adapter.ts) following
the same pattern as LoggerAdapter/LocaleAdapter/StorageAdapter.
V0PopoverAdapter (the default) reproduces today's CSS anchor
positioning output byte-for-byte - verified by the existing
usePopover test suite passing unchanged. v0 ships no JS-engine
adapter of its own; the deliverable is the contract.

usePopover's adapter.setup() context carries the previously-missing
activator element via a new attachAnchor() (companion to attach()),
plus a normalized { side, align } placement descriptor derived from
positionArea (derivePlacement, in adapters/placement.ts), with the
raw CSS value always available as an escape hatch.

Popover.Root, Tooltip.Root, Select.Root, and createCombobox all
thread an adapter option through to their usePopover() call
(positionAdapter on createCombobox, since it already has its own
adapter option for query filtering).

@johnleider johnleider left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Architecture skim: the adapter seam is right (PopoverAdapter + V0PopoverAdapter CSS default, no floating-ui, attachAnchor, positionAdapter on combobox). One descriptor bug before merge — inline on placement.ts.

Comment thread packages/0/src/composables/usePopover/adapters/placement.ts Outdated
…ottom

derivePlacement() was mapping logical inline-start/inline-end onto the
block axis (top/bottom) alongside block-start/block-end. inline-start/
inline-end are the inline axis (left/right in LTR) - conflating the
two axes hands a JS positioning adapter the wrong side. V0PopoverAdapter
is unaffected either way since it renders `raw` verbatim, never the
normalized side/align.

Replaces the if/else-if chain with a keyword lookup table per lint
(unicorn/prefer-switch) and updates the block-start/inline-start test
case to assert the correct axis.
@sridhar-3009

Copy link
Copy Markdown
Contributor Author

Good catch, fixed in b410039inline-start/inline-end now map to left/right instead of the block axis. Added a test case asserting the corrected axis, and swapped the if/else-if chain for a keyword lookup table since the switch was flagged by unicorn/prefer-switch. Full suite still green, coverage still 100% on the changed files.

@johnleider johnleider added this to the v1.1.0 milestone Aug 20, 2026
@johnleider
johnleider merged commit 1864803 into vuetifyjs:dev Aug 20, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants