Tooltip migration: dataviews consumers (3/5)#78470
Conversation
|
Size Change: +32.4 kB (+0.4%) Total Size: 8.16 MB 📦 View Changed
ℹ️ View Unchanged
|
| { fieldLabel } | ||
| </span> | ||
| </WCTooltip> | ||
| <Tooltip.Root> |
There was a problem hiding this comment.
Quickest way to test this tooltip is to
diff --git i/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx w/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx
index fcbd9f34d76..256374438f8 100644
--- i/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx
+++ w/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx
@@ -49,8 +49,8 @@ export default function SummaryButton< Item >( {
} ) {
const { labelPosition, editVisibility } =
field.layout as NormalizedPanelLayout;
- const errorMessage = getFirstValidationError( validity );
- const showError = touched && !! errorMessage;
+ const errorMessage = 'Test error message'; //getFirstValidationError( validity );
+ const showError = true; //touched && !! errorMessage;
const labelClassName = getLabelClassName( labelPosition, showError );
const labelContent = getLabelContent( showError, errorMessage, fieldLabel );
const className = clsx(
Then, "quick edit" a page in the site editor and hover over the little warning icon:
There was a problem hiding this comment.
Exposed the errorMessage to assistive technology by rendering it inside a VisuallyHidden component (so that it chains to the fieldLabel strings)
| <WCIcon icon={ errorIcon } size={ 16 } /> | ||
| </span> | ||
| </WCTooltip> | ||
| <Tooltip.Root> |
There was a problem hiding this comment.
Quickest way to test this tooltip is to
diff --git i/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx w/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx
index fcbd9f34d76..44232673f5e 100644
--- i/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx
+++ w/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx
@@ -47,10 +47,10 @@ export default function SummaryButton< Item >( {
onClick: () => void;
'aria-expanded'?: boolean;
} ) {
- const { labelPosition, editVisibility } =
- field.layout as NormalizedPanelLayout;
- const errorMessage = getFirstValidationError( validity );
- const showError = touched && !! errorMessage;
+ const { editVisibility } = field.layout as NormalizedPanelLayout;
+ const labelPosition = 'none';
+ const errorMessage = 'Test error message'; //getFirstValidationError( validity );
+ const showError = true; //touched && !! errorMessage;
const labelClassName = getLabelClassName( labelPosition, showError );
const labelContent = getLabelContent( showError, errorMessage, fieldLabel );
const className = clsx(
Then, "quick edit" a page in the site editor and hover over the little warning icon:
There was a problem hiding this comment.
Exposed the errorMessage to assistive technology via aria-label (and role="img" so that the span actually exposes that aria-label)
| 'has-values': hasValues, | ||
| 'is-not-clickable': isLocked, | ||
| } | ||
| <Tooltip.Root> |
| </Tooltip.Popup> | ||
| </Tooltip.Root> | ||
| { canResetOrRemove && ( | ||
| <Tooltip.Root> |
There was a problem hiding this comment.
This one is an accessibility regression, so I think we should fix here. Should be just a matter of adding an aria-label to the button.
There was a problem hiding this comment.
Extracted tooltip text to resetOrRemoveLabel variable, used it as the aria-label of the button
| { field.header } | ||
| </FlexItem> | ||
| </WCTooltip> | ||
| <Tooltip.Root> |
There was a problem hiding this comment.
Something to explore separately but why are we showing a tooltip here? Seems redundant.
There was a problem hiding this comment.
Looking a bit more into it, I suspect those are the main two reasons for why a Tooltip was added in the first place:
- The visual text is
field.header(which defaults to be the same asfield.labelbut can be anyReactElement); therefore, a tooltip consumingfield.labelmay guarantee a more accessible content (especially when paired witharia-labelon the always-visible anchor text); - the tooltip would be also useful when the text gets truncated
We should discuss and revisit in a follow-up — we may not necessarily need a tooltip.
efaccfd to
92025d5
Compare
There was a problem hiding this comment.
Code diff gets a lot smaller when hiding whitespace changes
92025d5 to
bee91f0
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. |
| { fieldLabel } | ||
| </span> | ||
| </WCTooltip> | ||
| <Tooltip.Root> |
| <WCIcon icon={ errorIcon } size={ 16 } /> | ||
| </span> | ||
| </WCTooltip> | ||
| <Tooltip.Root> |
| </Tooltip.Popup> | ||
| </Tooltip.Root> | ||
| { canResetOrRemove && ( | ||
| <Tooltip.Root> |
There was a problem hiding this comment.
This one is an accessibility regression, so I think we should fix here. Should be just a matter of adding an aria-label to the button.
bee91f0 to
5d815f0
Compare
Migrates 4 consumer call sites in `@wordpress/dataviews` 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): - dataviews/dataviews-filters/filter (filter chip + reset/remove buttons; non-interactive `<div role="button">` and `<button>` triggers) - dataviews/dataform-layouts/panel/utils/get-label-content (validation error tooltip on `<span>` label content) - dataviews/dataform-layouts/panel/summary-button (validation error tooltip on `<span>` icon) - dataviews/dataviews-layouts/grid/composite-grid (field-label tooltip on `FlexItem`) The mechanical rewrite was produced by the jscodeshift codemod landed in PR 1; import placement was finished by hand. `placement="top"` is the default for the new `Tooltip` so no `<Tooltip.Positioner>` is emitted — visual placement is unchanged. Shell-level `<Tooltip.Provider>`s mounted in PR 2 (edit-post + edit-site) and PR 5 (boot) cover dataviews wherever it is composed inside one of those shells.
5d815f0 to
52f5b9d
Compare
|
@mirka all feedback should be addressed, I will merge this PR as soon as CI checks pass. Happy to iterate in follow-ups in case I missed anything |
|
Flaky tests detected in 52f5b9d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26440135854
|
…ended Now that the in-repo migration of `Tooltip` consumers from `@wordpress/components` to `@wordpress/ui` is complete (PRs #78411, #78466, #78470, #78691, #78692), this PR flips the recommendation: - `@wordpress/eslint-plugin` (`use-recommended-components`): - Adds `Tooltip` to the `@wordpress/ui` allow-list. - Adds a `Tooltip` entry to the `@wordpress/components` deny-list pointing to `Tooltip` from `@wordpress/ui`. - `@wordpress/components` Storybook story: flips `componentStatus.status` from `recommended` to `not-recommended` and adds a `notes` field pointing to `@wordpress/ui`. - `@wordpress/ui` Storybook story: flips `componentStatus.status` from `use-with-caution` to `recommended` and drops the previous compat caveat (resolved by #78441). - Removes the per-import `// eslint-disable-next-line @wordpress/use-recommended-components` directives that were added during the migration in PRs 1-3 (20 files across `block-editor`, `block-directory`, `editor`, `edit-post`, `edit-site`, and `dataviews`). Note: this PR is based on trunk and therefore does NOT yet remove the analogous directives added by PRs 4 and 5 (#78691, #78692). Once those merge, this PR will be rebased and the remaining directives removed in the same commit.



What?
Follow up to #78095 and #78411 (now landed on trunk).
Migrates 4 consumer call sites in
@wordpress/dataviewsfrom the legacyTooltip(@wordpress/components) to the new compositionalTooltip(@wordpress/ui, built on base-ui). Part 3 of a 5-PR series.Sites migrated in this PR
dataviews/components/dataviews-filters/filter— filter chip (non-interactive<div role="button">trigger) and the adjacent reset/remove<button>trigger (a11y fix: the reset/remove button now carriesaria-labelmatching the tooltip text, restoring an accessible name lost when the legacyaria-describedbyinjection went away)dataviews/components/dataform-layouts/panel/utils/get-label-content— validation error tooltip on the field label<span>(a11y fix: error message now exposed to AT inline via a<VisuallyHidden>child)dataviews/components/dataform-layouts/panel/summary-button— validation error tooltip on the icon<span>(a11y fix: icon-only span now usesrole="img" aria-label={ errorMessage })dataviews/components/dataviews-layouts/grid/composite-grid— field-label tooltip on<FlexItem>Full 5-PR plan
block-editor + block-directory (also lands the codemod)— Tooltip migration: block-editor + block-directory consumers (1/5) #78411 ✅ landededitor + edit-post + edit-site (+ shell-level— Tooltip migration: editor + edit-post + edit-site consumers (2/5) #78466 ✅ landedTooltip.Provider)Tooltip.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; only import placement was 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/dataviewsplacement="top"is the default for the newTooltip, so no<Tooltip.Positioner>is emitted for these sites — visual placement is unchanged.Why no `` in this package?
Dataviews is a library that's composed inside one of the editor shells (post editor, site editor, dashboard). The shell-level
<Tooltip.Provider>s mounted in PR 2 (edit-post + edit-site) and PR 5 (boot) cover dataviews automatically.Testing Instructions
×button on a filter chip with active values — tooltip "Reset" / "Remove".field.labeltooltip should appear.npm run test:unit -- packages/dataviews— Jest unit tests should pass.Testing Instructions for Keyboard
Esc.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 (or repro steps) for 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.