Tooltip migration: editor + edit-post + edit-site consumers (2/5)#78466
Tooltip migration: editor + edit-post + edit-site consumers (2/5)#78466ciampo wants to merge 1 commit into
Conversation
|
Size Change: +92.5 kB (+1.15%) Total Size: 8.13 MB 📦 View Changed
ℹ️ View Unchanged
|
| aria-label={ __( 'Drag to resize' ) } | ||
| aria-describedby={ separatorHelpId } | ||
| { ...bindDragGesture() } | ||
| <Tooltip.Root> |
| aria-valuenow={ | ||
| frameRef.current?.resizable?.offsetWidth || | ||
| undefined | ||
| <Tooltip.Root> |
| { commentDateText } | ||
| </time> | ||
| </WCTooltip> | ||
| <Tooltip.Root> |
| if ( name && ( ! showBadge || label ) ) { | ||
| return <WCTooltip text={ name }>{ avatar }</WCTooltip>; | ||
| return ( | ||
| <Tooltip.Root> |
| } } | ||
| onClick={ () => blockRef.current?.focus() } | ||
| aria-label={ STATUS_LABELS[ status ] } | ||
| <Tooltip.Root> |
| key="handle" | ||
| role="separator" | ||
| aria-orientation="vertical" | ||
| <Tooltip.Root> |
| { previewContent } | ||
| </div> | ||
| </WCTooltip> | ||
| <Tooltip.Root> |
There was a problem hiding this comment.
e849468 to
d19cf3d
Compare
d19cf3d to
3f27b09
Compare
|
Flaky tests detected in 3f27b09. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26279118535
|
3f27b09 to
ef9deb2
Compare
ef9deb2 to
2a411e3
Compare
Migrates 7 consumer call sites in `@wordpress/editor`,
`@wordpress/edit-post`, and `@wordpress/edit-site` from the legacy
`Tooltip` exported by `@wordpress/components` to the compositional
`Tooltip` exported by `@wordpress/ui` (built on top of base-ui under
the hood):
- editor/collab-sidebar/note-byline (non-interactive `<time>` trigger;
visual-only on hover)
- editor/collaborators-presence/avatar (non-interactive `<div role="img">`
trigger; visual-only on hover)
- editor/post-revisions-preview/diff-markers
- editor/resizable-editor/resize-handle
- editor/template-actions-panel/block-theme-content
- edit-post/layout (resize separator)
- edit-site/resizable-frame
The mechanical rewrite was produced by the jscodeshift codemod landed in
PR 1; import placement / ordering and a `jsx-a11y` disable directive
that the codemod could not preserve were finished by hand.
Also mounts a shell-level `<Tooltip.Provider>` inside
`edit-post/layout` and `edit-site/layout`, so tooltips inside each
editor coordinate as a group (e.g. once the first tooltip in a group
has been shown, subsequent siblings open instantly). PR 5 will do the
same for the new dashboard / `@wordpress/boot` shell.
Avatar unit tests previously relied on the legacy `Ariakit.TooltipAnchor`
adding `tabindex="0"` on a non-interactive trigger; they have been
rewritten to verify tooltip *presence* by hovering the avatar and
asserting the popup content becomes visible (wrapped in a
`Tooltip.Provider` with `delay={ 0 }` so the assertion doesn't race the
real-world hover delay).
The `edit-post/layout/index.native.js` consumer is intentionally left
on the legacy import: it uses `<WCTooltip.Slot>`, which is the
SlotFill pattern of the legacy implementation and has no equivalent in
the new compositional `Tooltip`.
2a411e3 to
fd8c681
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. |
mirka
left a comment
There was a problem hiding this comment.
Accessibility issues in three out of four call sites… Yes, tooltip primitive usage seems to be correlated to custom interactive components, and therefore prone to implementation errors. Good insights into usage patterns.
| <Tooltip.Root> | ||
| <Tooltip.Trigger | ||
| render={ | ||
| /* role="separator" does in fact support aria-valuenow */ |
There was a problem hiding this comment.
Is this comment still needed? Looks like it was just there to justify the eslint-disable.
| tabIndex={ 0 } | ||
| aria-label={ tooltipText } | ||
| onClick={ () => setIsSwapModalOpen( true ) } | ||
| onKeyPress={ () => setIsSwapModalOpen( true ) } |
There was a problem hiding this comment.
Seems like this onKeyPress would open the modal on a tab key press too 😅 Perhaps another one for the accessibility follow up list.
| */ | ||
| function renderAvatar( ui: React.ReactElement ): ReturnType< typeof render > { | ||
| return render( | ||
| <Tooltip.Provider delay={ 0 } closeDelay={ 0 }> |
There was a problem hiding this comment.
This won't error on CI because this test file happens to be exempt from type checking, but closeDelay is currently not part of our official API surface. Should we support all the upstream props?







What?
Follow up to #78095 and #78411 (now landed on trunk).
Migrates 7 consumer call sites in
@wordpress/editor,@wordpress/edit-post, and@wordpress/edit-sitefrom the legacyTooltip(@wordpress/components) to the new compositionalTooltip(@wordpress/ui, built on base-ui). Part 2 of a 5-PR series; also mounts the shell-level<Tooltip.Provider>for the post and site editors.Sites migrated in this PR
editor/components/collab-sidebar/note-byline— non-interactive<time>trigger; visual-only on hover, flagged for a11y follow-upeditor/components/collaborators-presence/avatar— non-interactive<div role="img">trigger; visual-only on hover, flagged for a11y follow-upeditor/components/post-revisions-preview/diff-markerseditor/components/resizable-editor/resize-handleeditor/components/template-actions-panel/block-theme-contentedit-post/components/layout(resize separator)edit-site/components/resizable-frameedit-post/components/layout/index.native.jsis intentionally left on the legacy import — it uses<WCTooltip.Slot>, which is the SlotFill pattern of the legacy implementation and has no equivalent in the new compositionalTooltip.Full 5-PR plan
block-editor + block-directory (also lands the codemod)— Tooltip migration: block-editor + block-directory consumers (1/5) #78411 ✅ landedTooltip.Provider) ← this PRTooltip.Provider)Why?
#78095 introduced the new compositional
TooltipAPI in@wordpress/ui. This PR series migrates all consumers outside@wordpress/componentsitself to the new API, so that future tooltip work (delays, providers, positioner, base-ui upgrades) can happen in one place.Note
Call sites inside
@wordpress/components(notablyButton's internal Tooltip andTooltipInternalContext) stay on the legacy implementation — tracked as a separate follow-up.How?
Each
<Tooltip>is rewritten to the compositional API by the jscodeshift codemod landed in #78411; import placement / ordering and onejsx-a11ydisable directive that the codemod couldn't preserve were finished by hand. Each migratedTooltipimport also carries a per-site// eslint-disable-next-line @wordpress/use-recommended-componentsdirective (matching the approach in #78411) —Tooltipfrom@wordpress/uiis not yet on the recommended allow-list and we'd rather flip the recommendation in one go after the migration series has bedded in.Codemod
npx jscodeshift -t tools/codemods/tooltip-components-to-ui.js \ --extensions=js,jsx,ts,tsx --parser=tsx \ packages/editor packages/edit-post packages/edit-siteThe codemod's API mapping and bailouts are documented in #78411.
Shell-level `Tooltip.Provider`
Mounted in
packages/edit-post/src/components/layout/index.jsandpackages/edit-site/src/components/layout/index.js, inside the existing<SlotFillProvider>, so that tooltips throughout each editor coordinate as a group: once the first tooltip in a group has been shown, subsequent siblings open instantly. PR 5 will do the same for the new dashboard /@wordpress/bootshell.Avatar test rewrite
packages/editor/src/components/collaborators-presence/avatar/test/index.tsxpreviously relied on the legacyAriakit.TooltipAnchoraddingtabindex="0"on a non-interactive trigger. base-ui'sTooltip.Triggerdoesn't do that. The affected tests now verify tooltip presence by hovering the avatar and asserting the popup content becomes visible (the helper wraps the test tree in aTooltip.Providerwithdelay={ 0 }so the assertion doesn't race the real-world hover delay).The assertions follow the same idiom as
packages/ui/src/tooltip/test/index.test.tsx:await screen.findByText( name )for the positive cases (no moregetByText( … , { selector: 'div *' } )selector-internal hack) and structural counts/string equality for the negative cases. No assertion now relies on the popup's internal DOM shape.Testing Instructions
edit-post/layout) and the site editor canvas resize handle (edit-site/resizable-frame) — both stayed on<button role="separator">with the samearia-*plumbing they already had.Tooltip.Providercoordinating the group).npm run test:unit -- packages/editor packages/edit-post packages/edit-site— Jest unit tests should pass.Testing Instructions for Keyboard
Esc.<time>(incollab-sidebar/note-byline) and<div role="img">(incollaborators-presence/avatar) triggers are not keyboard-reachable on purpose; flagged above for a11y follow-up.Screenshots or screencast
No visual change is intended versus the legacy
Tooltip(same placement, same text, same hover/focus behaviour). Each migrated site has a dedicated inline review comment on this PR with a screenshot of the post-migration tooltip in context.Use of AI Tools
This PR was authored with assistance from Cursor (Claude). All changes were reviewed by a human before being committed; the codemod and migrated files were also exercised locally (Jest, lint, Prettier) before being pushed.