Newsletter: flip the modernization filter default + retire legacy (PR 6 / #48530)#48613
Open
keoshi wants to merge 6 commits intotry/newsletter-modernization-wpds-cardsfrom
Open
Newsletter: flip the modernization filter default + retire legacy (PR 6 / #48530)#48613keoshi wants to merge 6 commits intotry/newsletter-modernization-wpds-cardsfrom
keoshi wants to merge 6 commits intotry/newsletter-modernization-wpds-cardsfrom
Conversation
6 tasks
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Feature] Masterbar
Contributor
|
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. |
github-actions
Bot
added
the
[Status] Needs Author Reply
keoshi
added
[Status] Needs Review
Every Jetpack site now lands on the unified Subscribers/Settings chassis we've been building since PR 1. The legacy `AdminPage` chrome stays in the bundle behind the same filter, so hosts that need the old surface back can opt out with `add_filter( 'rsm_jetpack_ui_modernization_newsletter', '__return_false' );` until the legacy code paths are removed in a follow-up.
The unified Newsletter page now owns the Subscribers tab, so neither the wp-admin Subscribers submenu nor the "Subscribers ↗" Calypso link belongs in the sidebar anymore. * `modules/subscriptions.php`: drop the `add_subscribers_menu` method (and its `jetpack_admin_menu` hook) that registered the Calypso shortcut on self-hosted, plus the `Subscribers_Dashboard` init that registered the standalone wp-admin Subscribers page on hosts with `jetpack_wp_admin_subscriber_management_enabled` on. Drop the now-unused `Admin_Menu` and `Redirect` imports. * `jetpack-mu-wpcom/wpcom-admin-menu.php`: collapse the `// Jetpack > Subscribers.` if/else into a single `wpcom_hide_submenu_page` call — the Calypso link no longer surfaces here either way, but we keep the hide so anything else that adds it back stays out of the sidebar. Drop `Subscribers_Dashboard` import and remove `'subscribers'` from the Jetpack submenu order list. Newsletter is now the single Subscribers entry point. The `subscribers-dashboard` package will be deleted in the next commit.
Now that no plugin instantiates `Subscribers_Dashboard` (the previous commit retired both call sites), the package has no consumers and can go. Delete `projects/packages/subscribers-dashboard/` outright. Pull the `automattic/jetpack-subscribers-dashboard` dep out of `plugins/jetpack/composer.json`, `packages/jetpack-mu-wpcom/composer.json`, and `packages/masterbar/composer.json` (none of those packages were using it directly), refresh the Jetpack plugin's `composer.lock` and the workspace `pnpm-lock.yaml` so the autoload classmap and pnpm graph stop carrying the package. Standalone `Subscribers` page is fully retired — Newsletter is the single Subscribers entry now.
Record a tracks event whenever the visitor flips to a different tab —
`{ site_type, tab }`. The guard skips the no-op case where
`next === activeTab`, so refreshes and same-tab nav don't pollute the
counts. Analytics is already initialized at the page Stage (PR 2), so
the event reaches Tracks immediately on first interaction.
PR 2 lifted analytics initialization to the page Stage but `load_admin_scripts` was still short-circuiting before any script enqueue ran when modernization was on. `analytics.initialize` queues events into `window._tkq`, but without the Tracks transport (`stats.wp.com/w.js`) loaded, the queue grows forever and no `pixel.wp.com/t.gif` requests fire — telemetry from PR 6's new `jetpack_newsletter_tab_view` event went nowhere. Move the `jp-tracks` enqueue above the modernized short-circuit so it runs on both surfaces; everything else (the legacy `jetpack-newsletter` bundle, JetpackScriptData, etc.) stays gated to the legacy path.
keoshi
force-pushed
the
try/newsletter-modernization-flip-flag
branch
from
7f749e1 to
cc6bb99
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #48530 — final PR of the six-PR Newsletter modernization rollout. Stacked on #48608 (Settings on WPDS Card primitives). Once #48608 merges, GitHub auto-flips this PR's base to trunk.
Proposed changes
true.Newsletter\Settings::is_modernized()now defaults thersm_jetpack_ui_modernization_newsletterfilter totrue. Every Jetpack site lands on the unified Subscribers/Settings chassis without any opt-in. Hosts that need the legacyAdminPagesurface back canadd_filter( 'rsm_jetpack_ui_modernization_newsletter', '__return_false' );. The Subscribers REST gate (class-wpcom-rest-api-v2-endpoint-subscribers-list.php) flips its default the same way so the proxy registers without an explicit filter.modules/subscriptions.phpdrops theadd_subscribers_menumethod (the "Subscribers ↗" Calypso link on self-hosted) and theSubscribers_Dashboard::init()call (the wp-admin Subscribers page registration).jetpack-mu-wpcom'swpcom-admin-menu.phpcollapses the// Jetpack > Subscribers.if/else into awpcom_hide_submenu_pagecall so nothing surfaces the Calypso link, and'subscribers'drops out of the Jetpack submenu order list. Newsletter is the single Subscribers entry now.subscribers-dashboardpackage. Deleteprojects/packages/subscribers-dashboard/outright and pullautomattic/jetpack-subscribers-dashboardfromplugins/jetpack,packages/jetpack-mu-wpcom, andpackages/masterbar. Refreshcomposer.lock(Jetpack plugin) andpnpm-lock.yamlso the autoload classmap and pnpm graph stop carrying the package.jetpack_newsletter_tab_viewTracks event. Fire it fromNewsletterPage'sonTabChangewith{ site_type, tab }whenever the visitor flips to a different tab; same-tab clicks no-op. Analytics init was already lifted to the page Stage in PR 2, but PR 2'sload_admin_scriptsshort-circuit was also skipping thejp-tracksenqueue, so events queued into_tkqforever andpixel.wp.com/t.gifnever fired. Move thejp-tracksenqueue above the modernization short-circuit so the Tracks transport loads on both surfaces.ARIA + RTL audited in the browser:
role="tablist"+role="tab"+role="tabpanel"with the rightaria-selected/aria-labelledby/tabindexplumbing out of the box (Base UI under the hood).dir="rtl"flips the page header (logo + "Newsletter" sit on the right, in the correct semantic order), the Subscribers/Settings tab nav, and the Newsletter card's two-link footer (Privacy / Manage subscribers bracket the row in flipped positions).Related product discussion/links
Does this pull request change what data or activity we track or use?
jetpack_newsletter_tab_viewfires with{ site_type, tab }on tab change. No new identifiers or PII; matches the pattern other Jetpack page-tab events use.add_script_datapayload. No new keys (thesubscriberManagementEnabledkey landed in PR 4).Testing instructions
Pull this branch, run
pnpm install,composer installinsideprojects/plugins/jetpack/, andpnpm jetpack build packages/newsletter --depsto refresh the Newsletter package and its dependencies.Default-on (the new behaviour)
add_filter( 'rsm_jetpack_ui_modernization_newsletter', ... )lives in any mu-plugin /wp-config.php/ theme / Code Snippet.wp-admin/admin.php?page=jetpack-newsletter. The unified chassis renders straight away — page header with the Jetpack mark + "Newsletter", Subscribers/Settings tabs, the WPDS Card layout in Settings, the Subscribers DataViews list under the Subscribers tab.jetpack_wp_admin_subscriber_management_enabledon).pixel.wp.com/t.gif?...&_en=jetpack_newsletter_tab_view&tab=...requests fire when switching tabs (DevTools → Network, filterpixel.wp.com).Opt-out path
add_filter( 'rsm_jetpack_ui_modernization_newsletter', '__return_false' );to a mu-plugin or Code Snippet.AdminPagechrome with the green logo header). The settings still save, the cards still render with WPDS primitives (PR 5 already updated the shared body).RTL smoke check
dir="rtl"on<html>/<body>in DevTools.Keyboard / ARIA smoke check
Screenshots
To be added once the PR review begins (the visual surface is the same as #48608 with the modernization filter implicitly on).