Social: build the Overview tab on the modernization chassis (PR 2 / #48824)#48829
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?
|
|
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! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Social plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 3 files.
1 file is newly checked for coverage.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
There was a problem hiding this comment.
Please don't allow your AI to do this. It clutters the PR, it'll break the PR description when removed, and at some point you're going to forget to remove them before merging.
|
Another thing to look at when you're adding |
f6c81e3 to
7d0af04
Compare
anomiex
left a comment
There was a problem hiding this comment.
Seems like there's a lot going on in this PR to hack around shortcomings in the "chassis" build. But I'll leave reviewing that to people who work on the code; structurally, my concerns appear to be addressed.
…48824) Adds the Overview tab body and an "Add account" page-header action. The body wraps the existing `ConnectionManagement` list in a WPDS `Card.Root` (with the legacy `<h3>` and bottom button suppressed via new opt-in props so the legacy admin page is unchanged), and preserves the connection-error notice and JITM mount-point above the card. `ConnectionScreen` (site not connected) and `PricingPage` (free plan, pricing nudge not dismissed) still pre-empt the tabs the way they pre-empt the legacy dashboard, but the check moved to PHP: a new `should_preempt_to_legacy()` on `Social_Admin_Page` routes the menu callback back to the legacy `SocialAdminPage` shell when either holds, so those flows render exactly as today and the wp-build esbuild bundle stays free of the `@automattic/jetpack-connection` disconnect-dialog asset graph. Drive-by infrastructure to let the chassis re-use the legacy connection / services components without inflating wp-build: * Inline the previously-imported `default-avatar.svg` as JSX inside `ConnectionIcon` — same fallback, one less binary asset to bundle. * Switch the six service-walkthrough `.webp` imports in `services/utils.tsx` to runtime URLs built from `JetpackScriptData.social.assets_url`. Webpack copies `_inc/assets/` verbatim into `build/assets/` via the new `CopyWebpackPlugin` step so both bundlers resolve the same URL. * Expose a `./use-connection-error-notice` sub-path from `@automattic/jetpack-connection` and import via that path from the chassis, so esbuild stops pulling the disconnect-dialog `*.jpg` illustrations through the package barrel. * Expose `social-logo-colors.css` from the `social-logos` package via a `./colors.css` sub-path (and define the missing `--color-bluesky` brand variable). The chassis pulls it from `SocialPage` so the per-service brand colours resolve in the modal — the legacy admin bundle inlines these via `postcss-custom-properties { preserve: false }`, which the chassis esbuild pipeline doesn't run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
21af691 to
8a66487
Compare
* Paint the canvas with the `#fcfcfc` design surface, applied to the body, the wp-admin content wrappers, and the `admin-ui` Page itself. The natural `--wpds-color-bg-surface-neutral` token would be the cleanest fit, but a nested admin-ui `ThemeProvider` redefines the variable to `#fff` inside `.admin-ui-page`, so the variable resolves to white in that scope. Pinning the hex value keeps the canvas consistent regardless of which `ThemeProvider` wraps the descendant. * Cap the Overview column at 720px and centre it (`margin-inline: auto`) so the connected-accounts card sits in a comfortable reading measure on wide viewports. * Hide the "Connected accounts" `Card.Title` when the empty state renders — the empty state already carries the heading, and the duplicate title looked odd above the icon. * Centre the empty state inside the card (icon → title → description → CTA stack on the centerline) by wrapping it in a flex column helper with `text-align: center`. * Mount `ThemedConnectionsModal` at the tab level when the empty state renders. `ConnectionManagement` brings its own modal when the account list is non-empty, but switching to the empty state would unmount that consumer and orphan the page-header / empty-state "Add account" buttons. The conditional ensures exactly one modal instance is mounted at all times. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nline PR 2 inlined the default-avatar SVG so the chassis esbuild pipeline could bundle ConnectionIcon, but the `.wrapper img` rule (border-radius, 2px white border, white background, 24×24) no longer matched once the fallback became an inline `<svg>`. That regressed the no-profile-picture / image-error avatar across every ConnectionIcon consumer — including the legacy block editor and admin surfaces that render with the modernization flag off. Add a dedicated `.avatar` class to the inline SVG and extend the avatar rule to `img, .avatar` so the framing is restored. The `.social-icon` badge (also an `<svg>` in `.wrapper`) is left untouched, so broadening to a bare `svg` selector — which would paint a stray border on the badge — is avoided. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8a66487 to
4e7d6d8
Compare
| const hasConnections = useSelect( | ||
| select => ( select( socialStore ).getConnections() ?? [] ).length > 0, | ||
| [] | ||
| ); |
There was a problem hiding this comment.
This could simply be this
| const hasConnections = useSelect( | |
| select => ( select( socialStore ).getConnections() ?? [] ).length > 0, | |
| [] | |
| ); | |
| const hasConnections = useSelect( select => select( socialStore ).hasConnections(), [] ); |
Summary
Phase 2 of the Social modernization umbrella (#48824). Stacks on top of PR 1 (#48826) and fills the Overview tab with the connected-accounts surface.
openConnectionsModalinto theSocialPageactionsslot when the Overview tab is selected; the existingManageConnectionsModalhandles the rest.ConnectionManagementlist in a WPDSCard.Root(legacy admin page is unchanged; the wrap adds opt-inhideConnectButton+hideHeadingprops so the legacy bottom CTA +<h3>stay where they are when re-used elsewhere).@wordpress/uiEmptyStatewith the share icon + a primary "Add account" CTA that mirrors the page-header button.ConnectionScreen(site not connected) andPricingPage(free Jetpack, pricing nudge not dismissed) keep pre-empting the tabs the way they pre-empt the legacy dashboard — but the check moved to PHP: a newshould_preempt_to_legacy()onSocial_Admin_Pageroutes the menu callback back to the legacySocialAdminPageshell when either condition holds, so those flows render exactly as they do today and the wp-build esbuild bundle stays free of the@automattic/jetpack-connectiondisconnect-dialog asset graph.Drive-by chassis infrastructure
The chassis re-uses
ConnectionManagementand the rest of the existingservices/chain unchanged. wp-build's esbuild pipeline doesn't configure file loaders for.webp/.svg/.jpg, so a handful of asset imports along that chain would otherwise break the build. Three drive-by fixes resolve that without touching the legacy webpack pipeline:default-avatar.svgas JSX insideConnectionIcon. Same fallback, one less binary asset to bundle..webpimports inservices/utils.tsxto runtime URLs built fromJetpackScriptData.social.assets_url. Webpack copies_inc/assets/verbatim intobuild/assets/via a newCopyWebpackPluginstep so both bundlers resolve the same URL../use-connection-error-noticesub-path from@automattic/jetpack-connectionand import via that path from the chassis, so esbuild stops pulling thedisconnect-dialog/*.jpgillustrations through the package barrel.social-logo-colors.cssfrom thesocial-logospackage via a./colors.csssub-path (and define the missing--color-blueskybrand variable). The chassis pulls it fromSocialPageso the per-service brand colours resolve in the modal — the legacy admin bundle inlines these viapostcss-custom-properties { preserve: false }, which the chassis esbuild pipeline doesn't run.Screenshots
Overview tab — empty state
Overview tab — with one connected account
Add account modal
Settings tab (still empty — fills in PR 3)
Testing instructions:
jetpack build packages/publicize --deps(or rsync the Jetpack plugin to a Jurassic Ninja site).wp jetpack module activate publicize.wp option update jetpack-social_show_pricing_page 0). Without this step,should_preempt_to_legacy()correctly routes the request back to the legacySocialAdminPageso the PricingPage stays visible.add_filter( 'rsm_jetpack_ui_modernization_social', '__return_true' );),wp-admin → Jetpack → Socialrenders the legacy admin page unchanged.ManageConnectionsModalopens with the services list. Service icons render in brand colours (Facebook blue, Bluesky blue, Threads black, etc.).ConnectionManagementlist. The card header now reads "Connected accounts". The page-header "Add account" button continues to open the modal.SocialAdminPagewhich rendersConnectionScreen.jetpack-social_show_pricing_pageback to1. Expected: chassis routes back to the legacySocialAdminPagewhich rendersPricingPage.Does this pull request change what data or activity we track or use?
No. PR 2 wires existing components into a new tab body — it does not add Tracks events, new REST endpoints, new option reads/writes, or new external requests beyond what the legacy admin page already performs (
/wpcom/v2/publicize/services,/wpcom/v2/publicize/connections). Thejetpack_social_tab_viewTracks event mentioned in the umbrella lands in PR 5, not here.🤖 Generated with Claude Code