Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Newsletter: Only show the subscription placement "Preview and edit" link once the placement is enabled and saved.
48 changes: 43 additions & 5 deletions projects/packages/newsletter/src/settings/newsletter-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,26 @@ function normalizeSettings( settings: Record< string, unknown > ): NewsletterSet
// without the flash.
let cachedSettings: NewsletterSettings | null = null;

// Module-level cache of the last *persisted* settings — the saved baseline,
// as opposed to `cachedSettings` above which mirrors the optimistic (possibly
// unsaved) `data`. Kept in sync on fetch and after every successful save, and
// module-cached for the same reason: so a remount on tab-return knows the
// saved state immediately instead of treating it as unknown until the
// background refetch resolves. Consumed by the Subscriptions section to gate
// each placement's "Preview and edit" link on whether that placement is
// enabled *in the saved state* (an unsaved toggle previews nothing).
let cachedSavedSettings: NewsletterSettings | null = null;

/**
* Test-only: clear the module-level settings cache so each test starts cold.
* The cache deliberately outlives any single component instance (that's the
* whole point — it survives the tab unmount/remount), which also means it
* leaks across tests in a file. Call this in `beforeEach` to keep cold-cache
* Test-only: clear the module-level settings caches so each test starts cold.
* The caches deliberately outlive any single component instance (that's the
* whole point — they survive the tab unmount/remount), which also means they
* leak across tests in a file. Call this in `beforeEach` to keep cold-cache
* assertions order-independent.
*/
export function __resetNewsletterSettingsCacheForTests(): void {
cachedSettings = null;
cachedSavedSettings = null;
}

