Repository navigation
Conversation
|
@claude please review this PR. |
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Claude finished @kangzj's task in 1m 17s —— View job PR Review
Overall this is a clean, well-reasoned change. The code is minimal, focused on the stated goal, and consistent with the codebase's conventions. A few observations below.
|
|
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! |
…ipt (RSM-3319) Replace the parenthesized inline label "(Preview)" sitting in a plain <span> next to the AI Answers tab title with a semantic <sup> superscript and drop the parens. Visually this leans on the UA's own `vertical-align: super` baseline shift; we only override the font-size (0.7em) and tint the text with the neutral-weak design token so the qualifier reads as metadata instead of competing with the tab title. The translation call moves to _x() with a context note so translators see the badge intent. Removes the ` ` workaround that #48850 had to add against the new flex-based Tabs.Tab — putting the text inside <sup> means the UA superscript handles its own spacing.
…ne-height (claude[bot] review on PR #48852) Two follow-ups from claude[bot]'s review: 1. Drop `font-weight: 500`. The intent was for the qualifier to recede, so falling back to inherited 400 is more consistent with "muted metadata" — the medium weight was adding a small optical boost that worked against the intent. 2. Add explicit `vertical-align: super` and `line-height: 1`. The superscript was leaning on the UA stylesheet for the baseline shift; pinning it here is cheap insurance against a future reset stripping the UA value, and `line-height: 1` is the standard guard against `vertical-align: super` inflating the tab's line-box height.
b9fa359 to
e007a2d
Compare
|
Addressing claude[bot]'s review (issue comment ID
Both addressed in commit |
|
@copilot review |
|
@claude please re-review. |
|
Claude finished @kangzj's task in 45s —— View job PR Re-Review
Both items from the previous review are addressed. The PR is clean. Addressed feedback
Explicit Current state of the rule.jp-search-dashboard-tabs__tab-preview-label {
margin-inline-start: var(--wpds-dimension-padding-xs, 4px);
font-size: 0.7em;
line-height: 1;
vertical-align: super;
color: var(--wpds-color-fg-content-neutral-weak, #50575e);
}Five properties, each doing exactly one thing: spacing, size, line-box containment, baseline position, and tint. No redundancy, no missing pieces. Everything else holds
No further concerns. Ready to merge. |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
Review: LGTM ✅ The changes are clean and well-reasoned. Here's what I checked: JSX (
SCSS (
Tests
Changelog — present, correctly formatted, accurate. One cosmetic note: the commit message has a stray space ("A ddress review…") — not a blocker, just something to keep in mind for the squash message before merge. |
Fixes RSM-3319
Why
The "Preview" qualifier on the AI Answers tab read as part of the title —
AI Answers (Preview)in body weight, with parentheses doing the only work to separate it. Promoting it to a proper superscript turns it into a status marker the eye skims over instead of competing with the tab name, which is what every comparable design system (Material, Atlassian, GitHub, Carbon) does for in-line "Preview" / "Beta" / "New" labels.Proposed changes
dashboard-page.jsx: swap the<span>around(Preview)for a semantic<sup>and drop the parentheses (the visual treatment now does the separating)._x()with a context note so translators see the badge intent rather than just the bare word.dashboard-page.scss: keep the UA'svertical-align: superbaseline shift; only overridefont-size(0.7em), tint with the muted--wpds-color-fg-content-neutral-weaktoken, and add a 4px inline-start margin so the qualifier reads as metadata. workaround that Search Dashboard: restore visible space between "AI Answers" and "(Preview)" tab label #48850 had to add against the new flex-basedTabs.Tab— the text now lives inside<sup>so the UA superscript handles its own separation from the tab title.Screenshots
Related product discussion/links
@wordpress/uitab migration that introduced this label rendering): Search dashboard: adopt the unified @wordpress/ui tab pattern #48846 — RSM-3299 workaround it had to addDoes this pull request change what data or activity we track or use?
No.
Testing instructions
Per the issue's acceptance criteria:
AI Answers ᴾʳᵉᵛⁱᵉʷ—Previewraised above the baseline, smaller, in the secondary-foreground tone.Preview.<sup class="jp-search-dashboard-tabs__tab-preview-label">rather than<span>._x()with the badge-status context note.To verify against a real Jetpack ↔ WPCOM connection (Jurassic Ninja or a connected docker env):
jp build packages/my-jetpack && jp build plugins/jetpack && jp build packages/search && jp build plugins/search<sup class="jp-search-dashboard-tabs__tab-preview-label">Preview</sup>and the visual treatment matches the After screenshot.