Repository navigation
Conversation
…ttons and links Migrate the license activation screen (js-packages/licensing ActivationScreen, rendered at My Jetpack -> Add license) off legacy @wordpress/components and custom-styled primitives onto @wordpress/ui: - Activate button: @wordpress/ui Button with the built-in `loading` prop (replaces the manual Spinner) and disabled while activating. - Success-screen buttons: @wordpress/ui Button navigating via onClick (native button, no anchor); loading state uses the `loading` prop. - Support/product links (x3): normalized to native @wordpress/ui Link, dropping the green and black/underline overrides and the raw <a>. - Button/link styling reduced to WPDS tokens; container spacing moved to a flex gap so the button and its paired link keep their spacing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
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! |
Code Coverage SummaryCannot generate coverage summary while tests are failing. 🤐 Please fix the tests, or re-run the Code coverage job if it was something being flaky. |
| font-weight: var(--wpds-typography-font-weight-medium); | ||
| font-size: var(--wpds-typography-font-size-lg); | ||
| width: auto; | ||
| padding-inline: var(--wpds-dimension-padding-3xl); | ||
| padding-block: var(--wpds-dimension-padding-lg); |
There was a problem hiding this comment.
Do we need these style overrides? Ideally, we'd use the design system as-is without overrides, which are unwieldy to maintain.
There was a problem hiding this comment.
Looks quite off without them, there is not variant for size. Good thing is they are all under a single class, we can easily remove them. Let me get screenshots before calling for some design advise.
There was a problem hiding this comment.
Yah it might just need a design change/decision or keep as custom but good to make that decision intentionally rather than just leave be because that's how it was before. 🙂
There was a problem hiding this comment.
I think you could replace these styles with Notice component.
There was a problem hiding this comment.
likely yes, I was doing a simple pass before deciding to go replacing more stuff. There are other obvious candidates for replacement (TextControl, SelectControl, Modal, etc)
There was a problem hiding this comment.
Yah follow ups seems fine, just flagging. 🙂
There was a problem hiding this comment.
Some parts done in #48188 might be useful for this btw.
There was a problem hiding this comment.
Seems good candidate for Text component.
There was a problem hiding this comment.
These margin styles seem like good to replace with Stack 's gap.
There was a problem hiding this comment.
Looks good to be replaced with Text and Stack.
| <Link | ||
| openInNewTab | ||
| href={ getRedirectUrl( 'jetpack-support-license-activation' ) } | ||
| rel="noopener noreferrer" |
There was a problem hiding this comment.
You can remove rel here; it's not needed in modern browsers. We just removed it in ExternalLink, too.
| <Button | ||
| className="jp-license-activation-screen-success-info--button" | ||
| href={ | ||
| onClick={ navigateTo( |
There was a problem hiding this comment.
Flagging the need for recently added LinkButton, which will be available in the next @wordpress/ui release.
| <Button | ||
| className="jp-license-activation-screen-success-info--button" | ||
| onClick={ navigateTo( redirectSource ) } |
There was a problem hiding this comment.
This can also be switched to LinkButton later.
| onClick={ navigateTo( | ||
| getRedirectUrl( 'license-activation-view-my-plans', { site: siteRawUrl } ) | ||
| ) } |
There was a problem hiding this comment.
This can also be switched to LinkButton later.
… error notice to @wordpress/ui Addresses review feedback (simison) and continues the @wordpress/ui migration: - Form controls: TextControl -> @wordpress/ui InputControl and SelectControl -> @wordpress/ui SelectControl (onValueChange; SelectControl items/value object). Drops the field styling that targeted @wordpress/components markup, including the red/green border feedback (the error notice still communicates it). - Error notice: replace the hand-rolled icon+div/info-box with @wordpress/ui Notice (intent-driven color/icon) + Stack + Text; delete its stylesheet. - Illustration "Contact us" link: drop `rel` (not needed; matches Gutenberg's ExternalLink change). - primary-link: add a TODO to switch the onClick-nav buttons to @wordpress/ui LinkButton once it ships (Gutenberg #77098). The Activate button's WPDS-token overrides are kept intentionally (prominent CTA). Golden-token Modal -> Dialog is deferred to a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The @wordpress/ui Notice sat flush against the input and Activate button, and its rich errorInfo (raw <p> from useGetErrorContent) inherited the parent's 16px p rule while the Notice message rendered at Text body-md (13px) — leaving the secondary help text larger than the primary message. Re-add a small stylesheet that margins the notice and matches the errorInfo text to the notice size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Use the Notice component's own grid layout (Title + Description in the same column) instead of a hand-wrapped Stack, so the internal spacing/hierarchy come from the design system. Render the Description as a <div> since errorInfo is rich block content (<p>/<ol>/links). Keep a small external margin and normalize the errorInfo text to the notice body size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Text + Stack Replace the hand-rolled heading/paragraph and flex containers with @wordpress/ui primitives: the h1 and intro copy become Text (heading-2xl / body-lg), and the column layout becomes nested Stacks (outer space-between to pin the Activate button at the bottom; a logo+heading group; a main-content group). Removes the per-element margins/paddings and the flex CSS in favour of Stack gap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ror fixes - Manual-entry option uses a sentinel value so the SelectControl shows its label (not the empty-value placeholder) while the manual text input is displayed. - Two-column layout: give each column a determinate width (flex:1 + min-width:0) so a long SelectControl value truncates (built-in ellipsis) instead of reflowing the columns; drop the now-redundant field width:100%. - Clear the activation error when the license changes (select pick or manual input) so a stale invalid-key notice doesn't linger. - ProductLink companion link: add line-height so it doesn't look cramped on mobile. - Error notice: drop its external margin (the controls Stack gap owns spacing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Proposed changes
Migrates the license activation screen —
js-packages/licensing'sActivationScreen, rendered at My Jetpack → Add license (admin.php?page=my-jetpack#/add-license) — off legacy@wordpress/components/ custom-styled primitives and onto@wordpress/ui. This screen was skipped by the earlier My Jetpack modernization because its UI lives in the sharedlicensingpackage, not in My Jetpack itself.Buttons
@wordpress/uiButtonusing the built-inloadingprop (replaces the manual<Spinner/>), and now disabled while activating so it can't be double-submitted.@wordpress/uiButton. These only navigate the current tab, so they use anonClickhandler rather thanrender={<a>}— that keeps a native<button>whose ui styling renders untouched (a solid button rendered as an anchor would inherit wp-admin's global link color over its own label). The fetching placeholder uses theloadingprop.--wpds-typography-*,--wpds-dimension-padding-*). Hover/focus/disabled now come from@wordpress/ui.Links — normalized all three to native
@wordpress/uiLink:<a>; now aLinkwithopenInNewTab.<a>color override.ProductLink("View latest backup") — dropped the blackunderline !importantoverride; kept the button's font (weight/size) so it reads as the button's pair.Layout
gapon the container.Findings / notes for reviewers
@wordpress/uiinjects its styles under@layer wp-ui(and some resets at runtime, unlayered, single-class specificity). Custom overrides in this package are unlayered, so they win — but a bare single-class selector ties the runtime reset and loses on source order. The button/link rules are therefore scoped under their container (specificity 0,2,0) to override reliably.@wordpress/theme/design-tokens.css.js-packages/licensingis also consumed by the standalone Jetpack plugin's license page — if that context doesn't load the tokens, the tokenvar()s would need fallbacks. Worth confirming before this leaves draft.--wpds-dimension-padding-3xl(32px) vs the old literal 45px inline — intentionally the WPDS-standard button padding, so buttons are a bit narrower than before.Not in this pass (possible follow-ups)
AdminPage/Container/ColandJetpackLogo(@automattic/jetpack-components), theTextControl/SelectControlfields, and the golden-tokenModal(@wordpress/components) are still on the old primitives.Does this pull request change what data or activity we track or use?
No. Analytics events are unchanged.
Testing instructions
/wp-admin/admin.php?page=my-jetpack#/add-license).