-
Notifications
You must be signed in to change notification settings - Fork 842
Newsletter: Implement comprehensive settings UI with DataForm #46136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
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. |
Code Coverage SummaryCoverage changed in 2 files.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
This is the initial, very rough work-in-progress of bringing the newsletter settings UI into wp-admin using DataForms. It's mostly untested, needs styling etc.
e4259ed to
90c16c2
Compare
Register the settings screen, even if not adding it to the menu due to it being disabled. This means that after disabling newsletter (subscriptions) module, users can still reload the page and re-enable it. Note that WordPress.com simple sites cannot disable the module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a comprehensive newsletter settings UI using WordPress DataForm components, unifying Jetpack and Calypso implementations into a single, modern interface. The implementation follows a card-based design pattern with distinct sections for different settings groups.
Key Changes:
- Introduces React-based settings UI with DataForm for declarative form handling
- Implements mixed save patterns: auto-save for some sections, manual save with staged changes for others
- Adds PHP backend support for menu registration with platform-specific logic (wpcom simple/atomic vs Jetpack)
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
projects/plugins/jetpack/load-jetpack.php |
Initializes Newsletter Settings class early for admin access |
projects/packages/newsletter/src/class-settings.php |
Adds menu registration logic, settings data exposure, and module state checking |
projects/packages/newsletter/src/settings/index.tsx |
Main settings app with state management, API integration, and section orchestration |
projects/packages/newsletter/src/settings/types.ts |
TypeScript type definitions for settings data and Jetpack configuration |
projects/packages/newsletter/src/settings/utils.ts |
Utility functions for subscriber management URLs |
projects/packages/newsletter/src/settings/style.scss |
Card-based styling with disabled state handling and component theming |
projects/packages/newsletter/src/settings/sections/*.tsx |
Individual section components for newsletter, subscriptions, email config, categories, and welcome email |
projects/packages/newsletter/src/settings/components/*.tsx |
Shared components for header, byline preview, and toggle controls with editor links |
projects/packages/newsletter/webpack.config.js |
Adds babel plugin for textdomain replacement |
projects/packages/newsletter/package.json |
Adds dependencies for components, API, and WordPress packages |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes DOTCOM-15286
Proposed changes:
Unify Jetpack React and Calypso implementations of Newsletter Settings into a new WordPress DataForm-based UI, and in the darkness bind them.
Creates a new UI that will eventually replace the two existing UIs:
This fixes DOTCOM-15286, there are further tasks to add tracks, make it load on simple sites, remove old implementations etc.
!! This does not work on simple sites yet !! Jetpack loads funny there. It also doesn't break anything and doesn't load anywhere unless you opt-in, so it's fine.
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
No changes to data tracking or activity tracking. Tracking events will be added in a separate PR.
Testing instructions
Test Steps:
add_filter( 'jetpack_wp_admin_newsletter_settings_enabled', '__return_true' );run early, e.g. by adding it to a new mu-plugin or just by pasting it into the top ofjetpack-dev/jetpack.phpusing the plugin file editor on a jurassic ninja site.Platform-Specific Behavior
wpcom Simple Sites:
wpcom Atomic (WoA) Sites:
/wp-admin/admin.php?page=jetpack-newsletter)still works
Jetpack Sites:
Module State Handling
/wp-admin/admin.php?page=jetpack-newsletterThe settings screen should look like this on an atomic site:
