feat(emails): inline EmailPreview component

[kmwilkerson]

What this PR does

Adds an inline email preview component (NPPD-1525) that renders a live thumbnail of each email's HTML inside the DataViews grid built in #4727. Replaces the envelope-icon placeholder with real previews — readers see the actual rendered template at a glance.

⚠️ Stacked on #4727

Base branch is nppd-945-unified-emails-newspack-slice, not trunk.

The diff in this PR shows only the NPPD-1525 work. The slice 1 changes (#4727) are excluded from the diff by virtue of the base branch.

Code changes

Backend (includes/wizards/newspack/class-email-preview.php)

• New Email_Preview class registers GET /wizard/newspack-settings/emails/{post_id}/preview
• Returns the cached EMAIL_HTML_META post meta with sample-value token substitution applied
• Falls back to rendering the template file if no cached HTML is stored
• Substitutes 29 template tokens: reader/transaction values use stable fakes (e.g. "Sample Reader", "$25.00", "May 14, 2026"); site/branding values use real publisher values from get_option; action URLs use # anchors

Frontend (src/wizards/newspack/views/settings/emails/)

• New EmailPreview React component
• IntersectionObserver lazy-load via isVisible state bridge — cards only fetch when scrolled into view, avoiding 24 simultaneous requests on initial render
• Iframe srcDoc rendering with empty sandbox="" for security (scripts in email HTML cannot execute)
• Spinner loading state, envelope icon fallback on render error
• ResizeObserver-based proportional scaling: source iframe rendered at 1200×900, scaled via inline transform to match container width while preserving 4:3 aspect ratio
• Wired into emails.tsx to replace the envelope-icon placeholder in the Preview column

Loader fix (includes/class-newspack.php)

• Fixed missing tab indentation on the new class-email-preview.php include line

Tests

• 6 PHPUnit tests (tests/unit-tests/email-preview.php): substitution map, preview HTML with stored meta, template fallback, nonexistent post, wrong post type returns 404, successful API response
• 5 Jest tests (src/wizards/newspack/views/settings/emails/email-preview.test.js): loading state, successful iframe render, error fallback, lazy-load gating, correct endpoint path

Manual testing

Tested locally on katie.local (Homebrew-served Apache + PHP, symlinked plugin) with the slice 1 grid view. Verified:

• ✅ Each card renders a live iframe thumbnail of the rendered email HTML
• ✅ Token substitution working (publisher name, dates, sample amounts visible in rendered previews)
• ✅ IntersectionObserver gates fetching — initial page load doesn't trigger all 24 requests
• ✅ Iframe scales proportionally as container width changes
• ✅ Fallback envelope icon renders when REST returns an error

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

[thomasguillot]

Nice work on the lazy-load + scaling plumbing. Before this lands, please align the rendering contract with our existing NewsletterPreview in newspack-newsletters so the two grids feel like the same surface side-by-side. These are separate plugins — copy the relevant logic into newspack-plugin rather than extracting a shared package. Use newspack-newsletters as a read-only reference for the shape.

Reference (read-only, in newspack-newsletters)

• Component: src/components/newsletter-preview/index.js — see lines 43–215, particularly the useInlineStyles hook's iframe-readiness detection (lines 70–193) and the is-ready/fade-in pattern (lines 195–214).
• Styles: src/components/newsletter-preview/style.scss — note the .block-editor-block-preview__container { opacity: 0; transition: opacity 200ms ease-out; } → .is-ready & fade.
• Grid wrapper: src/admin-shell/screens/layouts-list/fields.js PreviewCard (lines 197–242) — uses aspectRatio: '1', viewportWidth={ 848 }, and rootMargin: "200px".
• Lazy helper: src/admin-shell/screens/layouts-list/lazy-preview.js.

Concrete changes to make in this PR

All paths below are in newspack-plugin.

1. Aspect ratio: 1 : 1

File: src/wizards/newspack/views/settings/emails/email-preview.scss

-.newspack-email-preview {
-    width: 100%;
-    aspect-ratio: 4 / 3;
-    overflow: hidden;
-    ...
-    background: #f6f7f7;
+.newspack-email-preview {
+    width: 100%;
+    aspect-ratio: 1;
+    overflow: hidden;
+    ...
+    background: transparent;

Rationale: newsletters layouts grid uses 1 : 1 (<LazyPreview placeholderStyle={{ aspectRatio: '1' }} />). Cards sit side-by-side in the same DataView shell, so matching makes them visually coherent. Transparent background lets the card chrome show through consistently.

2. Source viewport: 848 px wide, intrinsic height

File: src/wizards/newspack/views/settings/emails/email-preview.tsx

Replace the IFRAME_WIDTH = 600 constant and the fixed-size iframe with width 848, height measured from the loaded document. Concretely:

const IFRAME_WIDTH = 848; // matches NewsletterPreview's `viewportWidth={ 848 }`

Update the SCSS so __iframe no longer hardcodes height:

&__iframe {
    width: 848px;
    height: auto; // set dynamically after load
    border: 0;
    position: absolute;
    top: 0;
    left: 0;
    transform-origin: top left;
    pointer-events: none;
}

Then measure the natural document height after the iframe loads and set it inline. Add an onLoad handler:

const iframeRef = useRef<HTMLIFrameElement>( null );
const [ iframeHeight, setIframeHeight ] = useState< number | null >( null );
const [ isReady, setIsReady ] = useState( false );

const handleIframeLoad = () => {
    const doc = iframeRef.current?.contentDocument;
    if ( ! doc ) {
        return;
    }
    // Mirror NewsletterPreview's readiness detection: wait for stylesheets
    // and images before measuring + revealing, otherwise the height we read
    // is mid-layout and the iframe pops in unstyled.
    const awaitLoad = ( el: HTMLLinkElement | HTMLImageElement ) =>
        new Promise< void >( resolve => {
            el.addEventListener( 'load', () => resolve(), { once: true } );
            el.addEventListener( 'error', () => resolve(), { once: true } );
        } );
    const linkPromises = Array.from(
        doc.querySelectorAll< HTMLLinkElement >( 'link[rel="stylesheet"]' )
    )
        .filter( link => ! link.sheet )
        .map( awaitLoad );
    const imgPromises = Array.from( doc.querySelectorAll< HTMLImageElement >( 'img' ) )
        .filter( img => ! img.complete )
        .map( awaitLoad );
    // 8s safety so a slow asset never strands the spinner — matches NewsletterPreview line 81.
    const safety = setTimeout( () => {
        setIframeHeight( doc.body.scrollHeight );
        setIsReady( true );
    }, 8000 );
    Promise.all( [ ...linkPromises, ...imgPromises ] ).then( () => {
        clearTimeout( safety );
        setIframeHeight( doc.body.scrollHeight );
        setIsReady( true );
    } );
};

And on the <iframe> element:

<iframe
    ref={ iframeRef }
    className="newspack-email-preview__iframe"
    srcDoc={ html }
    sandbox=""
    tabIndex={ -1 }
    title="Email preview"
    onLoad={ handleIframeLoad }
    style={ {
        transform: `scale(${ scale })`,
        height: iframeHeight ? `${ iframeHeight }px` : undefined,
    } }
/>

Scale stays containerWidth / IFRAME_WIDTH — but since IFRAME_WIDTH is now 848, the math changes accordingly.

Also: the PR description says 1200 × 900 but the SCSS as written is 600 × 1800. Please remove that line from the PR description once 848 × auto is the actual value.

3. Ready-state fade-in

Add the is-ready class pattern from NewsletterPreview.

Component:

<div
    ref={ containerRef }
    className={ `newspack-email-preview${ isReady ? ' is-ready' : '' }` }
>

SCSS:

.newspack-email-preview {
    ...
    &__iframe {
        ...
        opacity: 0;
        transition: opacity 200ms ease-out;
    }

    &.is-ready &__iframe {
        opacity: 1;
    }
}

Keep the existing spinner placeholder mounted while ! isReady so the transition is spinner → fade-in iframe (no white flash).

4. Render gating

The current condition is html && ! hasError && ! isLoading && scale > 0. After the changes above, also render the iframe as soon as html arrives (so onLoad can fire and measure) — the iframe itself is hidden by opacity: 0 until isReady flips, and the spinner placeholder stays mounted in the meantime. Concretely:

{ html && ! hasError && scale > 0 && (
    <iframe ... />
) }
{ ( isLoading || ( html && ! isReady ) ) && ! hasError && (
    <div className="newspack-email-preview__placeholder">
        <Spinner />
    </div>
) }

5. Iframe-state reset on postId change

When postId changes, reset html, isReady, iframeHeight, hasError so the next preview doesn't show stale content while loading. Add to the fetch effect:

useEffect( () => {
    if ( ! isVisible ) {
        return;
    }
    setIsLoading( true );
    setIsReady( false );
    setIframeHeight( null );
    setHasError( false );
    setHtml( null );
    apiFetch...
}, [ isVisible, postId ] );

6. Tests

Update src/wizards/newspack/views/settings/emails/email-preview.test.js:

• The "renders iframe on successful fetch" test will need to simulate onLoad. After srcdoc is set, fire iframe.dispatchEvent(new Event('load')) and assert the container gains the is-ready class.
• Stub iframe.contentDocument to return an object with querySelectorAll returning [] and body.scrollHeight returning a number, so handleIframeLoad resolves immediately.
• Add a test for the reset-on-postId-change behaviour.

Out of scope (don't do)

• Don't extract a shared component into a package — these are separate plugins, duplication is fine.
• Don't honour meta.background_color / fonts here (system emails don't carry that meta; the newsletters version reads it from layout meta).
• Don't reuse LazyPreview from newsletters — your inline IntersectionObserver already matches its rootMargin: '200px' contract, which is what matters.

Acceptance check

When done, an Email card and a Newsletter Layout card placed side-by-side should:

• Be the same shape (1 : 1).
• Show a comparable amount of vertical content (header + first content block roughly fills the square).
• Fade in from spinner with no FOUC.
• Show transparent background behind the rendered HTML.

[Copilot]

Pull request overview

Adds a live “thumbnail” preview for each transactional email in the Newspack Settings → Emails DataViews UI by introducing a REST endpoint that returns rendered email HTML (with sample token substitution) and a React component that lazy-loads and displays that HTML in a sandboxed iframe.

Changes:

• Backend: Introduces Email_Preview with a GET /wizard/newspack-settings/emails/{post_id}/preview REST route to return cached email HTML (or template fallback) with token substitutions.
• Frontend: Adds an EmailPreview component (IntersectionObserver + ResizeObserver) and wires it into the Emails DataViews “Preview” column.
• Tests: Adds PHPUnit coverage for preview HTML/token substitution and Jest coverage for the EmailPreview component behavior.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
includes/wizards/newspack/class-email-preview.php	Adds REST route + preview HTML rendering/token substitution logic.
includes/class-newspack.php	Includes the new class-email-preview.php loader.
src/wizards/newspack/views/settings/emails/emails.tsx	Replaces preview placeholder with <EmailPreview /> in the DataViews grid.
src/wizards/newspack/views/settings/emails/email-preview.tsx	New React component that lazy-loads and renders sandboxed iframe previews.
src/wizards/newspack/views/settings/emails/email-preview.scss	Styling for the preview thumbnail container/iframe/placeholder states.
src/wizards/newspack/views/settings/emails/emails.scss	Removes now-obsolete preview placeholder styles.
tests/unit-tests/email-preview.php	New PHPUnit tests for backend preview/substitution and REST handler behavior.
src/wizards/newspack/views/settings/emails/email-preview.test.js	New Jest tests for lazy-load gating, success/error states, and endpoint path.
src/wizards/newspack/views/settings/emails/emails.test.js	Adjusts Emails tests to mock EmailPreview and provide ResizeObserver.
includes/reader-activation/integrations/README.md	Adds documentation for the reader activation integrations framework.

Comments suppressed due to low confidence (3)

includes/wizards/newspack/class-email-preview.php:114

• get_sample_substitutions() uses function_exists( 'Newspack\Emails::get_reply_to_email' ), but function_exists does not work for class methods, so this condition will always be false and the preview will ignore Emails::get_reply_to_email() (and any Reader Activation override). Use method_exists( Emails::class, 'get_reply_to_email' ) or is_callable( [ Emails::class, 'get_reply_to_email' ] ) instead, or just call Emails::get_reply_to_email() directly since this class already depends on Newspack\Emails.

		$site_logo_url   = wp_get_attachment_url( get_theme_mod( 'custom_logo' ) );
		$site_title      = get_bloginfo( 'name' );
		$site_url        = get_bloginfo( 'wpurl' );
		$reply_to_email  = function_exists( 'Newspack\Emails::get_reply_to_email' ) ? Emails::get_reply_to_email() : get_bloginfo( 'admin_email' );
		$site_address    = self::get_site_address();

includes/wizards/newspack/class-email-preview.php:127

• The *CONTACT_EMAIL* substitution builds an <a href="mailto:..."> string using $reply_to_email without escaping/sanitizing. Even if the value is expected to be an email, it should be passed through sanitize_email() and escaped for both the href attribute and link text to avoid HTML injection issues and to keep output consistent with WP escaping practices.

			'*SITE_ADDRESS*'           => $site_address,
			'*SITE_CONTACT*'           => $site_contact,
			'*CONTACT_EMAIL*'          => sprintf( '<a href="mailto:%s">%s</a>', $reply_to_email, $reply_to_email ),

src/wizards/newspack/views/settings/emails/email-preview.tsx:71

• EmailPreview assumes ResizeObserver exists. In environments without ResizeObserver, this will throw and prevent the iframe from rendering. Add a feature check with a safe fallback (e.g. compute scale from node.getBoundingClientRect().width once, or default scale to 1 and skip observing).

	// Measure container width and compute iframe scale.
	useEffect( () => {
		const node = containerRef.current;
		if ( ! node ) {
			return;
		}

		const ro = new ResizeObserver( ( [ entry ] ) => {
			setScale( entry.contentRect.width / IFRAME_WIDTH );
		} );

		ro.observe( node );
		return () => ro.disconnect();

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kmwilkerson]

Addressed in f889cd6. All six items from the review:

1. Aspect ratio 1:1 — aspect-ratio: 1 in SCSS, background set to transparent
2. Source viewport 848px — IFRAME_WIDTH = 848, iframe width 848px in SCSS with height: auto
3. Ready-state fade-in — is-ready class on container, iframe starts at opacity: 0 with transition: opacity 200ms ease-out, revealed when is-ready is set
4. Render gating — iframe mounts as soon as HTML arrives (hidden via opacity), spinner stays until isReady
5. Iframe height measurement — onLoad handler waits for stylesheets + images via Promise.all, measures contentDocument.body.scrollHeight, 8s safety timeout
6. State reset on postId change — resets html, isReady, iframeHeight, hasError at the top of the fetch effect

Tests updated: is-ready class assertion after simulated onLoad, contentDocument stub, and reset-on-postId-change test. Build and 217 tests pass.

[thomasguillot]

Code review — EmailPreview component (slice 2 of unified emails)

Reviewed at 2d6fe465b against base branch nppd-945-unified-emails-newspack-slice (PR #4727 head ebb58fbed). Structured handoff for the agent that will pick this up — each finding includes file, line, severity, why, and remediation. There is one real bug (finding 1), one security trade-off worth a comment in code (finding 2), and a handful of resilience / cleanup items.

---

Bugs

1. function_exists() cannot test a Class::method — Emails::get_reply_to_email() is never called

includes/wizards/newspack/class-email-preview.php:118

$reply_to_email = function_exists( 'Newspack\Emails::get_reply_to_email' )
    ? Emails::get_reply_to_email()
    : get_bloginfo( 'admin_email' );

function_exists() only tests global functions. Verified locally:

php -r 'var_dump(function_exists("DateTime::format"));'   // bool(false)
php -r 'var_dump(method_exists("DateTime", "format"));'    // bool(true)

So the conditional is always false — Emails::get_reply_to_email() is unreachable, and *CONTACT_EMAIL* always falls back to admin_email. On sites where Reader Activation sets a contact_email_address option (which is exactly the publisher-facing reply-to behaviour previews should show), the preview will display the wrong address relative to the real outbound email.

Replace with method_exists:

$reply_to_email = method_exists( Emails::class, 'get_reply_to_email' )
    ? Emails::get_reply_to_email()
    : get_bloginfo( 'admin_email' );

Even simpler: Emails is use-imported at the top of the file and lives in the same plugin, so the guard is unnecessary. Just call Emails::get_reply_to_email() directly.

Pin it with a PHPUnit case: set the newspack_ras_contact_email_address option (or filter newspack_reply_to_email), generate a preview from a template containing *CONTACT_EMAIL*, and assert the rendered HTML contains the configured address rather than admin_email. The current substitution-map test would not have caught this because it doesn't render against a real template.

---

Security

2. sandbox="allow-same-origin" weakens the iframe sandbox — document the trade-off

src/wizards/newspack/views/settings/emails/email-preview.tsx:164

The commit message (2d6fe465b) explains why the change was made: without allow-same-origin, iframe.contentDocument is null, so the handleIframeLoad measurement logic can't read body.scrollHeight or query stylesheets. That's correct — same-origin is required for the measurement contract borrowed from NewsletterPreview.

But the security analysis isn't in the code. The relevant facts:

• allow-scripts is not in the sandbox attribute, so scripts in the email HTML cannot execute. Good.
• allow-same-origin alone (without allow-scripts) does not let the iframe steal cookies or localStorage because it can't run JS.
• But <form action="…"> submissions still work without scripts, and with allow-same-origin they submit from the same origin (i.e. with the admin's cookies). Email HTML rarely contains forms, but the preview reads HTML from EMAIL_HTML_META, which is publisher-editable. A malicious editor (or a compromised plugin writing to that meta) could embed a form pointing at an admin endpoint and trigger it on preview render.

Realistic risk: low — this is admin-only code and the attacker would already need write access to email post meta. But the safest hardening that preserves the measurement contract is to inject a <meta http-equiv="Content-Security-Policy" content="default-src 'none'; img-src *; style-src 'unsafe-inline' *; font-src *;"> into the srcDoc, which blocks form action / link navigation from the iframe entirely. This is also where you'd want <base target="_blank"> in case any anchor escapes the sandbox.

Minimum bar: add a comment above the sandbox attribute explaining (a) why allow-same-origin is required, (b) why scripts are still blocked, and (c) the residual form-action risk. The next reader should not need this review thread to reach the same conclusion.

---

Resilience / lifecycle

3. In-flight fetch isn't cancelled on postId change or unmount

src/wizards/newspack/views/settings/emails/email-preview.tsx:90-112

useEffect( () => {
    if ( ! isVisible ) return;
    setIsLoading( true );
    // ...resets...
    apiFetch( { /* ... */ } )
        .then( response => setHtml( response.html ) )
        .catch( () => setHasError( true ) )
        .finally( () => setIsLoading( false ) );
}, [ isVisible, postId ] );

If postId changes (DataViews re-uses rows when scrolling) or the component unmounts, the old promise still resolves and writes to state. The state-reset at the top of the new run happens before the next fetch, but the OLD then callback can still fire after the new fetch's then, leaving the iframe showing stale HTML.

Fix:

useEffect( () => {
    if ( ! isVisible ) return;
    let cancelled = false;
    setIsLoading( true );
    setIsReady( false );
    setIframeHeight( null );
    setHasError( false );
    setHtml( null );
    apiFetch< { html: string; post_id: number } >( {
        path: `/newspack/v1/wizard/newspack-settings/emails/${ postId }/preview`,
    } )
        .then( response => { if ( ! cancelled ) setHtml( response.html ); } )
        .catch( () => { if ( ! cancelled ) setHasError( true ); } )
        .finally( () => { if ( ! cancelled ) setIsLoading( false ); } );
    return () => { cancelled = true; };
}, [ isVisible, postId ] );

Same cancelled pattern already exists in NewsletterPreview (repos/newspack-newsletters/src/components/newsletter-preview/index.js:80) — worth aligning.

4. Safety timeout in handleIframeLoad is not cleared on unmount or postId change

src/wizards/newspack/views/settings/emails/email-preview.tsx:135-144

const safety = setTimeout( () => { setIframeHeight( ... ); setIsReady( true ); }, 8000 );
Promise.all( [...] ).then( () => { clearTimeout( safety ); /* ... */ } );

clearTimeout( safety ) only fires when Promise.all resolves. If the user scrolls the row out of view, switches tabs, or the component re-mounts before 8 seconds, the timer still ticks and calls setIframeHeight/setIsReady on a stale render. React will warn about state updates on an unmounted component (or silently overwrite state on the same postId, depending on timing).

Hoist the timeout into a ref or attach it via an effect with a cleanup:

const handleIframeLoad = useCallback( () => {
    // ... build promises ...
    const safetyId = setTimeout( finalize, 8000 );
    Promise.all( [...] ).then( () => {
        clearTimeout( safetyId );
        finalize();
    } );
    // No way to clean from here — either ref it, or track `cancelled` via the parent effect.
}, [] );

Cleaner: drive the load tracking from the same useEffect that fetches HTML, so a single cancelled flag covers both fetch and post-load measurement.

5. Promise.all().then() runs even after the safety timeout fires — redundant state writes

Same block as finding (4). If Promise.all resolves after 8 s (slow asset), both code paths fire setIframeHeight and setIsReady. React batches the second call, but it's still wasted work and obscures intent. The fix from (4) (single cancellation source) resolves this too.

6. No onError handler on the iframe element

src/wizards/newspack/views/settings/emails/email-preview.tsx:160-172

Network-level iframe failures (rare with srcDoc, but not impossible) leave the spinner spinning until the 8 s safety. Add onError={ () => setHasError( true ) }.

7. Initial paint shows nothing for a frame

src/wizards/newspack/views/settings/emails/email-preview.tsx:39, 159

const [ scale, setScale ] = useState( 0 );
// ...
{ html && ! hasError && scale > 0 && <iframe ... /> }

scale === 0 until ResizeObserver fires (one tick after mount). For that tick, neither the spinner nor the iframe is rendered (the spinner is gated by isLoading || (html && !isReady), and right after fetch resolves both flags can be false simultaneously for a frame). Cleanest fix: initialise scale to null and use scale !== null && scale > 0 in the guard, OR always render the spinner placeholder when ! isReady.

---

PHP — quality & correctness

8. *CONTACT_EMAIL* substitution embeds raw $reply_to_email into HTML

includes/wizards/newspack/class-email-preview.php:131

'*CONTACT_EMAIL*' => sprintf( '<a href="mailto:%s">%s</a>', $reply_to_email, $reply_to_email ),

This mirrors Emails::get_email_payload() (includes/emails/class-emails.php:215-218), which has the same issue. If the reply-to address ever contains characters that need escaping (it shouldn't for a sanitised email, but defence in depth), this becomes an HTML/URL injection vector. Recommend:

'*CONTACT_EMAIL*' => sprintf(
    '<a href="%s">%s</a>',
    esc_url( 'mailto:' . $reply_to_email ),
    esc_html( $reply_to_email )
),

Calling out the parallel bug in class-emails.php so this slice can either fix both or file a follow-up.

9. *DATE* uses gmdate() rather than site time

includes/wizards/newspack/class-email-preview.php:144

'*DATE*' => gmdate( get_option( 'date_format', 'F j, Y' ) ),

Real transactional emails use the publisher's timezone (wp_date() or date_i18n()). For a preview, a UTC date may show "May 14" while the publisher's locale shows "May 15". Use wp_date( get_option( 'date_format', 'F j, Y' ) ) — same call signature, but applies the site timezone and locale.

10. Template fallback uses raw include

includes/wizards/newspack/class-email-preview.php:82

$template_data = include $template_path;

Email templates today return arrays, but include runs arbitrary PHP. If a template file ever runs side-effecting code (action hooks, option writes), the preview request will trigger it. The is_readable() guard added in commit 92d6a42 protects against missing files but not against malicious or stateful templates.

Mitigations:

• Tighten the template path to a known safe directory: if ( 0 !== strpos( realpath( $template_path ), realpath( NEWSPACK_ABSPATH . 'includes/templates' ) ) ) return '';.
• Or: load the template via Emails::get_email_config_by_type() instead of include-ing the path yourself. The class already has the loading semantics worked out (class-emails.php:340 load_email_template).

11. No caching — every grid render does 12-24 substitution passes per preview

includes/wizards/newspack/class-email-preview.php:103-107

foreach ( self::get_sample_substitutions() as $token => $value ) {
    $html = str_replace( $token, $value, $html );
}

29 str_replace passes over the HTML, called per post_id, with no transient cache. For the visible grid (12-24 cards depending on viewport), that's 12-24 × 29 passes per page view. On a publisher's slow shared host this adds up.

Two cheap wins:

1. Use strtr( $html, $map ) — single pass over the string instead of N passes:

   return strtr( $html, self::get_sample_substitutions() );

2. Cache per post_id keyed on post_modified_gmt of the email post (transient or wp_cache_*). Invalidates automatically when the publisher edits the email.

12. Substitution map has no filter hook

includes/wizards/newspack/class-email-preview.php:114-164

get_sample_substitutions() is public static, which suggests it's extension-ready, but there's no apply_filters( 'newspack_email_preview_substitutions', $substitutions, $post_id ) to let other plugins inject sample values for their own tokens (e.g. group-subscription invite tokens, future tokens from PR follow-ups). Without this, every new token will require editing this file.

Add the filter, and have apply_sample_substitutions() accept $post_id so the filter can vary by email type.

13. *SITE_ADDRESS* / *SITE_CONTACT* logic duplicates Emails::get_email_payload

includes/wizards/newspack/class-email-preview.php:174-200 vs includes/emails/class-emails.php:186-211

The address-formatting logic is copy-pasted from Emails::get_email_payload. The docblock acknowledges this ("Mirrors the logic in Emails::get_email_payload()") but doesn't reference the line range, so the next refactor of either will silently diverge. Two options:

• Extract Emails::get_site_address(): string (or similar) as a public static helper and call it from both.
• At minimum, expand the docblock to "Mirror of Emails::get_email_payload() lines 186-211; keep in sync."

14. function_exists/class_exists checks for Newspack_Newsletters

includes/wizards/newspack/class-email-preview.php:210-212, 14-15

use Newspack_Newsletters;
// ...
return class_exists( 'Newspack_Newsletters' );

Importing a class via use doesn't trigger autoloading, so the class_exists check works. But the file unconditionally reads Newspack_Newsletters::EMAIL_HTML_META at line 58 — if Newspack_Newsletters is missing, get_source_html() runs before is_supported() is called from get_preview_html(), and the constant access would fail.

Trace: get_preview_html() (line 34) → is_supported() (line 35) returns false → returns false. Before reaching get_source_html(). So the order is safe.

But get_source_html() is private static and only called from get_preview_html() which guards, so it's defensive enough. Worth a one-line comment saying so.

15. *MAGIC_LINK_OTP* is a single fixed string

includes/wizards/newspack/class-email-preview.php:162

'*MAGIC_LINK_OTP*' => '123456' — a 6-digit fixed sample. If *MAGIC_LINK_OTP* ever lives in HTML that uses character-count constraints (very tight inline styles), 123456 is a reasonable typical width. No issue, but worth mentioning that if the real OTP format changes (e.g. 8 chars), this sample needs to track it.

---

Tests

16. The PHP tests don't cover the function_exists bug surface

tests/unit-tests/email-preview.php:69-90

The substitution-map test asserts keys exist; it doesn't verify that *CONTACT_EMAIL*'s value resolves through Emails::get_reply_to_email() when the option is set. Add a case that:

1. Sets update_option( 'newspack_ras_contact_email_address', 'reply@example.org' ) (or uses the filter).
2. Generates a preview of a template containing *CONTACT_EMAIL*.
3. Asserts the rendered HTML contains reply@example.org, not admin_email.

This pins finding (1).

17. Permission-check path isn't asserted

tests/unit-tests/email-preview.php

api_permissions_check() is annotated @codeCoverageIgnore, but a negative test (non-admin user → 403) round-trips the security contract. Two lines:

public function test_api_permissions_check_rejects_non_admin() {
    wp_set_current_user( self::factory()->user->create( [ 'role' => 'subscriber' ] ) );
    $result = Email_Preview::api_permissions_check();
    self::assertInstanceOf( 'WP_Error', $result );
}

18. 404 test should assert the HTTP status, not just the error code

tests/unit-tests/email-preview.php:133-143

self::assertEquals( 'newspack_email_preview_not_found', $response->get_error_code() );

Add:

self::assertEquals( 404, $response->get_error_data()['status'] );

19. Jest tests don't cover the race condition (finding 3) or the safety-timeout (finding 4)

src/wizards/newspack/views/settings/emails/email-preview.test.js

Specifically missing:

• postId changes mid-fetch: old fetch resolves after new fetch. Assert the displayed srcDoc reflects the new postId. Easiest setup: two mockImplementationOnce returning controlled promises; resolve them out of order.
• 8 s safety timeout: mock setTimeout, advance Jest's fake timers, assert is-ready flips even when no load event fires.
• iframe onError (after finding 6 is added).

20. simulateIframeLoad stubs contentDocument.querySelectorAll to always return []

src/wizards/newspack/views/settings/emails/email-preview.test.js:60

That means the Promise.all path always resolves with zero promises (synchronously). The real iframe will have an asynchronous-load story. The test never exercises the link/img load-wait path. Add a test that stubs querySelectorAll to return a fake link with no .sheet and an unresolved load event, then asserts the spinner stays visible until the load event fires.

21. renders loading state while fetching relies on Spinner's role=presentation

src/wizards/newspack/views/settings/emails/email-preview.test.js:74-81

expect( screen.getByRole( 'presentation' ) ).toBeTruthy();

This works because <svg role="presentation"> lives inside <Spinner>. If the Spinner implementation in @wordpress/components ever drops that role, the test breaks for the wrong reason. Prefer expect( document.querySelector( '.newspack-email-preview__placeholder' ) ).toBeInTheDocument().

---

CSS

22. Placeholder colour is a hardcoded hex

src/wizards/newspack/views/settings/emails/email-preview.scss:36

color: #949494;

Per Newspack's design conventions (see packages/colors/DEVELOPMENT.md), admin surfaces should use WordPress core colour tokens or the Newspack colour palette. Replace with:

@use "~@wordpress/base-styles/colors" as wp-colors;
// ...
color: wp-colors.$gray-400;

(Or the equivalent Newspack neutral. The existing emails.scss already imports wp-colors — same pattern.)

23. pointer-events: none on the iframe blocks user-initiated focus

src/wizards/newspack/views/settings/emails/email-preview.scss:20

Combined with tabIndex={ -1 } on the iframe element, this correctly makes the preview a non-interactive thumbnail. Good. No change.

24. transition: opacity 200ms on the iframe means the spinner can briefly overlap

src/wizards/newspack/views/settings/emails/email-preview.scss:22

The container shows the spinner while ! isReady, and the iframe is opacity: 0 until is-ready is added. During the 200ms transition both can be visible. If the spinner has a solid background-coloured placeholder, no flash; if it's transparent, the iframe shows through. Worth a manual smoke test on slow connections.

---

Repo / process

25. PR diff includes a 354-line README that's unrelated to this PR

includes/reader-activation/integrations/README.md

The README addition comes from commit dcb057df5 docs(integrations): add framework README (#4722), which is an unrelated merged PR. It shows in this diff because the base branch (nppd-945-unified-emails-newspack-slice) was cut before #4722 merged to trunk. When PR #4727 rebases on / merges into trunk, this file will fall out of the diff naturally.

Nothing to fix here, but flagging so the reviewer agent doesn't waste cycles auditing it. The base branch could be rebased onto current trunk to clean up, but rebasing a stacked-PR base mid-review is usually more disruptive than the noise it removes.

26. PR is stacked on PR #4727 — review order matters

baseRefName: nppd-945-unified-emails-newspack-slice

This PR's correctness depends on:

• EmailItem.post_id being a valid email post ID (PR feat(emails): unified email registry + DataViews list #4727's api_get_email_settings() contract).
• Emails::POST_TYPE posts existing for every row (also feat(emails): unified email registry + DataViews list #4727).

If PR #4727 merges with any changes to those contracts (e.g. the duplicate-fetch consolidation tracked as NPPD-1531), the preview component may need a touchup. Keep the stack tight and merge in order.

27. Preview component is not exposed via useWizardData store

src/wizards/newspack/views/settings/emails/email-preview.tsx:100

Same pattern as the slice 1 review (PR #4727 finding 5): apiFetch directly bypasses the @wordpress/data store. For 24 cards this is fine — the cardinality is bounded. But if the preview is ever reused in a wizard with paginated emails, the store would dedupe requests across cards with the same post_id. Note for future consolidation.

---

Verified / non-issues

• IntersectionObserver and ResizeObserver guards correctly fall back to "eager load" and "scale=1" (email-preview.tsx:48-51, 72-75). Addresses Copilot finding 3247899534.
• is_readable() guard before include (class-email-preview.php:78) addresses Copilot finding 3247899474.
• tabIndex={ -1 } plus pointer-events: none correctly takes the iframe out of focus/click reach for screen readers and pointer users — the row is the actionable unit.
• The is-ready opacity-fade contract is borrowed cleanly from NewsletterPreview (repos/newspack-newsletters/src/components/newsletter-preview/index.js:80-106). Pattern is sound.
• permission_callback uses manage_options — matches every other Newspack settings endpoint.
• POST_TYPE 404 check in api_get_preview() (class-email-preview.php:258) means non-email posts can't be probed for HTML content.

---

Suggested split for the agent picking this up

1. Fix-up commit (must-have): finding (1) function_exists bug + finding (16) test that pins it. Two-line code change plus a short test.
2. Security-comment commit (must-have): finding (2) — comment block above the sandbox attribute explaining the trade-off, optionally plus a CSP <meta> injected into the srcDoc.
3. Resilience commit: findings (3), (4), (5), (6), (7) — unify under a single cancelled flag in the fetch effect, clear the safety timeout, add onError, defend initial paint. About 30 lines.
4. PHP polish commit: findings (8), (9), (11), (12) — escape mailto, use wp_date, strtr instead of foreach str_replace, add the filter hook. About 15 lines.
5. Test fill-in commit: findings (17), (18), (19), (20), (21). About 50 lines of test code.
6. Follow-up tickets: (10) template-path hardening, (13) extract Emails::get_site_address, (27) data-store consolidation alongside NPPD-1531.

Findings (1), (2), and (3) are the ones most likely to bite real users. Everything else is polish or defensive.