Repository navigation
Search dashboard: adopt the unified @wordpress/ui tab pattern - #48846
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
This comment has been minimized.
This comment has been minimized.
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
4b1ff2f to
51700e3
Compare
|
Pivoted per design direction — force-pushed
Updated screenshots show the before/after (the change is subtle: just the absence of the separator). PR body and title updated to reflect the new (much narrower) scope. |
51700e3 to
48d2060
Compare
|
Revised approach — force-pushed The wrapper, defined in PR title and body updated; screenshots refreshed. cc @adamwoodnz — does this look right? |
48d2060 to
d60ce2b
Compare
Wraps `Tabs.List` from `@wordpress/ui` in `<div class="jp-admin-page-tabs">` with `variant="minimal"`, matching the pattern used by Podcast, Newsletter, Scan, and the other recently modernized Jetpack admin dashboards. The wrapper (defined in `@automattic/jetpack-base-styles/admin-page-layout`) sticky-pins the strip, applies the `--wpds-*` border + background tokens that match `@wordpress/admin-ui`'s page-header treatment so the strip reads as a visual continuation of the header, and aligns the first tab's start edge with the page title. `variant="minimal"` tightens the per-tab padding for the compact look the other dashboards use. No `AdminPage` prop overrides, no custom SCSS.
d60ce2b to
d534998
Compare
- `<Tabs.Tab>` from `@wordpress/ui` declares `font-size:
var(--wpds-typography-font-size-md)` inside `@layer wp-ui-components`.
wp-admin core ships an unlayered `button { font-size: inherit }` reset
that wins over any layered rule, so tabs silently inherit 16px from
their surrounding context instead of the 13px the token specifies.
Re-apply the token from an unlayered selector so the design system
lands as intended (tabs at 13px, smaller than the 15px page title).
- `.jp-admin-page-tabs` wrapper padding-inline set to the page header's
2xl (24px) so the first tab's content edge sits flush with the header
across all screen widths. Required because `variant="minimal"` drops
the per-<Tabs.Tab> 16px inline padding the shared wrapper's 8px
default relies on for alignment.
- Selectors are scoped under `body.jetpack_page_jetpack-search` to beat
the body-prefixed rule from `@automattic/jetpack-base-styles`. All
`var()` usages carry the token's literal value as a fallback.
1509b63 to
d1b5322
Compare
| { __( 'AI Answers', 'jetpack-search-pkg' ) }{ ' ' } | ||
| <span className="jp-search-dashboard-tabs__tab-preview-label"> | ||
| { __( '(Preview)', 'jetpack-search-pkg' ) } | ||
| </span> |
There was a problem hiding this comment.
Can we please insert a space between 'AI Answers' and '(Preview)'? It hurts my eyes 😬
There was a problem hiding this comment.
Addressed in follow-up #48850 (commit c8055f2). The new flex-based Tabs.Tab was collapsing the { ' ' } JSX whitespace between the text node and the span, so I moved a inside the preview-label span — sibling-whitespace collapsing in flex doesn't reach inside the element's own text content, so the gap now renders reliably. 🙏
…eview)" tab label (#48850) The new flex-based `Tabs.Tab` from `@wordpress/components` collapses JSX `{ ' ' }` whitespace between sibling nodes, so the "(Preview)" suffix rendered flush against "AI Answers". Move a non-breaking space inside the preview-label span so it survives the flex layout. Follow-up to #48846 (review comment #3245711759).
Fixes RSM-3299.
Why
The Search dashboard's tab strip didn't follow the unified
@wordpress/uiTabspattern that the rest of the modernized Jetpack admin pages adopt (Podcast, Newsletter, Scan, VideoPress, Protect). Two specific problems were visible:<Tabs.Tab>from@wordpress/uideclaresfont-size: var(--wpds-typography-font-size-md)inside@layer wp-ui-components. wp-admin core ships an unlayeredbutton { font-size: inherit }reset, and unlayered rules win over layered ones, so the tab text was silently inheriting 16px from its surrounding context instead of the 13px the design token specifies. Result: tab labels were taller than the 15px page title — visual hierarchy inverted..jp-admin-page-tabswrapper adds only 8px of inline padding, designed to compose with the default<Tabs.Tab>'s 16px (8 + 16 = 24px = the admin-ui Page header's 2xl padding). Withvariant="minimal"the per-tab padding is 0, so the first tab started 16px shy of the title's left edge.Proposed changes
<Tabs.List variant="minimal">in<div className="jp-admin-page-tabs">— the canonical wrapper from@automattic/jetpack-base-styles/admin-page-layoutused by every recently modernized Jetpack admin dashboard. It sticky-pins the strip, applies the--wpds-*border + background tokens that match@wordpress/admin-ui's page-header treatment, and now provides 24px of inline padding so the first tab is flush with the header start across all widths.--wpds-typography-font-size-mdto[role="tab"]from an unlayered selector so the design token actually lands. This compensates for wp-admin'sbutton { font-size: inherit }reset shadowing the layered@wordpress/uideclaration.var(--wpds-*)usages carry the token's literal value as a fallback, matching the convention in@automattic/jetpack-base-stylesand other Jetpack packages.No changes to
<AdminPage>props, the page title, or the subtitle — they all keep their canonical token-driven values.Screenshots
Related product discussion/links
projects/packages/podcast/src/dashboard/index.tsx), Newsletter (projects/packages/newsletter/_inc/components/newsletter-page.tsx), Scan, VideoPress, Protect.Does this pull request change what data or activity we track or use?
No.
Testing instructions
font-size: 13px(the value of--wpds-typography-font-size-md).<h1>stays at its native15px(admin-ui<Text variant="heading-lg">).tab=query param updates (?tab=plan-usage/?tab=settings/?tab=ai-answers).