From 624c5a902960c42bd8b66f1d621e578dfaa403ca Mon Sep 17 00:00:00 2001 From: ramzi Date: Sat, 2 May 2026 14:35:50 +0100 Subject: [PATCH 1/2] fix(editor): block lossy visual conversion for custom HTML --- .../__tests__/EmailEditor/modeGuards.test.ts | 32 +++++++++++++ .../components/EmailEditor/EmailEditor.tsx | 47 +++++++------------ .../src/components/EmailEditor/modeGuards.ts | 27 +++++++++++ 3 files changed, 77 insertions(+), 29 deletions(-) create mode 100644 apps/web/__tests__/EmailEditor/modeGuards.test.ts create mode 100644 apps/web/src/components/EmailEditor/modeGuards.ts diff --git a/apps/web/__tests__/EmailEditor/modeGuards.test.ts b/apps/web/__tests__/EmailEditor/modeGuards.test.ts new file mode 100644 index 00000000..12573861 --- /dev/null +++ b/apps/web/__tests__/EmailEditor/modeGuards.test.ts @@ -0,0 +1,32 @@ +import {describe, expect, it} from 'vitest'; +import {getInitialEditorMode, getModeToggleDecision} from '../../src/components/EmailEditor/modeGuards'; + +describe('EmailEditor mode guards', () => { + it('starts in html mode for custom html templates', () => { + const customHtml = '
Hello
'; + + expect(getInitialEditorMode(customHtml)).toBe('html'); + }); + + it('allows switching simple html into visual mode', () => { + const simpleHtml = '

Hello world

'; + + expect( + getModeToggleDecision({ + currentMode: 'html', + htmlContent: simpleHtml, + }), + ).toEqual({action: 'switch', nextMode: 'visual'}); + }); + + it('blocks switching custom html into visual mode', () => { + const customHtml = '
Hello
'; + + expect( + getModeToggleDecision({ + currentMode: 'html', + htmlContent: customHtml, + }), + ).toEqual({action: 'block-custom-html'}); + }); +}); diff --git a/apps/web/src/components/EmailEditor/EmailEditor.tsx b/apps/web/src/components/EmailEditor/EmailEditor.tsx index 36dd2cf2..d6e5f19b 100644 --- a/apps/web/src/components/EmailEditor/EmailEditor.tsx +++ b/apps/web/src/components/EmailEditor/EmailEditor.tsx @@ -32,6 +32,7 @@ import { import {Code2, Eye, Monitor, Smartphone, Tablet, Upload, X} from 'lucide-react'; import {network} from '../../lib/network'; import {detectCustomHtmlPatterns, wrapEmailWithStyles} from '../../lib/emailStyles'; +import {getInitialEditorMode, getModeToggleDecision} from './modeGuards'; import 'tippy.js/dist/tippy.css'; interface EmailEditorProps { @@ -53,8 +54,7 @@ const commonVariables = [ ]; export function EmailEditor({value, onChange, placeholder, subject, from, replyTo}: EmailEditorProps) { - // Detect if initial value has custom HTML and start in appropriate mode - const initialMode = detectCustomHtmlPatterns(value) ? 'html' : 'visual'; + const initialMode = getInitialEditorMode(value); const [mode, setMode] = useState<'visual' | 'html'>(initialMode); const [htmlContent, setHtmlContent] = useState(value); @@ -151,41 +151,33 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT // eslint-disable-next-line react-hooks/exhaustive-deps }, [value, editor]); - // Use the same pattern detection as initialization (no editor manipulation) - const detectCustomHtml = (html: string): boolean => { - return detectCustomHtmlPatterns(html); - }; - const handleModeToggle = () => { - if (mode === 'visual') { - // Switching to HTML mode + const decision = getModeToggleDecision({ + currentMode: mode, + htmlContent, + }); + + if (decision.action === 'switch' && decision.nextMode === 'html') { const currentHtml = editor?.getHTML() || ''; setHtmlContent(currentHtml); setMode('html'); - } else { - // Switching to visual mode - check if custom HTML will be lost - if (detectCustomHtml(htmlContent)) { - setShowModeWarningDialog(true); - } else { - switchToVisualMode(); - } + return; + } + + if (decision.action === 'block-custom-html') { + setShowModeWarningDialog(true); + return; } - }; - const switchToVisualMode = () => { - // Only switch if we have an editor and html content if (editor) { editor.commands.setContent(htmlContent || ''); onChange(htmlContent); setMode('visual'); } - setShowModeWarningDialog(false); }; const stayInHtmlMode = () => { - // Explicitly stay in HTML mode and just close the dialog setShowModeWarningDialog(false); - // Ensure we're in HTML mode if (mode !== 'html') { setMode('html'); } @@ -701,8 +693,8 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT

- Your HTML contains custom formatting, styles, or elements that the visual editor doesn't support. - Switching to visual mode will cause these customizations to be lost or modified. + Your HTML contains custom formatting, styles, or elements that the visual editor doesn't support. To + preserve the original markup, this template must stay in HTML mode.

This may affect:

@@ -715,11 +707,8 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT
- -
diff --git a/apps/web/src/components/EmailEditor/modeGuards.ts b/apps/web/src/components/EmailEditor/modeGuards.ts new file mode 100644 index 00000000..df878379 --- /dev/null +++ b/apps/web/src/components/EmailEditor/modeGuards.ts @@ -0,0 +1,27 @@ +import {detectCustomHtmlPatterns} from '../../lib/emailStyles'; + +export type EmailEditorMode = 'visual' | 'html'; + +export type ModeToggleDecision = {action: 'switch'; nextMode: EmailEditorMode} | {action: 'block-custom-html'}; + +export const getInitialEditorMode = (value: string): EmailEditorMode => { + return detectCustomHtmlPatterns(value) ? 'html' : 'visual'; +}; + +export const getModeToggleDecision = ({ + currentMode, + htmlContent, +}: { + currentMode: EmailEditorMode; + htmlContent: string; +}): ModeToggleDecision => { + if (currentMode === 'visual') { + return {action: 'switch', nextMode: 'html'}; + } + + if (detectCustomHtmlPatterns(htmlContent)) { + return {action: 'block-custom-html'}; + } + + return {action: 'switch', nextMode: 'visual'}; +}; From 4b60159291752dfec41d3fd408878714df508427 Mon Sep 17 00:00:00 2001 From: ramzi Date: Mon, 25 May 2026 14:20:13 +0100 Subject: [PATCH 2/2] fix(editor): replace blocking dialog with reversible snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of blocking the HTML→Visual switch for custom HTML templates, allow the switch but snapshot the original HTML. A revert banner lets the user restore the original markup if the visual conversion dropped elements. This addresses the maintainer's concern about false positives in custom HTML detection while still protecting against accidental loss. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/EmailEditor/modeGuards.test.ts | 19 ++-- .../components/EmailEditor/EmailEditor.tsx | 86 ++++++++----------- .../src/components/EmailEditor/modeGuards.ts | 6 +- 3 files changed, 53 insertions(+), 58 deletions(-) diff --git a/apps/web/__tests__/EmailEditor/modeGuards.test.ts b/apps/web/__tests__/EmailEditor/modeGuards.test.ts index 12573861..e13bc29d 100644 --- a/apps/web/__tests__/EmailEditor/modeGuards.test.ts +++ b/apps/web/__tests__/EmailEditor/modeGuards.test.ts @@ -8,7 +8,7 @@ describe('EmailEditor mode guards', () => { expect(getInitialEditorMode(customHtml)).toBe('html'); }); - it('allows switching simple html into visual mode', () => { + it('allows switching simple html into visual mode without snapshot', () => { const simpleHtml = '

Hello world

'; expect( @@ -19,14 +19,23 @@ describe('EmailEditor mode guards', () => { ).toEqual({action: 'switch', nextMode: 'visual'}); }); - it('blocks switching custom html into visual mode', () => { + it('allows switching custom html into visual mode with snapshot for revert', () => { const customHtml = ''; + const decision = getModeToggleDecision({ + currentMode: 'html', + htmlContent: customHtml, + }); + + expect(decision).toEqual({action: 'switch', nextMode: 'visual', snapshot: customHtml}); + }); + + it('always allows switching from visual to html', () => { expect( getModeToggleDecision({ - currentMode: 'html', - htmlContent: customHtml, + currentMode: 'visual', + htmlContent: '

anything

', }), - ).toEqual({action: 'block-custom-html'}); + ).toEqual({action: 'switch', nextMode: 'html'}); }); }); diff --git a/apps/web/src/components/EmailEditor/EmailEditor.tsx b/apps/web/src/components/EmailEditor/EmailEditor.tsx index d6e5f19b..f7b8af99 100644 --- a/apps/web/src/components/EmailEditor/EmailEditor.tsx +++ b/apps/web/src/components/EmailEditor/EmailEditor.tsx @@ -29,7 +29,7 @@ import { SelectTrigger, SelectValue, } from '@plunk/ui'; -import {Code2, Eye, Monitor, Smartphone, Tablet, Upload, X} from 'lucide-react'; +import {Code2, Eye, Monitor, RotateCcw, Smartphone, Tablet, Upload, X} from 'lucide-react'; import {network} from '../../lib/network'; import {detectCustomHtmlPatterns, wrapEmailWithStyles} from '../../lib/emailStyles'; import {getInitialEditorMode, getModeToggleDecision} from './modeGuards'; @@ -60,7 +60,9 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT const [htmlContent, setHtmlContent] = useState(value); const [showVariableDialog, setShowVariableDialog] = useState(false); const [showImageDialog, setShowImageDialog] = useState(false); - const [showModeWarningDialog, setShowModeWarningDialog] = useState(false); + // WHY: snapshot stores the original HTML before a lossy visual-mode switch, + // so the user can revert if the conversion dropped markup + const [htmlSnapshot, setHtmlSnapshot] = useState(null); const [previewDevice, setPreviewDevice] = useState<'desktop' | 'tablet' | 'mobile'>('desktop'); const [imageUrl, setImageUrl] = useState(''); const [imageFile, setImageFile] = useState(null); @@ -157,30 +159,34 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT htmlContent, }); - if (decision.action === 'switch' && decision.nextMode === 'html') { + if (decision.nextMode === 'html') { const currentHtml = editor?.getHTML() || ''; setHtmlContent(currentHtml); + setHtmlSnapshot(null); setMode('html'); return; } - if (decision.action === 'block-custom-html') { - setShowModeWarningDialog(true); - return; - } - if (editor) { + if (decision.snapshot) { + setHtmlSnapshot(decision.snapshot); + } editor.commands.setContent(htmlContent || ''); onChange(htmlContent); setMode('visual'); } }; - const stayInHtmlMode = () => { - setShowModeWarningDialog(false); - if (mode !== 'html') { - setMode('html'); - } + const revertToSnapshot = () => { + if (!htmlSnapshot) return; + setHtmlContent(htmlSnapshot); + onChange(htmlSnapshot); + setHtmlSnapshot(null); + setMode('html'); + }; + + const dismissSnapshot = () => { + setHtmlSnapshot(null); }; const handleHtmlChange = (newHtml: string) => { @@ -377,6 +383,22 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT onInsertImage={() => setShowImageDialog(true)} canUploadImages={canUploadImages} /> + {htmlSnapshot && ( +
+

+ Some custom HTML may not display correctly in the visual editor. +

+
+ + +
+
+ )}
@@ -677,44 +699,6 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT - {/* Mode switch warning dialog */} - { - // Only allow closing (not opening) and ensure we stay in current mode - if (!open) { - setShowModeWarningDialog(false); - } - }} - > - - - Custom HTML Detected - -
-

- Your HTML contains custom formatting, styles, or elements that the visual editor doesn't support. To - preserve the original markup, this template must stay in HTML mode. -

-
-

This may affect:

-
    -
  • Custom HTML elements and attributes
    • -
  • Inline styles and CSS classes
    • -
  • Complex table structures
    • -
  • Custom formatting or layout
    • -
-
- -
- -
-
- - - {/* Image insertion dialog */} diff --git a/apps/web/src/components/EmailEditor/modeGuards.ts b/apps/web/src/components/EmailEditor/modeGuards.ts index df878379..ebaa7896 100644 --- a/apps/web/src/components/EmailEditor/modeGuards.ts +++ b/apps/web/src/components/EmailEditor/modeGuards.ts @@ -2,7 +2,7 @@ import {detectCustomHtmlPatterns} from '../../lib/emailStyles'; export type EmailEditorMode = 'visual' | 'html'; -export type ModeToggleDecision = {action: 'switch'; nextMode: EmailEditorMode} | {action: 'block-custom-html'}; +export type ModeToggleDecision = {action: 'switch'; nextMode: EmailEditorMode; snapshot?: string}; export const getInitialEditorMode = (value: string): EmailEditorMode => { return detectCustomHtmlPatterns(value) ? 'html' : 'visual'; @@ -19,8 +19,10 @@ export const getModeToggleDecision = ({ return {action: 'switch', nextMode: 'html'}; } + // WHY: custom HTML detection has false positives, so we allow the switch + // but snapshot the original HTML so the user can revert without data loss if (detectCustomHtmlPatterns(htmlContent)) { - return {action: 'block-custom-html'}; + return {action: 'switch', nextMode: 'visual', snapshot: htmlContent}; } return {action: 'switch', nextMode: 'visual'};