Repository navigation
Components: Move Newsletter's placement card to @automattic/jetpack-components as CheckboxCard - #53733
Conversation
Fixes CM-978 The Sharing & Likes screen needs the same selectable placement cards as the Subscriptions section of Jetpack > Newsletter (CM-976). Moving the card to @automattic/jetpack-components lets both screens share one component, so they look and behave the same. The card ships with a PlacementCardGrid wrapper, the two-column grid that drops to one column below 720px, so both screens lay the cards out the same way. Both are exported from the main entry point and from a ./placement-card subpath: wp-build screens import components through subpaths so they don't bundle the package's whole set of exports. Newsletter now imports the shared card and grid and drops its own copy. Its illustrations stay in the newsletter package. The props are unchanged; the classes move from jetpack-newsletter-placement__* to jp-placement-card__*, and "Preview and edit" is translated against the jetpack-components domain.
|
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! |
… test The card only renders the link when it gets a previewUrl; the saved-state gating lives in the Newsletter section and is tested there.
… renders CheckboxControl only renders its label element when given a non-empty label, so the rule hiding it matched nothing.
Code Coverage Summary2 files are newly checked for coverage.
|
…d-shared-component
…d-shared-component
The card used CheckboxControl from @wordpress/components, which the new @wordpress/use-recommended-components lint rule flags, and needed CSS overrides to strip that control's chrome and to make the illustration span the card. The checkbox is now the @wordpress/ui Checkbox primitive inside a Field, with the illustration as the Field's label, so a click on it still toggles the card. The title still names the checkbox through aria-labelledby, and still sits outside the label. Card.FullBleed makes the illustration span the card edge to edge, and the focus ring uses the outset-ring__focus mixin from @wordpress/base-styles, so it matches core's components. None of the component overrides remain.
…de list The file moved to @automattic/jetpack-components on this branch, so CI's exclude-list check flags its entry as fixed.
|
BTW the component name "PlacementCard" refers closely to subscription form's placement; something more generic might work as Jetpack component name better. E.g. |
"Placement" describes what Newsletter and Sharing & Likes use the card for, not what the card is: a card with a checkbox, an illustration and a title. A generic name fits a component shared across Jetpack better. PlacementCard, PlacementCardGrid and their props types become CheckboxCard, CheckboxCardGrid, CheckboxCardProps and CheckboxCardGridProps; the subpath export moves from ./placement-card to ./checkbox-card, and the classes from jp-placement-card* to jp-checkbox-card*. Nothing outside this branch used the old names.
|
Good point, thanks! Renamed in ec6b333: it's now |
Fixes CM-978
Proposed changes
Newsletter's Subscriptions section draws its placements as selectable cards, but that card lived inside the Newsletter package. The Sharing & Likes screen needs the same cards for its template placements (CM-976), so this moves the card to
@automattic/jetpack-components, where both screens can use it.CheckboxCardis a card with a checkbox: an illustration, a title under it that also names the checkbox, and an optional "Preview and edit" link. Only a click on the illustration toggles it; the title and link don't. Props are the ones Newsletter already used (id,name,title,illustration,checked,onChange,previewUrl,onPreviewClick,disabled).CheckboxCardGridlays the cards out in two columns, one below 720px, so both screens use the same grid.@automattic/jetpack-components/checkbox-cardsubpath. wp-build screens import components through those subpaths, so they don't bundle every export the package has.jp-checkbox-card*, and "Preview and edit" uses thejetpack-componentstext domain.Jetpack > Newsletter looks the same as on trunk, apart from a few checkbox details (see the note under Screenshots) :)
Screenshots
Note
@simison suggested a few changes in the review comments on the card's
index.tsxandstyle.scss(thanks!), so af7a007 rebuilds the card from@wordpress/uiprimitives:Checkboxprimitive inside aField, with the illustration as the field's label, instead ofCheckboxControlfrom@wordpress/components. Clicking the illustration still toggles the card, and the title still names the checkbox without toggling it.Card.FullBleedmakes the illustration span the card, so the CSS that reset the old checkbox's spacing is gone.outset-ring__focus()mixin from base styles.@simison also suggested a more generic name, since nothing in the card is specific to placements, so ec6b333 renames it from
PlacementCardtoCheckboxCard. The subpath export is now./checkbox-card.The first version of this PR was pixel-identical to trunk; this one isn't quite. On a phone, the checkbox is now 16px instead of 24px (the old
CheckboxControlgrew on small screens, the primitive doesn't), and on desktop the check mark is drawn a little differently. The screenshots below show the current version.Jetpack > Newsletter > Settings, Subscriptions section, with two placements saved:
A focused card shows the checkbox's own focus ring and the card outline:
The new Storybook story:
Rationale
Props stay the same. Keeping Newsletter's props means Newsletter barely changes, and CM-976 builds on an API that already works on one screen. The card is a checkbox, though, so it doesn't handle an either/or choice between cards; a section that needs one card at a time (CM-976) enforces that itself.
Styles come with the card. The card imports its own stylesheet, like the other subpath components, so Newsletter's
style.scssno longer loads it. The card's CSS still lands in both of Newsletter's bundles, the wp-build dashboard and the legacy settings page.Related product discussion/links
Gravatar, from Forms to./gravatarDoes this pull request change what data or activity we track or use?
No.
Testing instructions
On a connected Jetpack site running a block theme:
In Storybook, open JS Packages > Components > Checkbox Card and click through its stories.