feat(admin): admin UX modernisation — settings rebuild + shared scaffolding (NEWS-2152, NEWS-2168, NEWS-2263)

.gitignore

@@ -29,6 +29,7 @@ Thumbs.db
 /vendor/
 
 # Built files
+/build
 /dist
 /packages/components/colors
 /packages/components/dist

src/wizards/newsletters/views/settings/index.js

@@ -11,10 +11,33 @@ import once from 'lodash/once';
 /**
  * WordPress dependencies
  */
-import { useEffect, useState, Fragment } from '@wordpress/element';
+import { useEffect, useRef, useState, Fragment } from '@wordpress/element';
 import apiFetch from '@wordpress/api-fetch';
 import { sprintf, __ } from '@wordpress/i18n';
-import { CheckboxControl, TextareaControl, ExternalLink, Notice } from '@wordpress/components';
+import {
+	CheckboxControl,
+	ExternalLink,
+	Notice,
+	__experimentalHStack as HStack, // eslint-disable-line @wordpress/no-unsafe-wp-apis
+} from '@wordpress/components';
+
+// Wizard-bridge events. Mirror of `newspack-newsletters/src/wizard-bridge/events.js`
+// — kept locally so this file is self-contained without a cross-repo import.
+const NN_EVENT_NAMESPACE = 'newspack-newsletters';
+const NN_EVENTS = {
+	BRIDGE_MOUNTED: `${ NN_EVENT_NAMESPACE }:bridge-mounted`,
+	OPEN_MODAL: `${ NN_EVENT_NAMESPACE }:open-local-list-modal`,
+	OPEN_CONFIRM_DELETE: `${ NN_EVENT_NAMESPACE }:open-local-list-confirm-delete`,
+	LOCAL_LIST_SAVED: `${ NN_EVENT_NAMESPACE }:local-list-saved`,
+	LOCAL_LIST_DELETED: `${ NN_EVENT_NAMESPACE }:local-list-deleted`,
+};
+const NN_FALLBACK_TIMEOUT_MS = 500;
+
+// Read the bridge-readiness flag synchronously rather than relying on a
+// one-shot `BRIDGE_MOUNTED` event. The bridge sets the flag before
+// dispatching, so listeners that register late still observe a ready
+// bridge — avoiding a spurious fallback redirect.
+const isBridgeReady = () => typeof window !== 'undefined' && window.newspackNewslettersBridgeReady === true;
 
 /**
  * Internal dependencies
@@ -251,7 +274,10 @@ export const Settings = ( {
 export const SubscriptionLists = ( { lockedLists, onUpdate, provider } ) => {
 	const [ error, setError ] = useState( false );
 	const [ inFlight, setInFlight ] = useState( false );
+	const [ togglingId, setTogglingId ] = useState( null );
 	const [ lists, setLists ] = useState( [] );
+	const fallbackTimerRef = useRef( null );
+
 	const updateConfig = data => {
 		setLists( data );
 		if ( typeof onUpdate === 'function' ) {
@@ -268,37 +294,75 @@ export const SubscriptionLists = ( { lockedLists, onUpdate, provider } ) => {
 			.catch( setError )
 			.finally( () => setInFlight( false ) );
 	};
-	const saveLists = () => {
+	const handleToggleActive = async ( list, next ) => {
+		if ( ! list?.db_id ) {
+			return;
+		}
+		const snapshot = lists;
+		updateConfig( lists.map( row => ( row.db_id === list.db_id ? { ...row, active: next } : row ) ) );
+		setTogglingId( list.db_id );
 		setError( false );
-		setInFlight( true );
-		apiFetch( {
-			path: '/newspack-newsletters/v1/lists',
-			method: 'post',
-			data: { lists },
-		} )
-			.then( updateConfig )
-			.catch( setError )
-			.finally( () => setInFlight( false ) );
-	};
-	const handleChange = ( index, name ) => value => {
-		const newLists = [ ...lists ];
-		newLists[ index ][ name ] = value;
-		updateConfig( newLists );
+		try {
+			const response = await apiFetch( {
+				path: `/newspack-newsletters/v1/lists/${ list.db_id }`,
+				method: 'PATCH',
+				data: { active: next },
+			} );
+			updateConfig( lists.map( row => ( row.db_id === list.db_id ? { ...row, ...response } : row ) ) );
+		} catch ( err ) {
+			updateConfig( snapshot );
+			setError( err );
+		} finally {
+			setTogglingId( null );
+		}
 	};
-	// Handle provider updates.
+
 	useEffect( () => {
 		setError( false );
 		if ( provider && ! lockedLists ) {
-			// Empty lists before fetching to prevent previous list from appearing while fetching.
 			setLists( [] );
 			fetchLists();
 		}
 	}, [ provider, lockedLists ] );
 
+	useEffect( () => {
+		const reload = () => fetchLists();
+		document.addEventListener( NN_EVENTS.LOCAL_LIST_SAVED, reload );
+		document.addEventListener( NN_EVENTS.LOCAL_LIST_DELETED, reload );
+		return () => {
+			document.removeEventListener( NN_EVENTS.LOCAL_LIST_SAVED, reload );
+			document.removeEventListener( NN_EVENTS.LOCAL_LIST_DELETED, reload );
+		};
+	}, [] );
+
+	const startFallbackTimer = fallbackUrl => {
+		if ( isBridgeReady() || ! fallbackUrl ) {
+			return;
+		}
+		clearTimeout( fallbackTimerRef.current );
+		fallbackTimerRef.current = setTimeout( () => {
+			if ( ! isBridgeReady() ) {
+				window.location.href = fallbackUrl;
+			}
+		}, NN_FALLBACK_TIMEOUT_MS );
+	};
+
+	const dispatchOpenAdd = () => {
+		document.dispatchEvent( new CustomEvent( NN_EVENTS.OPEN_MODAL, { detail: { mode: 'add' } } ) );
+		startFallbackTimer( newspack_newsletters_wizard.new_subscription_lists_url );
+	};
+	const dispatchOpenEdit = ( list, kind ) => {
+		document.dispatchEvent( new CustomEvent( NN_EVENTS.OPEN_MODAL, { detail: { mode: 'edit', kind, list } } ) );
+		startFallbackTimer( list?.edit_link );
+	};
+	const dispatchConfirmDelete = list => {
+		document.dispatchEvent( new CustomEvent( NN_EVENTS.OPEN_CONFIRM_DELETE, { detail: { list } } ) );
+		startFallbackTimer( list?.edit_link );
+	};
+
 	if ( ! inFlight && ! lists?.length && ! error ) {
 		return null;
 	}
-
 	if ( inFlight && ! lists?.length && ! error ) {
 		return (
 			<div className="flex justify-around mt4">
@@ -323,61 +387,62 @@ export const SubscriptionLists = ( { lockedLists, onUpdate, provider } ) => {
 			notificationLevel={ error ? 'error' : 'warning' }
 			hasGreyHeader
 			actionContent={
-				<>
-					{ newspack_newsletters_wizard.new_subscription_lists_url && (
-						<Button
-							variant="secondary"
-							disabled={ inFlight || lockedLists }
-							href={ newspack_newsletters_wizard.new_subscription_lists_url }
-						>
-							{ __( 'Add New', 'newspack-plugin' ) }
-						</Button>
-					) }
-					<Button isPrimary onClick={ saveLists } disabled={ inFlight || lockedLists }>
-						{ __( 'Save Subscription Lists', 'newspack-plugin' ) }
+				newspack_newsletters_wizard.new_subscription_lists_url && (
+					<Button variant="secondary" disabled={ inFlight || lockedLists } onClick={ dispatchOpenAdd }>
+						{ __( 'Add New', 'newspack-plugin' ) }
 					</Button>
-				</>
+				)
 			}
 			disabled={ inFlight || lockedLists }
 		>
 			{ ! lockedLists &&
 				! error &&
-				lists.map( ( list, index ) => (
-					<ActionCard
-						key={ index }
-						isSmall
-						simple
-						hasWhiteHeader
-						title={ list.name }
-						description={ list?.type_label ? list.type_label : null }
-						disabled={ inFlight }
-						toggleOnChange={ handleChange( index, 'active' ) }
-						toggleChecked={ list.active }
-						className={
-							list?.id && ( list.id.startsWith( 'group' ) || list.id.startsWith( 'tag' ) ) ? 'newspack-newsletters-sub-list-item' : ''
-						}
-						actionText={
-							list?.edit_link ? <ExternalLink href={ list.edit_link }>{ __( 'Edit', 'newspack-plugin' ) }</ExternalLink> : null
-						}
-					>
-						{ list.active && 'local' !== list?.type && (
-							<>
-								<TextControl
-									label={ __( 'List title', 'newspack-plugin' ) }
-									value={ list.title }
-									disabled={ inFlight || 'local' === list?.type }
-									onChange={ handleChange( index, 'title' ) }
-								/>
-								<TextareaControl
-									label={ __( 'List description', 'newspack-plugin' ) }
-									value={ list.description }
-									disabled={ inFlight || 'local' === list?.type }
-									onChange={ handleChange( index, 'description' ) }
-								/>
-							</>
-						) }
-					</ActionCard>
-				) ) }
+				lists.map( ( list, index ) => {
+					const isLocal = 'local' === list?.type;
+					const rowDisabled = inFlight || togglingId === list?.db_id;
+					return (
+						<ActionCard
+							key={ index }
+							isSmall
+							simple
+							hasWhiteHeader
+							title={ list.name }
+							description={ () => (
+								<>
+									{ list.description }
+									{ list.description && list?.type_label && <br /> }
+									{ list?.type_label && (
+										<small className="newspack-newsletters-sub-list-item__type-label">{ list.type_label }</small>
+									) }
+								</>
+							) }
+							disabled={ rowDisabled }
+							toggleOnChange={ next => handleToggleActive( list, next ) }
+							toggleChecked={ list.active }
+							className={
+								list?.id && ( list.id.startsWith( 'group' ) || list.id.startsWith( 'tag' ) )
+									? 'newspack-newsletters-sub-list-item'
+									: ''
+							}
+							actionText={
+								<HStack spacing={ 2 } justify="flex-end" expanded={ false }>
+									<Button
+										variant="link"
+										onClick={ () => dispatchOpenEdit( list, isLocal ? 'local' : 'esp' ) }
+										disabled={ rowDisabled }
+									>
+										{ __( 'Edit', 'newspack-plugin' ) }
+									</Button>
+									{ isLocal && (
+										<Button variant="link" isDestructive onClick={ () => dispatchConfirmDelete( list ) } disabled={ rowDisabled }>
+											{ __( 'Delete', 'newspack-plugin' ) }
+										</Button>
+									) }
+								</HStack>
+							}
+						/>
+					);
+				} ) }
 		</ActionCard>
 	);
 };

src/wizards/newsletters/views/settings/index.test.js

@@ -0,0 +1,138 @@
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import apiFetch from '@wordpress/api-fetch';
+
+import { SubscriptionLists } from './index';
+
+jest.mock( '@wordpress/api-fetch', () => jest.fn() );
+
+const NN_EVENTS = {
+	BRIDGE_MOUNTED: 'newspack-newsletters:bridge-mounted',
+	OPEN_MODAL: 'newspack-newsletters:open-local-list-modal',
+	OPEN_CONFIRM_DELETE: 'newspack-newsletters:open-local-list-confirm-delete',
+	LOCAL_LIST_SAVED: 'newspack-newsletters:local-list-saved',
+	LOCAL_LIST_DELETED: 'newspack-newsletters:local-list-deleted',
+};
+
+beforeAll( () => {
+	global.newspack_newsletters_wizard = {
+		new_subscription_lists_url: 'https://example.test/wp-admin/post-new.php?post_type=newspack_nl_list',
+	};
+} );
+
+beforeEach( () => {
+	apiFetch.mockReset();
+	apiFetch.mockResolvedValue( [
+		{ id: 'tag-1', name: 'Local A', type: 'local', active: false, db_id: 1, edit_link: 'https://example.test/edit-local-a' },
+		{ id: 'group-1', name: 'Remote group', type: 'group', active: true, db_id: 2, edit_link: 'https://example.test/edit-remote' },
+	] );
+	// Mark the bridge ready so the fallback timer doesn't navigate the test window.
+	window.newspackNewslettersBridgeReady = true;
+} );
+
+afterEach( () => {
+	delete window.newspackNewslettersBridgeReady;
+} );
+
+describe( 'SubscriptionLists — wizard-bridge wiring', () => {
+	it( 'dispatches OPEN_MODAL with mode=add when Add New is clicked', async () => {
+		const listener = jest.fn();
+		document.addEventListener( NN_EVENTS.OPEN_MODAL, listener );
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( screen.getByRole( 'button', { name: /^Add New$/ } ) ).toBeEnabled() );
+		fireEvent.click( screen.getByRole( 'button', { name: /^Add New$/ } ) );
+		expect( listener ).toHaveBeenCalled();
+		expect( listener.mock.calls[ 0 ][ 0 ].detail ).toEqual( { mode: 'add' } );
+		document.removeEventListener( NN_EVENTS.OPEN_MODAL, listener );
+	} );
+
+	it( 'dispatches OPEN_MODAL with mode=edit + kind=local when Edit is clicked on a local row', async () => {
+		const listener = jest.fn();
+		document.addEventListener( NN_EVENTS.OPEN_MODAL, listener );
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( screen.getByText( 'Local A' ) ).toBeInTheDocument() );
+		fireEvent.click( screen.getAllByRole( 'button', { name: /^Edit$/ } )[ 0 ] );
+		expect( listener.mock.calls[ 0 ][ 0 ].detail ).toEqual(
+			expect.objectContaining( { mode: 'edit', kind: 'local', list: expect.objectContaining( { db_id: 1 } ) } )
+		);
+		document.removeEventListener( NN_EVENTS.OPEN_MODAL, listener );
+	} );
+
+	it( 'dispatches OPEN_MODAL with mode=edit + kind=esp when Edit is clicked on a remote row', async () => {
+		const listener = jest.fn();
+		document.addEventListener( NN_EVENTS.OPEN_MODAL, listener );
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( screen.getByText( 'Remote group' ) ).toBeInTheDocument() );
+		// Remote rows now have an Edit button too — second one in the list.
+		fireEvent.click( screen.getAllByRole( 'button', { name: /^Edit$/ } )[ 1 ] );
+		expect( listener.mock.calls[ 0 ][ 0 ].detail ).toEqual(
+			expect.objectContaining( { mode: 'edit', kind: 'esp', list: expect.objectContaining( { db_id: 2 } ) } )
+		);
+		document.removeEventListener( NN_EVENTS.OPEN_MODAL, listener );
+	} );
+
+	it( 'commits the active toggle immediately via PATCH /lists/{db_id}', async () => {
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( screen.getByText( 'Local A' ) ).toBeInTheDocument() );
+		// Configure the next response (a successful PATCH echoing the row).
+		apiFetch.mockResolvedValueOnce( { id: 'tag-1', db_id: 1, active: true } );
+		fireEvent.click( screen.getAllByRole( 'checkbox' )[ 0 ] );
+		await waitFor( () =>
+			expect( apiFetch ).toHaveBeenLastCalledWith( {
+				path: '/newspack-newsletters/v1/lists/1',
+				method: 'PATCH',
+				data: { active: true },
+			} )
+		);
+	} );
+
+	it( 'does not render the bulk Save Subscription Lists button', async () => {
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( screen.getByText( 'Local A' ) ).toBeInTheDocument() );
+		expect( screen.queryByRole( 'button', { name: /Save Subscription Lists/ } ) ).not.toBeInTheDocument();
+	} );
+
+	it( 'does not render inline title/description fields on remote rows', async () => {
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( screen.getByText( 'Remote group' ) ).toBeInTheDocument() );
+		expect( screen.queryByLabelText( /List title/ ) ).not.toBeInTheDocument();
+		expect( screen.queryByLabelText( /List description/ ) ).not.toBeInTheDocument();
+	} );
+
+	it( 'dispatches OPEN_CONFIRM_DELETE when Delete is clicked on a local row', async () => {
+		const listener = jest.fn();
+		document.addEventListener( NN_EVENTS.OPEN_CONFIRM_DELETE, listener );
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( screen.getByText( 'Local A' ) ).toBeInTheDocument() );
+		fireEvent.click( screen.getByRole( 'button', { name: /^Delete$/ } ) );
+		expect( listener.mock.calls[ 0 ][ 0 ].detail ).toEqual( expect.objectContaining( { list: expect.objectContaining( { db_id: 1 } ) } ) );
+		document.removeEventListener( NN_EVENTS.OPEN_CONFIRM_DELETE, listener );
+	} );
+
+	it( 'reloads lists when LOCAL_LIST_SAVED fires', async () => {
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( apiFetch ).toHaveBeenCalledTimes( 1 ) );
+		document.dispatchEvent( new CustomEvent( NN_EVENTS.LOCAL_LIST_SAVED, { detail: { listId: 1, mode: 'edit' } } ) );
+		await waitFor( () => expect( apiFetch ).toHaveBeenCalledTimes( 2 ) );
+	} );
+
+	it( 'reloads lists when LOCAL_LIST_DELETED fires', async () => {
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( apiFetch ).toHaveBeenCalledTimes( 1 ) );
+		document.dispatchEvent( new CustomEvent( NN_EVENTS.LOCAL_LIST_DELETED, { detail: { listId: 1 } } ) );
+		await waitFor( () => expect( apiFetch ).toHaveBeenCalledTimes( 2 ) );
+	} );
+
+	it( 'does not redirect when the bridge mounted before the wizard listener registered', async () => {
+		// The flag is already set in beforeEach, simulating the bridge having
+		// completed boot before this component mounted. The fallback timer
+		// must NOT navigate.
+		jest.useFakeTimers();
+		const originalHref = window.location.href;
+		render( <SubscriptionLists lockedLists={ false } provider="mailchimp" /> );
+		await waitFor( () => expect( screen.getByRole( 'button', { name: /^Add New$/ } ) ).toBeEnabled() );
+		fireEvent.click( screen.getByRole( 'button', { name: /^Add New$/ } ) );
+		jest.advanceTimersByTime( 600 );
+		expect( window.location.href ).toBe( originalHref );
+		jest.useRealTimers();
+	} );
+} );