Overlays: Extend positioner slot pattern to Popover, Select, Autocomplete#78168
Conversation
4df805c to
2345aeb
Compare
172ffaa to
558b586
Compare
|
Size Change: +126 B (0%) Total Size: 7.95 MB 📦 View Changed
ℹ️ View Unchanged
|
2345aeb to
c255011
Compare
558b586 to
9e90f93
Compare
9e90f93 to
bee6cbd
Compare
|
Flaky tests detected in bab4324. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25787349880
|
24bc2ec to
732c158
Compare
897b195 to
316247e
Compare
316247e to
66bcbce
Compare
The Popover, Select, and Autocomplete entries added in this PR were missing the PR number link, breaking the convention followed by every neighboring entry.
The Popover, Select, and Autocomplete entries added in this PR were missing the PR number link, breaking the convention followed by every neighboring entry.
05ced53 to
4630029
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
6a672e1 to
bab4324
Compare
The Popover, Select, and Autocomplete entries added in this PR were missing the PR number link, breaking the convention followed by every neighboring entry.
|
|
||
| ### When to add a new slot prop | ||
|
|
||
| Only when there is a concrete consumer that needs to reach a Base UI subcomponent's customization. Do not preemptively expose slots. |
Expose a renderable `Popover.Positioner` subcomponent that wraps Base
UI's `_Popover.Positioner` with our default styles (box-sizing reset,
`.positioner` class for the `--wp-ui-popover-z-index` cascade) and our
default props (`side="bottom"`, `align="center"`, `sideOffset={ 8 }`,
`arrowPadding={ 8 }`).
Exposed ahead of being wired into `Popover.Popup`'s composition; the
next commit threads it through as the default for a new `positioner`
slot prop, mirroring the pattern already established by `Tooltip`.
`Popover.Popup` removes `side`, `align`, `sideOffset`, `alignOffset`, `anchor`, `arrowPadding`, `collisionAvoidance`, `collisionBoundary`, `collisionPadding`, and `sticky` (breaking). Replace them with a `positioner?: ReactElement<Omit<PositionerProps, 'children'>>` slot prop accepting a `<Popover.Positioner />` element, mirroring the existing `portal` slot pattern. When omitted, the default `<Popover.Positioner />` is used. `Popover.Positioner` exposes Base UI's full positioner surface rather than the previous hand-picked subset. Internal callsites migrated: Popover stories and tests. JSDoc updated on `Popover.Root` to reflect the new subcomponent.
Add a renderable `Select.Positioner` subcomponent and an optional
`positioner` slot prop on `Select.Popup`, mirroring the pattern
already established by `Tooltip` and `Popover`.
Purely additive — `Select.Popup` did not previously expose flat
positioner props, so existing callsites are unaffected.
`Select.Positioner` defaults to the previously hard-coded
`ITEM_POPUP_POSITIONER_PROPS` (`align="start"`, `sideOffset={ 8 }`,
`collisionPadding={ 12 }`) and `alignItemWithTrigger={ false }`, so
default placement is unchanged. Internal `Select.Popup` now composes
the new `Select.Positioner` rather than inlining `_Select.Positioner`.
Add a renderable `Autocomplete.Positioner` subcomponent and an
optional `positioner` slot prop on `Autocomplete.Popup`, mirroring
the pattern already established by `Tooltip`, `Popover`, and `Select`.
Purely additive — `Autocomplete.Popup` did not previously expose flat
positioner props, so existing callsites are unaffected.
`Autocomplete.Positioner` defaults to the previously hard-coded
`ITEM_POPUP_POSITIONER_PROPS` (`align="start"`, `sideOffset={ 8 }`,
`collisionPadding={ 12 }`), so default placement is unchanged.
Internal `Autocomplete.Popup` now composes the new
`Autocomplete.Positioner` rather than inlining `_Autocomplete.Positioner`.
Codify the pattern adopted across `Tooltip`, `Popover`, `Select`, and `Autocomplete` so future overlay primitives stay consistent: - Each customizable Base UI subcomponent gets a same-named wrapper subcomponent (e.g. `Tooltip.Positioner`) and a same-named slot prop on `Popup` accepting a React element of the matching subcomponent. - `renderSlotWithChildren` centralizes the cloneElement composition. - New slot props should be added only when a concrete consumer needs to reach the underlying subcomponent. - High-level wrappers (e.g. `IconButton`) re-expose the same slot props by the same name/shape.
Reword the JSDoc lead from "Base UI's Positioner" to "`Popover.Positioner`" to keep the story description framed around our public API surface rather than the underlying primitive. Matches the cleanup applied elsewhere in the Popover and Tooltip stories.
The Popover, Select, and Autocomplete entries added in this PR were missing the PR number link, breaking the convention followed by every neighboring entry.
…nedSize The JSDoc said the `style` prop "targets the positioner", but since #77452 `style` on `Popover.Popup` is forwarded to the inner Base UI popup. The example itself is still correct — Base UI maintainers explicitly recommend applying `max-height: var(--available-height)` on `Popup` (the CSS variables cascade from the positioner). Just the doc wording was wrong.
The Select Positioner wrapper has long overridden Base UI's alignItemWithTrigger default (true) to false, but that override was not commented. Add a short note so a future contributor does not "fix" it back to the upstream default. No behavior change.
Drop the satisfies-Partial<_Select.Positioner.Props> constraint, which was misleading now that the constant is consumed by both Select and Autocomplete positioner wrappers. With as const, each value is narrowed to its literal type and validated at the consuming wrapper where the prop type is known, so a mismatch with either Select or Autocomplete's positioner type surfaces as a type error at the use site. Also expand the JSDoc to mention both consumers explicitly.
Inside each component's types module, drop the Select/Autocomplete prefix on the positioner props type so the export matches the PortalProps convention (unprefixed in all four overlay packages) and matches Tooltip and Popover, which already export PositionerProps. Type is namespace-scoped (consumed via Select.PositionerProps / Autocomplete.PositionerProps if anyone re-exports it), so there's no public-API rename.
Cover the new `positioner` slot prop on `*.Popup`: pass a custom `<*.Positioner data-testid="…" />`, open the popup, and assert that the custom positioner element ends up in the DOM wrapping the popup content. This verifies the slot prop is honored and that `renderSlotWithChildren` correctly injects the popup subtree as children — i.e. the wiring we added in this PR. Mirrors the existing `portal` slot tests in shape, scoped to our own API (does not re-test Base UI positioning behavior).
…r wrappers Replace the per-key destructure-with-default pattern with a single spread of `ITEM_POPUP_POSITIONER_PROPS`, followed by the Base UI default overrides we still need to apply (`alignItemWithTrigger=false` on Select), then the consumer's `...props`. Same merge order, same runtime behavior — but adding a new shared default key now propagates to both wrappers without touching each destructure list. The `alignItemWithTrigger` override comment is preserved inline on the JSX so the intent stays visible.
bab4324 to
e4c9417
Compare
…er types Aligns these wrapper types with the package-wide convention used for other Base UI-derived subcomponent types (`Root`, `Trigger`, `Popup`, etc., across `Collapsible`, `Fieldset`, `Select`, …). `ComponentProps` normalizes `className`, `style`, and `render` to our standard shapes, which is the same surface we already expose for sibling subcomponents. The previous `ComponentPropsWithoutRef< typeof _X.Portal | _X.Positioner >` was an outlier that leaked Base UI's callback-form `className`/`style` and two-arg `render` into our public types. Per Mirka's suggestion on #78168.
…ositioner types Finishes the consistency sweep started in the previous commit: every overlay's `Portal`/`Positioner` wrapper type now goes through `ComponentProps< typeof _X.Subcomponent >`, matching the convention used across the rest of the package for Base UI-derived subcomponent types. This drops the last `ComponentPropsWithoutRef` outliers and stops leaking Base UI's callback-form `className`/`style` and two-arg `render` into our public types.
The previous two commits switched every overlay's `Portal` wrapper type from `ComponentPropsWithoutRef< typeof _X.Portal >` to `ComponentProps< typeof _X.Portal >`, which is the package's standard shape but drops Base UI's `(state) => …` callback forms of `className` / `style` and the two-arg `(props, state) => …` form of `render`. The `Portal` types ship in 0.12.0, so the narrowing is breaking at the TypeScript level (runtime is unchanged). The matching `Positioner` narrowings don't need an entry: those types are either new in this PR or new in unreleased #78089, so they never shipped in the wider form.
`Popover.Popup` stopped accepting `collisionBoundary` directly when #78168 introduced the `Popover.Positioner` slot subcomponent. The prop is now silently ignored on `Popup`, so the cross-iframe story's collision avoidance regressed after the rebase onto trunk. Route the boundary through `Popover.Positioner` (matching the modern `Popover.Popup`'s `positioner` slot pattern) so the popup honors the iframe's clipping edge again. This file is `.jsx` so the type system didn't catch the silent prop-drop.
`Popover.Popup` stopped accepting `collisionBoundary` directly when #78168 introduced the `Popover.Positioner` slot subcomponent. The prop is now silently ignored on `Popup`, so the cross-iframe story's collision avoidance regressed after the rebase onto trunk. Route the boundary through `Popover.Positioner` (matching the modern `Popover.Popup`'s `positioner` slot pattern) so the popup honors the iframe's clipping edge again. This file is `.jsx` so the type system didn't catch the silent prop-drop.
`Popover.Popup` stopped accepting `collisionBoundary` directly when #78168 introduced the `Popover.Positioner` slot subcomponent. The prop is now silently ignored on `Popup`, so the cross-iframe story's collision avoidance regressed after the rebase onto trunk. Route the boundary through `Popover.Positioner` (matching the modern `Popover.Popup`'s `positioner` slot pattern) so the popup honors the iframe's clipping edge again. This file is `.jsx` so the type system didn't catch the silent prop-drop.
`Popover.Popup` stopped accepting `collisionBoundary` directly when #78168 introduced the `Popover.Positioner` slot subcomponent. The prop is now silently ignored on `Popup`, so the cross-iframe story's collision avoidance regressed after the rebase onto trunk. Route the boundary through `Popover.Positioner` (matching the modern `Popover.Popup`'s `positioner` slot pattern) so the popup honors the iframe's clipping edge again. This file is `.jsx` so the type system didn't catch the silent prop-drop.
…ot (#78183) * Draggable: Migrate clone wrapper to wp compat overlay slot Replace the legacy body-level / element-wrapper placement and its `z-index: 1000000000` with a portal-style migration onto the `@wordpress/ui` compat overlay slot (#77851). When the slot is available, the drag clone joins the slot's body-level stacking context across all three placement modes, so an active drag automatically shares stacking with any `@wordpress/ui` overlay opened mid-drag without needing per-version z-index races. Auto-enabled in WordPress environments via the slot helper's `window.wp.components` auto-detect; standalone hosts that bundle `@wordpress/components` directly fall back to the previous placement until they call `useEnableWpCompatOverlaySlot()`. `@wordpress/components` imports `getWpCompatOverlaySlot()` directly from `@wordpress/ui`'s public exports (also promoted from internal in this change). The `@wordpress/components` dep on `@wordpress/ui` is transitional, scoped to the legacy-overlay migration. Cross-document drags (e.g. dragging an element inside an iframe while the slot is in the parent document) fall back to the previous placement so the clone's viewport-relative geometry stays in a single coordinate space. The default placement mode (`appendToOwnerDocument: false`, no `dragComponent`) previously appended the clone to the dragged element's parent. In WP environments where the slot is now in effect, the clone instead lives in the slot — a body-level location. In-repo ripgrep finds no CSS or event-delegation scoping anchored to the clone's previous in-flow parent; external consumers that relied on that ancestry must either not opt into the slot or migrate their scoping. * Draggable: Storybook: render docs-page stories in iframes The drag clone uses `position: fixed`, which Storybook's docs-page wrappers break because they apply `transform`s that establish new containing blocks. As a result the clone resolves against those wrappers instead of the viewport and lands in the wrong place on the autodocs page. Render each Draggable story in its own iframe on the autodocs page via `parameters.docs.story.inline: false`, which restores viewport-relative positioning for the clone. * Draggable: Storybook: polish cross-document fallback playground story Three small follow-ups on the iframe regression story: - Inject the Draggable SCSS into the iframe via Vite's `?inline` import (same pattern `WithGlobalCSS` uses with `global-basic.scss?inline`) instead of duplicating rule bodies in `srcDoc`. Single source of truth; future SCSS edits flow through automatically. - Guard the style injection on `iframeDoc?.head` so the brief about:blank → srcDoc transition doesn't throw on the initial `useEffect` pass. - Align the slot-presence display with the public `getWpCompatOverlaySlot()` API: it now returns `undefined` rather than `null` when no slot is registered. * CHANGELOG: Restore entries dropped during rebase Three Unreleased entries were inadvertently removed when the Draggable migration commit was rebased onto trunk: - @wordpress/components Internal: `Modal`, `Menu`, `DropdownMenu` motion-token adoption (#76097). - @wordpress/components Internal: `Popover` close-button z-index cleanup (#78180). - @wordpress/ui Bug Fixes: `Text` CSS-defense values for paragraph and heading variants (#78172). Restore them under their original headings. * Storybook: Fix popover-with-slotfill cross-iframe collision boundary `Popover.Popup` stopped accepting `collisionBoundary` directly when #78168 introduced the `Popover.Positioner` slot subcomponent. The prop is now silently ignored on `Popup`, so the cross-iframe story's collision avoidance regressed after the rebase onto trunk. Route the boundary through `Popover.Positioner` (matching the modern `Popover.Popup`'s `positioner` slot pattern) so the popup honors the iframe's clipping edge again. This file is `.jsx` so the type system didn't catch the silent prop-drop. * Draggable: Storybook: refresh AppendElementToOwnerDocument JSDoc The story's JSDoc still described the legacy "escape ancestor stacking context" rationale, which now contradicts the updated `appendToOwnerDocument` JSDoc in `types.ts` for hosts that opt into the `@wordpress/ui` compat overlay slot — where the clone always lives in the body-level slot and the prop is a no-op. Update the story's docblock to mirror the type-level guidance and call out the cross-document fallback exception. * Draggable: CHANGELOG: Call out default-mode in-flow ancestor change The original Draggable entry covered the stacking + cross-document fallback story, but left the load-bearing behavior change for third-party consumers in the PR description only: in the default placement mode (no `appendToOwnerDocument`, no `__experimentalDragComponent`), the clone used to be a DOM descendant of the dragged element's parent. With the slot active, it now lives at the body-level slot regardless. Surface that change directly in the CHANGELOG entry, including a migration hint for consumers that scoped CSS or event delegation on the clone's previous ancestry. * Draggable: Add e2e regression for chip-inside-compat-slot Lock in the structural guarantee that underpins the stacking claim in #78183: when `@wordpress/components`'s `Draggable` runs in a WordPress environment, the drag chip is rendered inside the body-level `[data-wp-compat-overlay-slot]`. That single structural assertion subsumes the visual stacking contract — the slot creates an isolated stacking context with `z-index: 1000000003`, so anything appended into it stacks above any `@wordpress/components` overlay opened mid-drag (which live outside the slot at lower `z-index`s). Asserting structure rather than visual layering keeps the test robust against unrelated overlay z-index churn, and avoids a brittle `elementFromPoint`-style probe across the parent-doc/canvas-iframe boundary. * Storybook: Trim file-level docblocks on playground stories Drop the file-level brain-dump JSDoc from the `draggable-cross-document-fallback` and `popover-with-slotfill` playground stories. The story body and any per-story copy carry the user-facing explanation; the file-level prose was internal reasoning that doesn't belong in the story source. Per mirka's review on PR #78183 (empty-suggestion blocks). * Storybook: popover-with-slotfill story: use public @wordpress/ui API Switch the playground story to consume `Popover` from the public `@wordpress/ui` entry point instead of reaching into `packages/ui/src/popover`. Inline a small `IframePortal` helper locally so the story no longer depends on `packages/ui/src/popover/stories/utils` either (those story utilities are not part of any public surface). Also swap the `Slot` ref from `useRef` to a state setter so the popup re-renders once the slot's container element mounts, which removes a first-render race the previous `useRef` pattern had. Per mirka's review on PR #78183. * Storybook: draggable cross-doc story: load components styles via Storybook bundle Swap the iframe's style injection from a `?inline` import of `packages/components/src/draggable/style.scss` (reaching into another package's source) to Storybook's own `storybook/package-styles/components-ltr.lazy.scss`, which is the canonical bundle of `@wordpress/components` styles for stories. The injected CSS is now broader than strictly necessary (the whole package stylesheet rather than only Draggable's rules), but this is a debug fixture and the cost is negligible. In exchange we drop the cross-package src reach. Per mirka's review on PR #78183. * Storybook: Move cross-document fallback story under "Debug fixtures" The cross-document fallback story is strictly defensive regression coverage and doesn't illustrate a pattern non- maintainers would seek out. Move it under a `Debug fixtures` sub-section in the sidebar so the main `Playground/` namespace stays focused on intended-usage demos. Per mirka's review on PR #78183. * Storybook: Drop redundant `parameters.sourceLink` from playground stories The `source-link` Storybook addon already derives the GitHub source path from `storyData.importPath` when no explicit `parameters.sourceLink` is provided (see `storybook/addons/source-link/manager.ts`). For stories living under `storybook/stories/playground/`, that fallback resolves to the same value the explicit `sourceLink` was hard-coding, so the declaration is pure duplication. Per mirka's review on PR #78183 (empty-suggestion blocks covering the `parameters: { sourceLink: ... }` literal). * Draggable: Migrate styles from SCSS to a CSS module Move the (already small) Draggable stylesheet to a CSS module so its rules travel via `@wordpress/style-runtime` (and therefore into any iframe wrapped in `<StyleProvider>` — e.g. the block-editor canvas) without needing the package-level `build-style/style.css` bundle. Drops the `@use` line from `packages/components/src/style.scss`, following the same shape as the `AlignmentMatrixControl` (#73714/#73757) and `AnglePickerControl` (#73786) migrations. The CSS-module class names are standard (hashed). The legacy `components-draggable__*` / `is-dragging-components-draggable` class names are kept by adding them alongside the hashed ones in the JS `classList.add(...)` calls, since several other Gutenberg packages reference them in their own stylesheets (block-editor's `list-view`, `block-tools`, `block-library`'s `navigation` editor, `edit-widgets`' `widget-area` editor) and block-editor runtime JS reads `is-dragging-components-draggable` off `document.body`. Dropping those names would silently break those consumers. Per mirka's review on PR #78183 (CSS-module option for the iframe story); the corresponding Storybook simplification follows in a separate commit. * Storybook: Simplify cross-document fallback story with StyleProvider Now that Draggable's styles ship as a CSS module routed through `@wordpress/style-runtime`, the cross-document fallback story no longer needs to manually `?inline`-import and inject the whole `components-ltr` SCSS bundle into the iframe's `<head>`. Wrap the portaled iframe content in `<StyleProvider document={iframeDoc}>` from `@wordpress/components` instead — `StyleProvider` calls `registerDocument()` on the iframe document, and the style registry replays every registered CSS module (Draggable included) into that document. The visible behavior is unchanged: the orange clone still tracks the cursor inside the iframe, demonstrating the cross-document fallback. Per mirka's review on PR #78183. * Draggable: CHANGELOG: Move entry to Unreleased and slim it down The Enhancements entry for this PR ended up rolled into the already-cut `33.1.0` release section during an earlier rebase, and had grown to a 700-character paragraph spelling out every edge case (cross-document fallback, `appendToOwnerDocument` semantics, in-flow ancestor migration hints). Move it back to `## Unreleased` and trim to a two-sentence summary in line with the surrounding entries. The dropped detail still lives in the JSDoc, the code comments, and the PR description's <details> blocks. * Draggable: Trim verbose inline code comments Sweep across the comments added by this PR, dropping redundant duplication, narration of self-evident code, and prose that already lives in the PR description / JSDoc: - Drop the duplicate compat-slot note from the `AppendElementToOwnerDocument` story JSDoc (the interaction is already described on the prop's TS JSDoc in `types.ts`). - Tighten the prop JSDoc for `appendToOwnerDocument` to a single short paragraph. - Slim the same-document-only slot guard comment in `Draggable.start()` (the conditional itself reads as "slot if same document"). - Compact the rationale comment for `parameters.docs.story.inline: false` in the Draggable autodocs config to a single explanation. - Trim the structural-stacking assertion comment in the Playwright `draggable-blocks` spec. - Drop the forward-looking "can be removed on a future Stylelint upgrade" note from `CSS_BASELINE_2024_FUNCTIONS`. No behavior change. * @wordpress/ui CHANGELOG: Move #78183 entry to Unreleased The `getWpCompatOverlaySlot()` export bullet was left inside the already-released `## 0.13.0` section when the parallel `@wordpress/components` entry was moved to `## Unreleased`. * Draggable: Keep physical `left` for the invisible drag image Mirroring this offscreen stand-in in RTL has no benefit — either side hides it equally — so revert to the original physical property and silence the logical-properties lint with a targeted comment. * @wordpress/ui CHANGELOG: Trim #78183 entry * @wordpress/ui: Restore unrelated tsconfig change * Storybook: Drop redundant story-name overrides * Draggable: Keep body cursor class global The cursor flip is also triggered by external code (block-editor keyboard drag, etc.) that toggles `is-dragging-components-draggable` directly. Targeting the legacy class globally preserves that flow, which a module-hashed selector silently broke. * Draggable: Guard class arrays against the Jest CSS-module mock `jest-preset-default`'s style mock returns `undefined` for any class, which `classList.add()` would coerce to a literal "undefined" token. Filter falsy entries to keep test DOM clean. * Draggable: Address minor self-review nits - Note why the invisible drag image bypasses the compat slot. - Drop a redundant chip-count assertion in the e2e spec. - Flag the SCSS-only stylelint override pattern explicitly. * Storybook: Group compat-slot fixtures under Debug fixtures Consolidates the manual verification stories (`WP Compat Overlay Slot`, `Popover with SlotFill`) alongside the existing Draggable fixture. * Draggable: Use kebab-case for CSS module class names --- Co-authored-by: ciampo <[email protected]> Co-authored-by: mirka <[email protected]>
What
Apply the same
positionerslot pattern introduced forTooltip/IconButtonin #78089 to the remaining positioned overlays:Popover— breaking. Remove flatside,align,sideOffset,alignOffset,anchor,arrowPadding,collisionAvoidance,collisionBoundary,collisionPadding,stickyfromPopover.Popup. Replace with apositionerslot accepting<Popover.Positioner />.Select— additive. ExposeSelect.Positioner+positionerslot onSelect.Popup. No existing flat positioner props to remove.Autocomplete— additive. Same shape as Select.Also codify the rule in
packages/ui/CONTRIBUTING.mdso future overlay primitives stay consistent.While here, align every overlay's
PortalandPositionerwrapper type on the package's standardComponentProps< typeof _X.Subcomponent >shape — matchingRoot/Trigger/Popupwrappers elsewhere. Type-only sweep, but tightens the public surface (see Breaking change below).Why
Finish the migration started in #78089. After this PR, every
*.Popupin@wordpress/uifollows the same slot-shaped API for bothportalandpositionercustomization, and every Base UI-derived subcomponent type goes through the sameComponentPropsutility.Breaking change
Popover.Popupflat positioner props → slot prop. Migration:Select.PopupandAutocomplete.Popupdid not previously expose flat positioner props, so existing callsites are unaffected.Portalsubcomponent types acrossTooltip,Popover,Select,Autocomplete,Dialog,Drawer,AlertDialogno longer accept Base UI's(state) => …callback form forclassName/styleor the two-arg(props, state) => …form forrender. Runtime behavior is unchanged.Testing Instructions
Storybook →
Design System / Components:Positioning,Anchored,Collision,Cross-iframe,Cross-iframe (SlotFill), andConstrainedshould render and behave identically to before.Slot pattern rule (added to CONTRIBUTING.md)
For each Base UI subcomponent we want to expose to consumers:
Tooltip.Positioner).PopupacceptingReactElement< Omit< MySubcomponentProps, 'children' > >.Popupuses the wrapper with default props.Popupclones the element and injects the subtree aschildren.High-level wrappers that hide
Popup(e.g.IconButton→Tooltip) re-expose the same slot props by the same name and shape. Add new slot props only when a concrete consumer needs them.Use of AI Tools
Cursor + Claude.