ui/Tooltip, ui/IconButton: Add positioner slot API#78089
Merged
Conversation
ciampo
added
[Type] Enhancement
|
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. |
Contributor
|
Nice, I needed this the other day :D |
ciampo
commented
May 8, 2026
ciampo
commented
May 8, 2026
|
Size Change: +179 B (0%) Total Size: 7.93 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 732c158. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25736659751
|
ciampo
changed the title
ciampo
force-pushed
the
ui/icon-button-tooltip-placement
branch
2 times, most recently
from
2345aeb to
c255011
Compare
ciampo
changed the title
Contributor
Author
|
@mirka, following your suggestion, I refactored the approach to use a
I'll apply the same change to the remaining overlay components in #78168 as a follow-up |
Member
|
At least for the primitive components, is it worth revisiting whether to mirror the underlying subcomponent structure of BaseUI rather than exposing it as a prop? I'm worried that we'll continue to run into this kind of issue for the components where we've chosen to absorb multiple BaseUI subcomponents into our own primitive abstraction.
Particularly for a higher-level component, is this actually a behavior that we want to support as a common pattern? Maybe so in this case, though I think the nice thing with these kinds of decisions is that if it's more of an edge-case, a consumer still has the option to compose the underlying primitives themselves (i.e. they're not strictly blocked from achieving the desired outcome). |
Contributor
Author
The main reason why we chose slot props over mirroring base UI is that it allows us to have a much leaner "default" React tree and better ergonomics: consumers would specify
If we do, do you think that the current pattern (ie. exposing them as a "slot prop") is a bad practice and/or a worse tradeoff compared to mirroring base ui structure? I'll also say that this pattern is mostly limited to
Sure, but one of the main reasons for In a sense, exposing |
mirka
approved these changes
May 11, 2026
| args: { | ||
| ...Default.args, | ||
| positioner: ( | ||
| <Tooltip.Positioner side="right" align="center" sideOffset={ 8 } /> |
Member
There was a problem hiding this comment.
I have mixed feelings on whether we should have a dedicated IconButton.Positioner subcomponent, but I think this is fine. In a way, it's clearer that the Positioner is related to the tooltip concerns.
Contributor
Author
There was a problem hiding this comment.
Same mixed feelings, and I got to the same conclusion
Member
I guess I'm not sure I follow the use-cases where we'd have a demand for this feature. There's no use-case in the original message. The
It's probably fine. The "default ergonomics" is a valid point, though we've already accepted that the component structure (especially at the primitive level) is known to be verbose to support the type of flexibility we're introducing here while simultaneously avoiding prop bloat. My concern with the deviation is largely coming from a place that we'd have to maintain this ourselves. Separately, I'd also personally been wondering if we'd be better off just exposing the BaseUI internals directly to reduce our ongoing maintenance burden, in which case adopting their internal structure would make for a smaller leap. |
Contributor
Author
Feature parity, plus a couple of legitimate use cases where tweaking the tooltip position can be useful (example). More generally, I think it's fair to assume that consumers of
The key differences is that with the current approach the props are optional, while if we mirrored the Base UI structure, all subcomponents would be mandatory. Also, in a way, exposing a
This is, to me, the key argument in favor of mirroring Base UI's structure: less maintenance on our end. |
The helper is a generic "fill an optional element-shaped prop with children" function. The old name implied it was portal-specific, which was misleading once we started using the same mechanism for other slot props (e.g. `positioner`). The implementation is unchanged. All seven overlay `Popup` files migrated: - `AlertDialog.Popup` - `Dialog.Popup` - `Drawer.Popup` - `Popover.Popup` - `Select.Popup` (form primitive) - `Autocomplete.Popup` (form primitive) - `Tooltip.Popup` JSDoc on the helper rewritten to describe the generic slot semantics. Stale references to the helper in `Dialog.Popup` and `Drawer.Popup` JSDoc removed entirely — internal utilities should not be named in public-facing JSDoc.
Expose a renderable `Tooltip.Positioner` subcomponent that wraps Base
UI's `_Tooltip.Positioner` with our default styles (box-sizing reset,
`.positioner` class for the `--wp-ui-tooltip-z-index` cascade) and our
default props (`side="top"`, `align="center"`, `sideOffset={ 4 }`).
This component is exposed ahead of being wired into `Tooltip.Popup`'s
composition; the next commit threads it through as the default for a
new `positioner` slot prop, mirroring the `portal` slot pattern.
Tooltip: - `Tooltip.Popup` removes `side`, `align`, and `sideOffset` (breaking). - Replaces them with a `positioner?: ReactElement<Omit<PositionerProps, 'children'>>` slot prop accepting a `<Tooltip.Positioner />` element, mirroring the existing `portal` slot pattern. When omitted, the default `<Tooltip.Positioner />` is used. - The new subcomponent exposes Base UI's full positioner surface (`align`, `alignOffset`, `anchor`, `arrowPadding`, `collisionAvoidance`, `collisionBoundary`, `collisionPadding`, `side`, `sideOffset`, `sticky`, etc.) rather than the previous `Pick<>` subset. IconButton: - Replaces the `tooltipPlacement` enhancement (added earlier in this PR branch and not yet released) with a `positioner` prop forwarded directly to `Tooltip.Popup`'s `positioner` slot. Internal callsites migrated: `Tabs` story, the `Positioning` Tooltip story, and the `IconButton` `WithCustomPositioner` story (renamed from `WithCustomTooltipPlacement`). Also adds a `WithCustomPositioner` Tooltip story showcasing the broader positioner surface.
Reword the Tooltip.Positioner story description and the Tooltip Popup positioner CHANGELOG entry to talk about "the positioner surface" rather than "Base UI's positioner surface". Base UI is an implementation detail and is not referenced elsewhere in the consumer-facing surface for this feature.
Drop align and sideOffset from the WithCustomPositioner story args and description. side is the main legitimate placement use case on a high-level wrapper like IconButton; demonstrating it alone keeps the story focused without implying that exposing the full positioner surface on IconButton is a recommended pattern.
ciampo
force-pushed
the
ui/icon-button-tooltip-placement
branch
from
24bc2ec to
732c158
Compare
Member
|
In many components we already have applied a good amount of compression (folding multiple subcomponents into one) at the primitive level. It's hard to say what the "best" amount of abstraction is even at the primitive level, since we don't have a ton of use cases at the moment. But in my mind, I'm still considering the primitives to be somewhat opinioniated in terms of visual design consistency, and that often dictates that certain subcomponents be folded into one. I'm generally on board with the current level of subcomponent folding, and haven't yet seen anything that strongly pushes me to reconsider it, including the maintenance angle. But I'm open to restructuring in the future when we have good reasons to do so. |
Contributor
Author
|
Reshaping the subcomponents APIs would be a large, breaking change anyway, so I feel like we should merge this PR anyway and, if needed, continue the conversation separately. I'll align the rest of the overlays in #78168. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replace the flat
side/align/sideOffsetprops onTooltip.Popupwith apositionerslot prop accepting a<Tooltip.Positioner />element, and expose the samepositionerprop onIconButton. Mirrors the existingportalslot pattern.Why
IconButtonpreviously had no escape hatch to customize how its tooltip is placed.IconButton(e.g.tooltipPlacement: { side, align, sideOffset }), reuse the slot-shaped pattern we already established forportal. Same mental model across all overlay slots.Tooltip.Positionerexposes Base UI's full positioner surface (alignOffset,collisionPadding,sticky, etc.) instead of the previous hand-picked subset onTooltip.Popup.How
Three commits:
Popupfiles).Tooltip.Positionersubcomponent (additive).Tooltip.Popupaccepts the newpositionerslot and dropsside/align/sideOffset(breaking).IconButtonexposes a matchingpositionerprop.Breaking change
Testing Instructions
Storybook →
Design System / Components:Screenshots
Default placement is unaffected:
Custom placement:
Slot pattern rule (applied here, expanded in follow-up)
For high-level overlay wrappers (those composing a
*.Popupinternally without exposing it), customization is offered through slot props that accept a React element of the corresponding primitive subcomponent. Same names and shape across the primitive's*.Popupand any higher-level wrapper. Today's slots:portal,positioner. Add slots only when there's a concrete consumer.A follow-up PR extends
positionerto the other positioned overlays (Popover,Select,Autocomplete) and documents the rule inpackages/ui/CONTRIBUTING.md.Use of AI Tools
Cursor + Claude.