Skip to content

[EMAIL-PREVIEW-1] PLU-386: add TFieldPreviewType and previewType to IFieldRichText#1652

Open
ogp-weeloong wants to merge 3 commits into
develop-v2from
claude/email-preview-modal
Open

[EMAIL-PREVIEW-1] PLU-386: add TFieldPreviewType and previewType to IFieldRichText#1652
ogp-weeloong wants to merge 3 commits into
develop-v2from
claude/email-preview-modal

Conversation

@ogp-weeloong

@ogp-weeloong ogp-weeloong commented May 28, 2026

Copy link
Copy Markdown
Contributor

Problem

We want pipe owners to be able to preview their email across a variety of email clients (Outlook, Yahoo etc).

High-level Approach

We will use the emaillens library to generate the preview. This is a front-end only change; high level approach is

  1. Introduce the concept of "previewing" a rich text field by adding a previewType property in the IRichTextField type.
    1. When this is specified, we will:
      1. Add a new "Preview" button to the field itself (to preview live rich text content)
      2. Update the "View" button in the corresponding row of the "Previous Result" table (to view formatted dataOut for that run).
    2. When clicked, the front end substitutes variables in the rich text content with test / dataOut data, and loads the appropriate react component to render it
  2. Add a email preview type.
    1. This pops up a modal which supports choosing between several email clients; it calls into the emaillens library with the chosen client to generate the client's HTML

IMPORTANT

The emailens library is technically meant to be run on a node backend. However, I want this to run on the frontend to reduce latency and save costs.

It turns out you can run this on the browser as long as we don't need the features [see github] that use node stuff:

  1. @emailens/engine/server — imports dns (the checkDeliverability + SpamAssassin stuff).
  2. @emailens/engine/compile — JSX/MJML/Maizzle compilation, which uses node:vm / isolated-vm / mjml etc.

I added a vite safeguard to prevent us from from doing so. In the event that emailens updates the code to make this impossible, we can always add a REST API to do this (non-ideal)

This PR

  • Creates the previewType field
  • Creates the email value for preview type and also a new PreviewEmailModal for previewing emails in selected clients

Tests

Only regression tests as this is a no-op.

  • Check that I can still compose postman emails in the rich text editor
  • Check that I can still view email from past executions

Full test in later PR

ogp-weeloong commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

How to use the Graphite Merge Queue

Add the label lfg to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ogp-weeloong ogp-weeloong changed the title feat: add TFieldPreviewType and previewType to IFieldRichText [EMAIL-PREVIEW-1] PLU-386: add TFieldPreviewType and previewType to IFieldRichText May 28, 2026
@linear

linear Bot commented May 28, 2026

Copy link
Copy Markdown

PLU-386

ogp-weeloong and others added 2 commits May 29, 2026 10:40
Adds a narrow union type marker (`'email'` for now) plus an optional
`previewType` field on rich-text fields. This will let downstream code
opt a rich-text field into a kind-specific preview modal in
FlowStepTestController without churning the schema for future preview
kinds (e.g. SMS).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Installs @emailens/engine in the frontend workspace; the slice we use
(transformForClient) only needs cheerio + css-tree which are
browser-safe. The engine declares engines.node: ">=18", which is a
deliberate tech-debt acceptance.

To guard against future regressions where the engine — or any other
dep — starts importing a node-only module, add a Rollup onwarn handler
in both vite.config.ts and vite.config.lib.ts that promotes the
"has been externalized for browser compatibility" warning (and
MISSING_NODE_BUILTINS) into a hard build error. The comment + error
message name @emailens/engine explicitly so future readers understand
why this guardrail exists.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a self-contained, lazy-loaded modal that renders an HTML email
body through @emailens/engine's transformForClient for one of four
target clients (Outlook Classic, Gmail, Apple Mail, Yahoo Mail),
defaulting to Outlook Classic (Word engine — worst-case rendering).

- index.tsx: React.lazy wrapper + Suspense boundary so consumers don't
  pay the engine's bundle cost up front.
- EmailPreviewModal.tsx: real implementation. Chakra Flex (fixed-width
  left rail of client options, flex="1" iframe on the right). The
  iframe uses sandbox="" (no allow-* flags) to block scripts, forms,
  and navigation in user HTML.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ogp-weeloong ogp-weeloong marked this pull request as ready for review June 2, 2026 06:38
@ogp-weeloong ogp-weeloong requested a review from a team as a code owner June 2, 2026 06:38
@ogp-weeloong ogp-weeloong force-pushed the claude/email-preview-modal branch from ba98c73 to aa87e38 Compare June 2, 2026 06:58

@kevinkim-ogp kevinkim-ogp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

other than a few minor things, lgtm!
can still create and send email, can still view past emails

)

const transformed = useMemo(() => {
return transformForClient(html, selectedClientId).html

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be wrapped in a try/catch: if it so happens to throw an error due to malformed HTML, the preview might break?
also do we want to only transform when the isOpen is true?

Comment on lines +47 to +67
// Guardrail for the @emailens/engine tech-debt: the engine declares
// `engines.node: ">=18"` but the slice we use (transformForClient) is
// browser-safe. If a future version of @emailens/engine — or any other
// dep — starts importing a node-only module (fs, path, crypto, etc.),
// Vite externalizes it and emits a warning. We promote that warning to
// a hard error so CI catches the regression instead of silently
// shipping a broken bundle.
onwarn(warning, defaultHandler) {
const msg = warning.message ?? ''
if (
msg.includes('has been externalized for browser compatibility') ||
warning.code === 'MISSING_NODE_BUILTINS'
) {
throw new Error(
`[vite-guardrail] A node-only import leaked into the frontend bundle. ` +
`This guardrail exists to catch @emailens/engine (and similar libraries) ` +
`pulling in backend-only modules. Original warning: ${msg}`,
)
}
defaultHandler(warning)
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: do we want to extract this to a shared helper since its the same thing in both vite.config.lib.ts and vite.config.ts ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants