Skip to content

Newsletters: modernize Page / footer / header styles #47399

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.

Sign up for GitHub

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

Open
simison wants to merge 5 commits into
base: trunk
Choose a base branch
from
+131 −102
Open

Newsletters: modernize Page / footer / header styles #47399

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
28df75a
Modernize newsletter settings UI with WordPress Admin UI components
simison Mar 2, 2026
cf89c54
Lockfile maintenance
simison Mar 2, 2026
3e07f5e
Remove header sticky
simison Mar 2, 2026
6328cef
Add back section styles
simison Mar 2, 2026
8af1559
Constraint container to settings width
simison Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions projects/packages/newsletter/changelog/update-newsletter-page
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: changed

Modernize newsletter settings UI with WordPress Admin UI components.
4 changes: 4 additions & 0 deletions projects/packages/newsletter/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,19 @@
"dependencies": {
"@automattic/jetpack-analytics": "workspace:*",
"@automattic/jetpack-api": "workspace:*",
"@automattic/jetpack-base-styles": "workspace:*",
"@automattic/jetpack-components": "workspace:*",
"@automattic/jetpack-script-data": "workspace:*",
"@automattic/jetpack-shared-extension-utils": "workspace:*",
"@wordpress/admin-ui": "1.8.0",
"@wordpress/api-fetch": "7.40.0",
"@wordpress/components": "32.2.0",
"@wordpress/dataviews": "11.3.0",
"@wordpress/element": "6.40.0",
"@wordpress/i18n": "6.13.0",
"@wordpress/notices": "5.40.0",
"@wordpress/theme": "0.7.0",
"@wordpress/ui": "0.7.0",
"@wordpress/url": "4.40.0",
"debug": "4.4.3"
},
Expand Down
10 changes: 9 additions & 1 deletion projects/packages/newsletter/src/class-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,15 @@ private function get_subscriber_management_url( $wp_admin_enabled, $is_wpcom_sim
*/
public function render() {
?>
<div id="newsletter-settings-root"></div>
<style>
/* Admin menu indicators */
ul#adminmenu a.wp-has-current-submenu::after,
ul#adminmenu > li.current > a.current::after {
border-right-color: #fff;
}
body { background: #fff; }
Comment on lines +296 to +301
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline <style> block includes global selectors (body, ul#adminmenu ...) which makes the page styling harder to maintain and can have unintended side effects if this markup is ever reused/embedded. Prefer moving these rules into the existing bundled stylesheet and scoping them to the newsletter admin page (e.g., body.jetpack_page_jetpack-newsletter ... or #jetpack-newsletter-wp-admin-app ...).

Suggested change
/* Admin menu indicators */
ul#adminmenu a.wp-has-current-submenu::after,
ul#adminmenu > li.current > a.current::after {
border-right-color: #fff;
}
body { background: #fff; }
/* Admin menu indicators scoped to the newsletter admin page */
body.jetpack_page_jetpack-newsletter ul#adminmenu a.wp-has-current-submenu::after,
body.jetpack_page_jetpack-newsletter ul#adminmenu > li.current > a.current::after {
border-right-color: #fff;
}
body.jetpack_page_jetpack-newsletter { background: #fff; }

Copilot uses AI. Check for mistakes.
</style>
Comment on lines +295 to +302
Copy link
Member Author

@simison simison Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WP Boot package applies these (and more) styles in similar manner, so this is future-compatible.

It basically applies white background so that loading state and notice-banners on top look better (otherwise they have grey background).

It also changes menu indicator triangle to be white insteead of grey to match page background.

Comment on lines +295 to +302
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This inline <style> block makes global admin CSS changes (body background and #adminmenu indicators) directly in the render output. Since the React app already bundles/enqueues style.scss, consider moving these rules into that stylesheet (scoped to the newsletter admin page body class, e.g. body.jetpack_page_jetpack-newsletter) to keep styling versioned/cached and avoid accumulating inline styles in PHP output.

Suggested change
<style>
/* Admin menu indicators */
ul#adminmenu a.wp-has-current-submenu::after,
ul#adminmenu > li.current > a.current::after {
border-right-color: #fff;
}
body { background: #fff; }
</style>

Copilot uses AI. Check for mistakes.
<div id="jetpack-newsletter-wp-admin-app"></div>
<?php
}

Expand Down
16 changes: 0 additions & 16 deletions projects/packages/newsletter/src/settings/components/header.scss
View file
Open in desktop

This file was deleted.

34 changes: 0 additions & 34 deletions projects/packages/newsletter/src/settings/components/header.tsx
View file
Open in desktop

This file was deleted.

102 changes: 58 additions & 44 deletions projects/packages/newsletter/src/settings/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,21 @@
*/
import analytics from '@automattic/jetpack-analytics';
import {
AdminPage,
Col,
Container,
JetpackLogo,
GlobalNotices,
useGlobalNotices,
JetpackFooter,
} from '@automattic/jetpack-components';
import { getSiteType, isSimpleSite } from '@automattic/jetpack-script-data';
import { Notice } from '@wordpress/components';
import { Page } from '@wordpress/admin-ui';
import { Disabled, Spinner, Notice } from '@wordpress/components';
import { createRoot, useCallback, useEffect, useState, useMemo } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { Stack } from '@wordpress/ui';
/**
* Internal dependencies
*/
import { fetchSettings, updateSettings } from './api';
import { Header } from './components/header';
import { getNewsletterScriptData } from './script-data';
import {
EmailContentSection,
Expand All @@ -33,8 +33,6 @@ import {
import type { NewsletterSettings } from './types';
import './style.scss';

const MODULE_NAME = __( 'Jetpack Newsletter', 'jetpack-newsletter' );

/**
* Normalize settings from API response
*
Expand All @@ -54,6 +52,37 @@ function normalizeSettings( settings: Record< string, unknown > ): NewsletterSet
};
}

/**
* Newsletter page container
*
* @param {React.ReactNode} children - The children to render inside the page
* @return {JSX.Element} The newsletter page container
*/
function NewsletterPage( { children }: { children: React.ReactNode } ): JSX.Element {
return (
<div className="jetpack-page-container">
<Page
title={
<Stack gap="xs" align="center" justify="start" direction="row">
<JetpackLogo showText={ false } width={ 20 } />
{ /** "Newsletter" is a product name, do not translate. */ }
<span>Newsletter</span>
</Stack>
}
subTitle={ __(
'Transform your blog posts into newsletters to easily reach your subscribers.',
'jetpack-newsletter'
) }
>
<Stack gap="md" direction="column" className="newsletter-settings">
{ children }
</Stack>
<JetpackFooter />
</Page>
</div>
);
}

/**
* Newsletter Settings App
*
Expand Down Expand Up @@ -333,50 +362,36 @@ function NewsletterSettingsApp(): JSX.Element | null {

if ( isLoading ) {
return (
<AdminPage moduleName={ MODULE_NAME } header={ <Header /> }>
<Container horizontalSpacing={ 3 }>
<Col>
<div className="newsletter-settings">
<p>{ __( 'Loading newsletter settings…', 'jetpack-newsletter' ) }</p>
</div>
</Col>
</Container>
</AdminPage>
<NewsletterPage>
<Stack justify="center" align="center" style={ { minHeight: '100%' } }>
<Spinner />
</Stack>
</NewsletterPage>
);
}

if ( error ) {
if ( error || ! data ) {
return (
<AdminPage moduleName={ MODULE_NAME } header={ <Header /> }>
<Container horizontalSpacing={ 3 }>
<Col>
<div className="newsletter-settings newsletter-settings--error">
<Notice status="error" isDismissible={ false }>
{ error }
</Notice>
</div>
</Col>
</Container>
</AdminPage>
<NewsletterPage>
<Notice status="error" isDismissible={ false }>
{ error || __( 'Failed to load settings.', 'jetpack-newsletter' ) }
</Notice>
</NewsletterPage>
);
}

if ( ! data ) {
return null;
}

const hasSubscriptionChanges = Object.keys( subscriptionChanges ).length > 0;
const hasSenderNameChanges = Object.keys( senderNameChanges ).length > 0;
const hasNewsletterCategoriesChanges = Object.keys( newsletterCategoriesChanges ).length > 0;
const hasWelcomeEmailChanges = Object.keys( welcomeEmailChanges ).length > 0;

return (
<AdminPage moduleName={ MODULE_NAME } header={ <Header /> }>
<Container horizontalSpacing={ 3 }>
<Col>
<div className="newsletter-settings">
{ ! isSimpleSite() && <NewsletterSection data={ data } onChange={ handleAutoSave } /> }
<NewsletterPage>
<Stack gap="md" direction="column" className="newsletter-settings">
{ ! isSimpleSite() && <NewsletterSection data={ data } onChange={ handleAutoSave } /> }

Comment on lines +389 to 392
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewsletterPage already wraps its children in a <Stack ... className="jetpack-newsletter-settings">, but NewsletterSettingsApp adds another nested Stack with the same class. This likely doubles up layout spacing/max-width/margins and makes future layout changes harder. Consider keeping the layout wrapper in one place (either remove the inner Stack here, or remove it from NewsletterPage).

Copilot uses AI. Check for mistakes.
<Disabled isDisabled={ ! data.subscriptions }>
<Stack gap="md" direction="column">
<SubscriptionsSection
data={ data }
onChange={ handleSubscriptionChange }
Expand Down Expand Up @@ -435,16 +450,15 @@ function NewsletterSettingsApp(): JSX.Element | null {
hasChanges={ hasWelcomeEmailChanges }
isNewsletterEnabled={ data.subscriptions }
/>

<GlobalNotices />
</div>
</Col>
</Container>
</AdminPage>
</Stack>
</Disabled>
</Stack>
<GlobalNotices />
</NewsletterPage>
);
}

const container = document.getElementById( 'newsletter-settings-root' );
const container = document.getElementById( 'jetpack-newsletter-wp-admin-app' );
if ( container ) {
const root = createRoot( container );
root.render( <NewsletterSettingsApp /> );
Expand Down
48 changes: 41 additions & 7 deletions projects/packages/newsletter/src/settings/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,43 @@
// Import dataviews styles for DataForm component
@use "@automattic/jetpack-base-styles/gutenberg-base-styles" as gb;
@import "@wordpress/theme/design-tokens.css";
@import "@wordpress/admin-ui/build-style/style.css";
@import "@wordpress/dataviews/build-style/style.css";
Comment on lines +1 to 4
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wordpress/admin-ui/build-style/style.css is imported here, but newsletter’s webpack config does not define a resolve.alias for that path. In this repo, the Forms dashboard build adds an explicit alias for this exact file (e.g., projects/packages/forms/tools/webpack.config.dashboard.js) to avoid resolution failures due to package export maps. Consider adding the same alias in projects/packages/newsletter/webpack.config.js (or importing via an exported entrypoint) so this stylesheet can be resolved reliably during bundling.

Copilot uses AI. Check for mistakes.

$content-max-width: 1040px;

.jetpack-page-container {
margin-left: -20px;
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.jetpack-page-container uses a physical margin-left: -20px;, which won’t behave correctly in RTL layouts. The surrounding styles already use logical properties (e.g., inset-inline-start/end), so consider switching this to a logical margin (e.g., margin-inline-start) to keep the layout RTL-safe.

Suggested change
margin-left: -20px;
margin-inline-start: -20px;

Copilot uses AI. Check for mistakes.
min-height: 100vh;
clear: both;

.jp-dashboard-footer {
border-top: 1px solid gb.$gray-100;
margin: 16px 0 0 0;
max-width: none;
padding: 16px;
width: auto;
}

// @TODO: Temporarily disable sticky header. Remove once we've reviewed
// all pages are compatible with Sticky header.
.admin-ui-page__header {
position: relative;
}
}

// Hello Dolly compatibility fix
.jetpack_page_jetpack-newsletter #dolly {
background: gb.$white;
Copy link
Member Author

@simison simison Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matches header https://github.com/WordPress/gutenberg/blob/aa714d8db9cbabaf604a4324c5d6bd53246bf9e2/packages/admin-ui/src/page/style.scss#L20C14-L20C20

clear: both;
display: block;
text-align: right;
width: 100%;
}

// Sections
.newsletter-settings {
max-width: 1200px;
margin-inline: auto;
max-width: $content-max-width;
margin: 20px auto;

// Card-based sections
&__section {
Expand Down Expand Up @@ -77,9 +111,9 @@
color: #646970;
}
}
}

// Notice component styling
.components-notice {
margin: 0 0 20px;
}
// Notice component styling
.components-notice {
margin: 0 0 20px;
}
1 change: 1 addition & 0 deletions projects/packages/newsletter/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export default {
requestMap: {
// Bundle the package with our assets until WP core exposes wp-admin-ui.
'@wordpress/admin-ui': { external: false },
'@wordpress/admin-ui/build-style/style.css': { external: false },
},
Comment on lines 121 to 125
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wordpress/admin-ui/build-style/style.css is imported (via src/settings/style.scss), but this webpack config does not add a resolve.alias for that subpath. In this repo, other bundles that import that CSS (e.g. Forms dashboard) alias it to the physical file under node_modules/@wordpress/admin-ui/build-style/style.css to avoid resolution failures caused by package exports. Please add the same alias here (or change the import to a supported entrypoint) so the build doesn’t break.

Copilot uses AI. Check for mistakes.
},
} ),
Expand Down
Loading