export type NewsletterSettingsBodyProps = {
Expand Down Expand Up @@ -136,6 +147,11 @@ export function NewsletterSettingsBody( {
// Seed from the module cache so a remount (e.g. returning to the Settings
// tab) paints immediately instead of flashing the full-page spinner.
const [ data, setData ] = useState< NewsletterSettings | null >( () => cachedSettings );
// Last persisted settings (saved baseline), seeded from the module cache so a
// remount knows the saved state without waiting on the background refetch.
const [ savedData, setSavedData ] = useState< NewsletterSettings | null >(
() => cachedSavedSettings
);
const [ isLoading, setIsLoading ] = useState( () => ! cachedSettings );
const [ error, setError ] = useState< string | null >( null );

Expand Down Expand Up @@ -203,7 +219,10 @@ export function NewsletterSettingsBody( {
useEffect( () => {
fetchSettings()
.then( ( settings: Record< string, unknown > ) => {
setData( normalizeSettings( settings ) );
const normalized = normalizeSettings( settings );
setData( normalized );
// Server truth on load is both the optimistic value and the saved baseline.
setSavedData( normalized );
setIsLoading( false );
} )
.catch( ( err: Error ) => {
Expand Down Expand Up @@ -238,6 +257,14 @@ export function NewsletterSettingsBody( {
}
}, [ data ] );

// Mirror the saved baseline to the module cache so it, too, survives a
// remount (parallel to `cachedSettings` above, but for persisted values).
useEffect( () => {
if ( savedData ) {
cachedSavedSettings = savedData;
}
}, [ savedData ] );

// Handle auto-save for newsletter toggle and email settings.
const handleAutoSave = useCallback(
( updates: Partial< NewsletterSettings > ) => {
Expand Down Expand Up @@ -277,6 +304,8 @@ export function NewsletterSettingsBody( {
// Save to backend.
updateSettings( updates )
.then( () => {
// Advance the saved baseline now that these values are persisted.
setSavedData( prev => ( { ...prev, ...updates } ) );
createSuccessNotice( __( 'Settings saved', 'jetpack-newsletter' ) );
} )
.catch( ( err: Error ) => {
Expand Down Expand Up @@ -308,6 +337,7 @@ export function NewsletterSettingsBody( {

updateSettings( senderNameChanges )
.then( () => {
setSavedData( prev => ( { ...prev, ...senderNameChanges } ) );
setSenderNameChanges( {} );
createSuccessNotice( __( 'Sender name saved', 'jetpack-newsletter' ) );
} )
Expand Down Expand Up @@ -341,6 +371,9 @@ export function NewsletterSettingsBody( {

updateSettings( subscriptionChanges )
.then( () => {
// Advance the saved baseline so each placement's "Preview and edit"
// link reflects the just-saved enabled state.
setSavedData( prev => ( { ...prev, ...subscriptionChanges } ) );
setSubscriptionChanges( {} );
createSuccessNotice( __( 'Settings saved', 'jetpack-newsletter' ) );
} )
Expand Down Expand Up @@ -395,6 +428,8 @@ export function NewsletterSettingsBody( {

updateSettings( apiUpdates )
.then( () => {
// Merge the staged (string) values so the baseline mirrors `data`'s shape.
setSavedData( prev => ( { ...prev, ...newsletterCategoriesChanges } ) );
setNewsletterCategoriesChanges( {} );
createSuccessNotice( __( 'Newsletter categories saved', 'jetpack-newsletter' ) );
} )
Expand Down Expand Up @@ -428,6 +463,7 @@ export function NewsletterSettingsBody( {

updateSettings( welcomeEmailChanges )
.then( () => {
setSavedData( prev => ( { ...prev, ...welcomeEmailChanges } ) );
setWelcomeEmailChanges( {} );
createSuccessNotice( __( 'Welcome email message saved', 'jetpack-newsletter' ) );
} )
Expand Down Expand Up @@ -459,6 +495,7 @@ export function NewsletterSettingsBody( {

updateSettings( subscribeModalChanges )
.then( () => {
setSavedData( prev => ( { ...prev, ...subscribeModalChanges } ) );
setSubscribeModalChanges( {} );
createSuccessNotice( __( 'Subscribe modal heading saved', 'jetpack-newsletter' ) );
} )
Expand Down Expand Up @@ -542,6 +579,7 @@ export function NewsletterSettingsBody( {

<SubscriptionsSection
data={ data }
savedData={ savedData }
onChange={ handleSubscriptionChange }
onSave={ saveSubscriptionSettings }
isSaving={ isSavingSubscriptions }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ const PLACEMENT_SLUG_BY_KEY: Record< string, string > = {

interface SubscriptionsSectionProps {
data: NewsletterSettings;
/**
* Last *persisted* settings. A placement's "Preview and edit" link only
* shows when the placement is enabled here (saved), not merely toggled on
* in the optimistic `data` — otherwise the link opens an empty preview.
*/
savedData: NewsletterSettings | null;
onChange: ( updates: Partial< NewsletterSettings > ) => void;
onSave: () => void;
isSaving: boolean;
Expand All @@ -60,6 +66,7 @@ interface SubscriptionsSectionProps {
*/
export function SubscriptionsSection( {
data,
savedData,
onChange,
onSave,
isSaving,
Expand Down Expand Up @@ -230,20 +237,32 @@ export function SubscriptionsSection( {
{ __( 'Homepage and posts', 'jetpack-newsletter' ) }
</Text>
<div className="jetpack-newsletter-placements-grid">
{ placements.map( placement => (
<PlacementCard
key={ placement.key }
id={ `placement-${ placement.key }` }
name={ String( placement.key ) }
title={ placement.title }
illustration={ placement.illustration }
previewUrl={ placement.previewUrl }
checked={ Boolean( data[ placement.key ] ) }
onChange={ handlePlacementChange }
onPreviewClick={ handlePlacementPreviewClick }
disabled={ ! isNewsletterEnabled }
/>
) ) }
{ placements.map( placement => {
// Surface "Preview and edit" only when the placement is
// enabled in the SAVED state AND still shown as checked —
// otherwise the link opens an empty Site Editor preview.
// Gating on `savedData` (the persisted baseline) rather than
// "untouched since save" keeps the link reversible: toggle a
// saved-on placement off then back on and it returns, while a
// freshly enabled-but-unsaved placement stays linkless until
// the save lands.
const isEnabledAndSaved =
Boolean( data[ placement.key ] ) && Boolean( savedData?.[ placement.key ] );
return (
<PlacementCard
key={ placement.key }
id={ `placement-${ placement.key }` }
name={ String( placement.key ) }
title={ placement.title }
illustration={ placement.illustration }
previewUrl={ isEnabledAndSaved ? placement.previewUrl : undefined }
checked={ Boolean( data[ placement.key ] ) }
onChange={ handlePlacementChange }
onPreviewClick={ handlePlacementPreviewClick }
disabled={ ! isNewsletterEnabled }
/>
);
} ) }
</div>
</Stack>

Expand Down
Loading
Loading