Skip to content

Commit 6d0566a

Browse files
feat(admin): companion to newspack-newsletters admin UX milestone (#4735)
* feat(newsletters): wire wizard SubscriptionLists to wizard-bridge events * test(newsletters): cover wizard SubscriptionLists bridge wiring * fix(newsletters-wizard): read bridge readiness synchronously The `bridgeMounted` flag was only set via the one-shot `newspack-newsletters:bridge-mounted` event listener. If the bridge bundle booted before this module evaluated, the event was missed and the 500ms fallback timer redirected to the legacy editor after every modal-open attempt. Replace the local mutable boolean with a sync read of `window.newspackNewslettersBridgeReady`, which the bridge now sets before dispatching the event. Refs NEWS-2152 * feat(newsletters): bundled-mode parity for ESP-list edit modal (NEWS-2168) Stacks the bundled-mode side onto the same wiring as NEWS-2152's local-list parity. The wizard's SubscriptionLists view dispatches OPEN_MODAL with kind='esp' for remote rows (replacing the legacy ExternalLink to the CPT editor) and the active toggle commits immediately via PATCH /lists/{db_id}. - Edit on remote rows opens the same modal in ESP mode through the wizard-bridge (legacy edit_link still serves as the fallback when the bridge isn't ready). - Inline TextControl/TextareaControl pair removed for remote rows — the modal owns title + description. - Active toggle on every row PATCHes that one row; bulk "Save Subscription Lists" button removed. - Description string surfaced under the bold ActionCard title (with the type label kept on a smaller line below) so publishers see what customisation is in place without opening the modal. * chore: ignore /build output directory * feat(components): shared scaffolding for newsletters settings (NEWS-2263) - integration-icons namespace with 6 brand SVGs (Active Campaign, Constant Contact, Fundraise Up, Mailchimp, Salesforce, Wisepops); fills customisable via var(--integration-icon-color) / --integration-icon-color-accent with brand-colour defaults. - useUnsavedChangesDialog hook: wraps useConfirmDialog with the standard "discard changes" messaging, intercepts outbound link clicks so the dialog fires instead of the silent native one, plus a beforeunload safety net. - CardSettingsGroup gains optional className and iconSize props. - CoreCard accepts iconSize; SCSS reads it via the new --newspack-card-icon-size custom property with fallback to existing defaults so no consumer changes are required. - SectionHeader: container margin now zeroes alongside its inner element when noMargin is set (via :has() so callsites need no change). - admin.css: chrome section header only gets margin-top when it's not the first child of newspack-wizard__content, fixing the doubled spacing on audience /edit/new/all. - Audience content-gates edit adopts useUnsavedChangesDialog so the same standardised confirm flow covers both wizards. * feat(newsletters): rebuild Settings page (NEWS-2263) Single-section wizard with sticky header, dirty-state tracking, and unsaved-changes guard via useUnsavedChangesDialog. Header chrome shows "Newsletters / Settings" with the Save button disabled until something is actually dirty. Layout (top to bottom, two-column rows with section header on the left): - Email service provider — 2x2 grid of brand-icon cards (Active Campaign, Mailchimp, Constant Contact, Manual / Other). Selecting a card sets the provider and reveals its credentials below the grid. Constant Contact shows its OAuth Authorize prompt inline when needed. - Newsletter posts — slug, comments, related-posts as ToggleControls. - Subscription lists — per-row ToggleControl with Local/ESP badge, list description as help text, dividers between rows, Edit/Delete on the right (Delete only for local lists). Below the list, a paragraph and secondary "Add new local list" button explain provider-specific syncing (label resolved per-ESP via the wizard's settings response). - Ads tracking — merged in from the previous separate tab; ToggleControls for click + impressions. The Tracking tab on the ads-list admin header is removed since the section now lives here. - Letterhead — separate section at the bottom for the Letterhead API key + help link. PHP: wizard /settings response now includes the active provider's labels (get_labels('local_list_explanation')), guarded with class_exists for standalone-newspack-plugin installs. Test selectors updated for the new card-based Add new flow. * fix(newsletters): address PR review feedback (NEWS-2263) - useUnsavedChangesDialog: track confirmed navigations via a ref so the beforeunload guard skips the native prompt after the user confirms via our custom dialog. - SubscriptionLists: per-row toggles use functional setState and a Set of in-flight db_ids so two quick toggles can't clobber each other if responses resolve out of order; rollback reverts the specific row to its previous active value rather than restoring a whole-list snapshot. - SubscriptionLists: render the locked-state warning even when no list data has loaded yet, so a provider change that sets lockedLists before a successful fetch still explains itself. - Settings (onboarding path): restore the provider filter so unselected ESPs' credential fields don't appear in the setup Services card. * fix(newsletters): expose labels response with explicit key (NEWS-2263) Replace `Provider::get_labels( 'local_list_explanation' )` (where the arg is a context hint, not a key selector — easy to misread) with a direct `Provider::label( 'local_list_explanation' )` call assigned to an explicit `local_list_explanation` key. Same runtime payload, but the intent is now obvious at the call site and the response surfaces only what the client actually consumes. * fix(newsletters): a11y labels on per-row Edit/Delete (NEWS-2263) * fix(newsletters): address second-pass review feedback (NEWS-2263) * fix(newsletters): disable settings controls during save (NEWS-2263) * fix(newsletters): hide Subscription Lists for manual provider (NEWS-2263) * fix(newsletters): a11y + bridge-event robustness (NEWS-2263) * fix(newsletters): queue bridge dispatch when not yet ready (NEWS-2263) * fix(newsletters): resolve bridge event name at replay time (NEWS-2263) * fix(newsletters): flush queue if bridge is ready at fallback (NEWS-2263) * fix(newsletters): clear stale queue on immediate dispatch (NEWS-2263) * fix(newsletters): friendlier error for missing PATCH endpoint (NEWS-2263) * fix(components,newsletters): hash-router guard + reload listener reattach (NEWS-2263) * fix(newsletters): defer hash-router guard to ConfirmDialog; wrap tests in Router (NEWS-2263) * fix(newsletters): re-render lists on ESP switch-back and notice on delete fallback * fix(components): structural single-consumer guard for unsaved-changes hook * fix(newsletters): drop stale tracking-admin remove_action * fix(newsletters): sync subscription lists with ESP connection status * refactor(newsletters): rename signature helper to match its scope
1 parent 58dcb83 commit 6d0566a

24 files changed

Lines changed: 1683 additions & 326 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ Thumbs.db
2929
/vendor/
3030

3131
# Built files
32+
/build
3233
/dist
3334
/packages/components/colors
3435
/packages/components/dist

includes/wizards/newsletters/class-newsletters-wizard.php

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ public function __construct() {
8080

8181
// Menu removals.
8282
remove_action( 'admin_menu', [ Newspack_Newsletters_Settings::class, 'add_plugin_page' ] );
83-
remove_action( 'admin_menu', [ Newspack_Newsletters_Tracking_Admin::class, 'add_settings_page' ] );
8483

8584
// Customize the Newsletter ads menu titles.
8685
add_filter(
@@ -227,9 +226,25 @@ function ( $acc, $value ) {
227226
},
228227
[]
229228
);
229+
230+
$labels = [];
231+
if ( class_exists( 'Newspack_Newsletters' ) ) {
232+
$provider = Newspack_Newsletters::get_service_provider();
233+
if ( $provider && method_exists( $provider, 'label' ) ) {
234+
// `label()` accepts a second `$context` arg (usually a list public id).
235+
// Intentionally omitted here: this is the generic explanation copy
236+
// shown above the "Add new local list" button, not a per-list label.
237+
$labels = [
238+
'local_list_explanation' => $provider::label( 'local_list_explanation' ),
239+
];
240+
}
241+
}
242+
230243
return [
231-
'configured' => $newsletters_configuration_manager->is_configured(),
232-
'settings' => $settings,
244+
'configured' => $newsletters_configuration_manager->is_configured(),
245+
'esp_connected' => (bool) $newsletters_configuration_manager->is_esp_set_up(),
246+
'settings' => $settings,
247+
'labels' => $labels,
233248
];
234249
}
235250

@@ -253,6 +268,12 @@ public function api_update_newsletters_settings( $request ) {
253268
$args = $request->get_params();
254269
$newsletters_configuration_manager = Configuration_Managers::configuration_manager_class_for_plugin_slug( 'newspack-newsletters' );
255270
$newsletters_configuration_manager->update_settings( $args );
271+
// The provider instance is memoized on `init`, before this save runs, so
272+
// refresh it before reading credential status — otherwise a provider switch
273+
// reports the previously-active provider's connection state.
274+
if ( method_exists( 'Newspack_Newsletters', 'memoize_service_provider' ) ) {
275+
\Newspack_Newsletters::memoize_service_provider();
276+
}
256277
return $this->api_get_newsletters_settings();
257278
}
258279

@@ -424,11 +445,6 @@ private function get_tabs() {
424445
];
425446
}
426447

427-
$tabs[] = [
428-
'textContent' => esc_html__( 'Tracking', 'newspack-plugin' ),
429-
'href' => admin_url( 'edit.php?post_type=' . Newspack_Newsletters::NEWSPACK_NEWSLETTERS_CPT . '&page=' . $this->slug . '#/tracking' ),
430-
];
431-
432448
return $tabs;
433449
}
434450

packages/components/src/card-settings-group/index.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import './style.scss';
1111
const CardSettingsGroup = ( {
1212
actionType = 'none',
1313
children,
14+
className,
15+
disabled = false,
1416
icon = null,
1517
headerAction,
1618
title = '',
@@ -21,6 +23,8 @@ const CardSettingsGroup = ( {
2123
}: {
2224
actionType?: 'chevron' | 'toggle' | 'button' | 'link' | 'none';
2325
children?: React.ReactNode;
26+
className?: string;
27+
disabled?: boolean;
2428
icon?: React.ReactNode;
2529
title: string;
2630
headerAction?: {
@@ -40,7 +44,7 @@ const CardSettingsGroup = ( {
4044
} ) => {
4145
return (
4246
<Card
43-
className="newspack-card--core--settings-group"
47+
className={ [ 'newspack-card--core--settings-group', className ].filter( Boolean ).join( ' ' ) }
4448
actionType={ actionType }
4549
isSmall
4650
__experimentalCoreCard
@@ -54,6 +58,7 @@ const CardSettingsGroup = ( {
5458
headerAction,
5559
onHeaderClick,
5660
onToggle: onEnable,
61+
disabled,
5762
icon,
5863
iconBackgroundColor: true,
5964
isActive,

packages/components/src/card/core-card.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const CoreCard = ( {
3232
headerStyle,
3333
childrenStyle,
3434
footerStyle,
35+
disabled,
3536
icon,
3637
iconBackgroundColor,
3738
isActive,
@@ -62,6 +63,7 @@ const CoreCard = ( {
6263
icon && 'newspack-card--core__has-icon',
6364
iconBackgroundColor && 'newspack-card--core__has-icon-background-color',
6465
isActive && 'newspack-card--core__is-active',
66+
disabled && 'newspack-card--core__is-disabled',
6567
children && 'newspack-card--core__has-children',
6668
noMargin && 'newspack-card--core__no-margin',
6769
hasGreyHeader && 'newspack-card--core__has-grey-header'
@@ -91,7 +93,9 @@ const CoreCard = ( {
9193
style={ headerStyle }
9294
size={ sizeProps }
9395
gap={ 4 }
94-
onClick={ onHeaderClick }
96+
onClick={ disabled ? undefined : onHeaderClick }
97+
disabled={ onHeaderClick && disabled ? true : undefined }
98+
aria-disabled={ onHeaderClick && disabled ? true : undefined }
9599
>
96100
{ isDraggable && (
97101
<div className="newspack-card--core__header__draggable-controls">

packages/components/src/card/style-core.scss

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@
5757
&__no-margin .components-card__body {
5858
padding: 0 !important;
5959
}
60+
&__is-disabled {
61+
cursor: not-allowed;
62+
opacity: 0.5;
63+
.newspack-card--core__header {
64+
pointer-events: none;
65+
}
66+
}
6067
&__header {
6168
color: wp-colors.$gray-700;
6269
flex-wrap: wrap;
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/**
2+
* WordPress dependencies.
3+
*/
4+
import { useEffect, useRef } from '@wordpress/element';
5+
import { __ } from '@wordpress/i18n';
6+
7+
/**
8+
* Internal dependencies.
9+
*/
10+
import useConfirmDialog from './use-confirm-dialog';
11+
12+
type UseUnsavedChangesDialogOptions = {
13+
when: boolean;
14+
};
15+
16+
// Stack of mounted guards; the last entry owns the document-level handler so
17+
// only the most-recently-mounted guard prompts when several are active. Using a
18+
// stack keeps ownership correct under out-of-order unmounts.
19+
const activeGuards: symbol[] = [];
20+
21+
/**
22+
* Returns true when `href` resolves to the same origin as the current page —
23+
* i.e. it is an internal navigation that would unload the wizard. External
24+
* `https://...` links, `mailto:`, `tel:`, and other schemes navigate to a new
25+
* context (new tab, mail client, dialer) without unloading the wizard, and
26+
* must not trigger the discard-changes prompt.
27+
*/
28+
function isSameOriginNavigation( link: HTMLAnchorElement ): boolean {
29+
try {
30+
const url = new URL( link.href, window.location.href );
31+
return url.origin === window.location.origin && ( url.protocol === 'http:' || url.protocol === 'https:' );
32+
} catch ( e ) {
33+
return false;
34+
}
35+
}
36+
37+
/**
38+
* Shared unsaved-changes guard. Wraps `useConfirmDialog` with standardized
39+
* messaging, intercepts same-origin link clicks so the dialog fires instead
40+
* of a silent navigation, and adds a `beforeunload` listener as the last-resort
41+
* guard for refresh / tab-close (browser-native, cannot be styled). The
42+
* returned `confirmDialog` element must be rendered in JSX.
43+
*
44+
* Single-consumer constraint: the click handler is attached at the document
45+
* level in capture phase. Two simultaneously-active instances will both fire
46+
* a dialog. A development-only warning surfaces this.
47+
*/
48+
function useUnsavedChangesDialog( { when }: UseUnsavedChangesDialogOptions ) {
49+
const { confirmDialog, requestConfirm } = useConfirmDialog( {
50+
when,
51+
message: __( 'You have unsaved changes that will be lost. Discard changes?', 'newspack-plugin' ),
52+
confirmButtonText: __( 'Discard changes', 'newspack-plugin' ),
53+
hideTitle: true,
54+
} );
55+
56+
// Tracks navigation the user has already approved via our custom dialog so
57+
// the beforeunload guard doesn't fire a second native prompt on top of it.
58+
const isNavigatingRef = useRef( false );
59+
60+
useEffect( () => {
61+
if ( ! when ) {
62+
return;
63+
}
64+
const ownerId = Symbol( 'unsaved-changes-guard' );
65+
activeGuards.push( ownerId );
66+
if ( process.env.NODE_ENV !== 'production' && activeGuards.length > 1 ) {
67+
// eslint-disable-next-line no-console
68+
console.warn(
69+
'useUnsavedChangesDialog: more than one active instance detected. ' +
70+
'Document-level click capture will fire a dialog per instance — ensure only one guard is active at a time.'
71+
);
72+
}
73+
const handler = ( e: MouseEvent ) => {
74+
// Only the top-of-stack guard prompts, so concurrent guards can't stack dialogs.
75+
if ( activeGuards[ activeGuards.length - 1 ] !== ownerId ) {
76+
return;
77+
}
78+
if ( e.defaultPrevented || e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button !== 0 ) {
79+
return;
80+
}
81+
const target = e.target as HTMLElement | null;
82+
const link = target?.closest( 'a[href]' ) as HTMLAnchorElement | null;
83+
if ( ! link ) {
84+
return;
85+
}
86+
const href = link.getAttribute( 'href' );
87+
if ( ! href || href.startsWith( '#' ) || href.startsWith( 'javascript:' ) ) {
88+
// All `#`-prefixed links are skipped here: plain anchors
89+
// (`#section`) are scroll targets, and HashRouter paths
90+
// (`#/route`) are intercepted by `ConfirmDialog`'s built-in
91+
// `history.block` listener instead. Routing them through
92+
// `window.location.href` here would trigger a hashchange
93+
// that the still-active block re-catches, double-prompting.
94+
return;
95+
}
96+
if ( link.target && link.target !== '_self' ) {
97+
return;
98+
}
99+
// Skip mailto:, tel:, external origins, and any non-http(s) scheme —
100+
// they don't unload the wizard, so a discard prompt would be wrong.
101+
if ( ! isSameOriginNavigation( link ) ) {
102+
return;
103+
}
104+
e.preventDefault();
105+
e.stopPropagation();
106+
const destination = link.href;
107+
requestConfirm( () => {
108+
isNavigatingRef.current = true;
109+
window.location.href = destination;
110+
} );
111+
};
112+
document.addEventListener( 'click', handler, true );
113+
return () => {
114+
document.removeEventListener( 'click', handler, true );
115+
const idx = activeGuards.indexOf( ownerId );
116+
if ( idx !== -1 ) {
117+
activeGuards.splice( idx, 1 );
118+
}
119+
};
120+
}, [ when, requestConfirm ] );
121+
122+
useEffect( () => {
123+
if ( ! when ) {
124+
return;
125+
}
126+
const handler = ( e: BeforeUnloadEvent ) => {
127+
if ( isNavigatingRef.current ) {
128+
return;
129+
}
130+
e.preventDefault();
131+
e.returnValue = '';
132+
};
133+
window.addEventListener( 'beforeunload', handler );
134+
return () => window.removeEventListener( 'beforeunload', handler );
135+
}, [ when ] );
136+
137+
return { confirmDialog, requestConfirm };
138+
}
139+
140+
export default useUnsavedChangesDialog;

packages/components/src/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ export { default as withWizardScreen } from './with-wizard-screen';
5151
export { default as Router } from './proxied-imports/router';
5252
export { default as hooks } from './hooks';
5353
export { default as useConfirmDialog } from './hooks/use-confirm-dialog';
54+
export { default as useUnsavedChangesDialog } from './hooks/use-unsaved-changes-dialog';
55+
import * as integrationIcons from './integration-icons';
56+
export { integrationIcons };
5457
export { default as utils } from './utils';
5558

5659
import './style.scss';
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
const activeCampaign = (
2+
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none">
3+
<path
4+
d="M16.6934 10.5811C17.1772 10.9036 17.4355 11.4194 17.4355 11.9678C17.4355 12.516 17.1449 13.0319 16.6934 13.3545L6.56445 20V18.6133C6.56445 18.1939 6.75844 17.7746 7.14551 17.5166L15.5 11.9678L7.14551 6.38672C6.79063 6.16086 6.56445 5.74175 6.56445 5.29004V4L16.6934 10.5811Z"
5+
fill="white"
6+
/>
7+
<path
8+
d="M6.56445 8.77441C6.56445 8.35519 7.04853 8.12873 7.37109 8.35449L12.6611 11.9355L11.9512 12.4189C11.4996 12.7091 10.9193 12.7092 10.4678 12.4189L9.30664 11.6777L6.56445 9.83887V8.77441Z"
9+
fill="white"
10+
/>
11+
</svg>
12+
);
13+
14+
export default activeCampaign;
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
const constantContact = (
2+
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none">
3+
<path
4+
d="M11.3261 20.9997C6.36799 20.9997 3 16.9581 3 12.6626C3 8.30073 6.43425 4.5352 11.0059 4.35852C11.1826 4.34748 11.3372 4.50207 11.3372 4.67875V6.01491C11.3372 6.18055 11.2047 6.3241 11.028 6.33514C7.67102 6.48974 4.96558 9.23935 4.96558 12.6736C4.96558 16.0305 7.61581 19.0341 11.3261 19.0341C14.9371 19.0341 17.4989 16.152 17.6535 12.9717C17.6646 12.8061 17.7971 12.6626 17.9738 12.6626H19.3099C19.4866 12.6626 19.6302 12.8061 19.6302 12.9938C19.4756 17.2342 16.0634 20.9997 11.3261 20.9997Z"
5+
fill="#1856ED"
6+
/>
7+
<path
8+
d="M11.3372 16.5827C9.12864 16.5827 7.41704 14.8269 7.41704 12.6625C7.41704 10.6417 8.97405 8.95223 10.9948 8.78659C11.1826 8.77555 11.3372 8.9191 11.3372 9.10683V10.443C11.3372 10.5976 11.2267 10.7411 11.0611 10.7632C10.0894 10.8957 9.38262 11.7239 9.38262 12.6736C9.38262 13.7337 10.2108 14.6281 11.3372 14.6281C12.2758 14.6281 13.115 13.9214 13.2475 12.9497C13.2696 12.7951 13.4132 12.6736 13.5678 12.6736H14.9039C15.0916 12.6736 15.2352 12.8282 15.2242 13.0159C15.0475 14.9925 13.3911 16.5827 11.3372 16.5827Z"
9+
fill="#1856ED"
10+
/>
11+
<path
12+
d="M19.0339 11.0283C18.8793 7.79277 16.2953 5.12046 12.9715 4.96586C12.8058 4.95482 12.6623 4.82231 12.6623 4.64563V3.32051C12.6623 3.14383 12.8058 3.00028 12.9936 3.00028C17.3112 3.16592 20.8338 6.62225 20.9994 11.0062C21.0105 11.1828 20.8559 11.3374 20.6792 11.3374H19.3431C19.1885 11.3374 19.0449 11.2049 19.0339 11.0283Z"
13+
fill="#FF9E1A"
14+
/>
15+
<path
16+
d="M13.0267 9.41602C12.8279 9.38289 12.6844 9.21725 12.6844 9.05161V7.74859C12.6844 7.56086 12.839 7.41731 13.0267 7.42835C14.9481 7.59399 16.4278 9.12891 16.5934 10.9841C16.6045 11.1718 16.4609 11.3264 16.2732 11.3264H14.8708C14.7383 11.3264 14.6389 11.238 14.6168 11.1055C14.5174 10.3215 13.9211 9.57062 13.0267 9.41602Z"
17+
fill="#FF9E1A"
18+
/>
19+
</svg>
20+
);
21+
22+
export default constantContact;
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
const fundraiseUp = (
2+
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none">
3+
<path
4+
d="M4.90143 6.11914C4.90143 4.94655 5.85727 4 7.03802 4H19.0985V5.75182C19.0985 6.92441 18.1427 7.87098 16.9619 7.87098H4.90143V6.11914Z"
5+
fill="white"
6+
/>
7+
<path
8+
d="M4.90143 11.4721C4.90143 10.6979 5.53463 10.0645 6.33667 10.0645H14.8778V11.824C14.8778 12.9924 13.921 13.9355 12.739 13.9355H4.90143V11.4721Z"
9+
fill="white"
10+
/>
11+
<path
12+
d="M4.90143 16.8329C4.90143 16.4387 5.22775 16.1291 5.62502 16.1291L10.657 16.129V17.8886C10.657 19.0569 9.69221 20 8.50045 20H4.90143V16.8329Z"
13+
fill="white"
14+
/>
15+
</svg>
16+
);
17+
18+
export default fundraiseUp;

0 commit comments

Comments
 (0)