diff --git a/CLAUDE.md b/CLAUDE.md index 16698f69..ff6ef4ba 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,8 +29,8 @@ cd packages/react && pnpm storybook # Storybook dev server | `@unlayer-internal/shared-elements` | `packages/shared` | No (private, bundled into react) | Framework-agnostic types, config, utils | | `@unlayer/elements-demo` | `packages/demo` | No | Demo/showcase app | | `@unlayer/from-react-email` | `packages/from-react-email` | No (private, bundled into migrate; API at `@unlayer/migrate/react-email`) | React Email → Elements converter (codemod, runtime with merge tags, check against the original); fidelity benchmark in `bench/`, results in `FIDELITY.md` | -| `@unlayer/migrate` | `packages/migrate` | Yes (npm) | React Email converter API at `@unlayer/migrate/react-email`; `npx @unlayer/migrate` CLI: migrate React Email templates, check them, write a report; `compare` for templates migrated by hand | -| `@unlayer/convert-core` | `packages/convert-core` | No (private, bundled into the converters) | Elements tree, conversion report, TSX printer and content check that converters share | +| `@unlayer/migrate` | `packages/migrate` | Yes (npm) | React Email converter API at `@unlayer/migrate/react-email`; `npx @unlayer/migrate` CLI: migrate React Email, MJML (`--from mjml`) and Beefree (`--from beefree`) templates, check them against the originals, write a report; `compare` for templates migrated by hand. The MJML and Beefree converters (`src/from-mjml`, `src/from-beefree`) build a design that `@unlayer/react-elements/codegen` writes as code; `src/design-source.ts` runs both (MJML's compile or Beefree's HTML export is what they're checked against). Measure them with `measure:mjml` and `scripts/measure-beefree.ts` | +| `@unlayer/convert-core` | `packages/convert-core` | No (private, bundled into the converters) | Elements tree, conversion report, TSX printer and content check that converters share; declared differences (`declared.ts`: content a converter knows it changes, such as a Social icon's alt text, which the check expects and lists) | ### Component Hierarchy (strict) diff --git a/packages/convert-core/src/cascade.ts b/packages/convert-core/src/cascade.ts index c367d969..d6d77416 100644 --- a/packages/convert-core/src/cascade.ts +++ b/packages/convert-core/src/cascade.ts @@ -2114,7 +2114,8 @@ function compareArrays(a: number[], b: number[]): number { export interface StyleDifference { /** * A style; "shown": text that is only the inbox preview on one side and in the email on the other, or that can't - * be made out on one side (its color on what's behind it: white on white); "line": a line break moved. + * be made out on one side (its color on what's behind it: white on white, "invisible"; or too faint where the other + * side's is clear, "hard to read"); "line": a line break moved. */ property: (typeof STYLE_PROPERTIES)[number] | "shown" | "line"; original: string; @@ -2227,6 +2228,11 @@ function luminance([r, g, b]: Rgba): number { const INVISIBLE = 1.2; /** At or above this, text can be read, if faintly (#cccccc on white is 1.6). */ const READABLE = 1.5; +/** + * At or above this, text reads clearly (WCAG's least for large text; #949494 on white is 3): such text taken under + * READABLE (#f7dd72 on white is 1.4) is hard to read, not a color alone. + */ +const CLEAR = 3; /** * How text in a style stands out from what's behind it: the lowest WCAG contrast ratio over the colors it may be @@ -2307,11 +2313,13 @@ export function compareStyles(originalHtml: string | StyledDocument, convertedHt } if (!a[i].shown || !b[i].shown) continue; compared++; - // Text a reader can't make out on one side only (white on white: a background lost): not the same words read. + // Text a reader can't make out on one side only (white on white: a background lost), or clear text made too faint + // to read (a black link in pale yellow): not the same words read. const [ca, cb] = [contrast(a[i].style), contrast(b[i].style)]; - if (ca !== undefined && cb !== undefined && ((ca >= READABLE && cb < INVISIBLE) || (ca < INVISIBLE && cb >= READABLE))) { - const original = ca < INVISIBLE ? `invisible (${onBackground(a[i].style)})` : "shown"; - const converted = cb < INVISIBLE ? `invisible (${onBackground(b[i].style)})` : "shown"; + const unread = (low: number, high: number) => (low < INVISIBLE && high >= READABLE) || (low < READABLE && high >= CLEAR); + if (ca !== undefined && cb !== undefined && (unread(ca, cb) || unread(cb, ca))) { + const shown = (c: number, style: WordStyle) => (c < INVISIBLE ? `invisible (${onBackground(style)})` : c < READABLE ? `hard to read (${onBackground(style)})` : "shown"); + const [original, converted] = [shown(ca, a[i].style), shown(cb, b[i].style)]; const key = `shown\u0000${original}\u0000${converted}`; const group = groups.get(key) ?? { property: "shown" as const, original, converted, words: [] }; group.words.push(a[i].word); diff --git a/packages/convert-core/src/declared.ts b/packages/convert-core/src/declared.ts new file mode 100644 index 00000000..b6a31df2 --- /dev/null +++ b/packages/convert-core/src/declared.ts @@ -0,0 +1,111 @@ +/** + * Declared differences: content a converter knows it changes, said up front so the check expects it instead of + * failing on it. Each declaration names exactly what it replaces (this image's alt text, this glyph in this element), + * so it can't hide anything else: the original's HTML gets the declared changes, then the check compares as usual, and + * every change made is reported. A declaration that matches nothing changes nothing. + */ + +import { parse, type DefaultTreeAdapterMap } from "parse5"; + +type Node = DefaultTreeAdapterMap["node"]; +type Element = DefaultTreeAdapterMap["element"]; +type TextNode = DefaultTreeAdapterMap["textNode"]; + +export type DeclaredDifference = + /** On images with exactly this `src` and exactly this alt text (`from`, "" for none), the migration writes `to`. */ + | { kind: "alt"; src: string; from: string; to: string; reason: string } + /** The migration shows this image (`to`) where the original shows `from`, exactly: another picture of the same thing. */ + | { kind: "src"; from: string; to: string; reason: string } + /** The migration leaves out text that is exactly `text` (trimmed) inside an element with the class `parentClass`. */ + | { kind: "text"; text: string; parentClass: string; reason: string } + /** + * The migration shows, at every width, the elements with the class `className` that the original hides at some + * (a menu folded into a hamburger button on phones): they're shown in the original too (`display: block`). + */ + | { kind: "shown"; className: string; reason: string } + /** The migration leaves out the elements with the class `className` (a hamburger button): hidden in the original too. */ + | { kind: "hidden"; className: string; reason: string }; + +export interface AppliedDifference { + declaration: DeclaredDifference; + /** How many places in the original it changed. */ + count: number; +} + +const attribute = (element: Element, name: string): string | undefined => element.attrs.find((a) => a.name === name)?.value; +const escapeAttribute = (value: string) => value.replace(/&/g, "&").replace(/"/g, """).replace(/ (element && "attrs" in element ? attribute(element, "class") ?? "" : "").split(/\s+/).filter(Boolean); + +/** + * The original's HTML as the migration is expected to show it, with what each declaration changed. Only the attribute + * values and text the declarations name are rewritten; the rest of the document is kept byte for byte. + */ +export function applyDeclared(html: string, declared: readonly DeclaredDifference[]): { html: string; applied: AppliedDifference[] } { + if (!declared.length) return { html, applied: [] }; + const document = parse(html, { sourceCodeLocationInfo: true }); + const edits: Array<{ start: number; end: number; text: string }> = []; + const counts = declared.map(() => 0); + + const replaceAttribute = (element: Element, name: string, value: string) => { + const location = element.sourceCodeLocation; + const at = location?.attrs?.[name]; + if (at) edits.push({ start: at.startOffset, end: at.endOffset, text: `${name}="${escapeAttribute(value)}"` }); + else if (location?.startTag) { + // ` { + if ("tagName" in node && node.tagName === "img") { + const src = attribute(node, "src") ?? ""; + const alt = attribute(node, "alt") ?? ""; + // One declaration per attribute of an image: the first that names it. + const altAt = declared.findIndex((d) => d.kind === "alt" && d.src === src && d.from === alt); + if (altAt >= 0 && replaceAttribute(node, "alt", (declared[altAt] as { to: string }).to)) counts[altAt]++; + const srcAt = declared.findIndex((d) => d.kind === "src" && d.from === src); + if (srcAt >= 0 && replaceAttribute(node, "src", (declared[srcAt] as { to: string }).to)) counts[srcAt]++; + } + if ("tagName" in node) { + const at = declared.findIndex((d) => (d.kind === "shown" || d.kind === "hidden") && classes(node).includes(d.className)); + if (at >= 0) { + const style = attribute(node, "style"); + const display = `${style ? `${style.replace(/;?\s*$/, ";")}` : ""}display:${declared[at].kind === "shown" ? "block" : "none"} !important`; + if (replaceAttribute(node, "style", display)) counts[at]++; + } + } + if (node.nodeName === "#text") { + const text = node as TextNode; + const at = declared.findIndex((d) => d.kind === "text" && text.value.trim() === d.text && classes(text.parentNode as Element).includes(d.parentClass)); + const location = text.sourceCodeLocation; + if (at >= 0 && location) { + edits.push({ start: location.startOffset, end: location.endOffset, text: "" }); + counts[at]++; + } + } + for (const child of "childNodes" in node ? node.childNodes : []) visit(child); + if ("content" in node && node.content) visit(node.content as unknown as Node); + }; + visit(document as unknown as Node); + + let out = html; + let last = Infinity; + for (const edit of edits.sort((a, b) => b.start - a.start)) { + if (edit.end > last) continue; // overlapping: the first one kept + out = out.slice(0, edit.start) + edit.text + out.slice(edit.end); + last = edit.start; + } + return { html: out, applied: declared.map((declaration, i) => ({ declaration, count: counts[i] })).filter((a) => a.count > 0) }; +} + +/** A declared difference as a report lists it: its reason, and what it changed. */ +export function describeDeclared(applied: AppliedDifference): { reason: string; detail: string } { + const { declaration: d, count } = applied; + const times = count > 1 ? ` (×${count})` : ""; + if (d.kind === "alt") return { reason: d.reason, detail: `alt "${d.from}" → "${d.to}"${times}` }; + if (d.kind === "src") return { reason: d.reason, detail: `${d.from} → ${d.to}${times}` }; + if (d.kind === "shown" || d.kind === "hidden") return { reason: d.reason, detail: `.${d.className}${times}` }; + return { reason: d.reason, detail: `"${d.text}"${times}` }; +} diff --git a/packages/convert-core/src/index.ts b/packages/convert-core/src/index.ts index cf920a75..95d38bb3 100644 --- a/packages/convert-core/src/index.ts +++ b/packages/convert-core/src/index.ts @@ -24,3 +24,4 @@ export { compareStyles, DESKTOP_WIDTH, normalizeWord, StyledDocument, type Style export { decodeHtmlEntities } from "./entities"; export { autoTableWidths, htmlText, unbreakableWords, wordWidth, type TableColumn } from "./text-width"; export { editorFonts, shareEditorFonts, type EditorFont } from "./fonts"; +export { applyDeclared, describeDeclared, type AppliedDifference, type DeclaredDifference } from "./declared"; diff --git a/packages/convert-core/test/declared.test.ts b/packages/convert-core/test/declared.test.ts new file mode 100644 index 00000000..71458f69 --- /dev/null +++ b/packages/convert-core/test/declared.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; +import { applyDeclared, describeDeclared, type DeclaredDifference } from "../src/declared"; +import { compareText } from "../src/verify"; + +const ICON = "https://cdn.example.com/icons/facebook.png"; +const ALT: DeclaredDifference = { kind: "alt", src: ICON, from: "Facebook", to: "Facebook icon", reason: "social icon alt text is the editor's" }; +const GLYPH: DeclaredDifference = { kind: "text", text: "☰", parentClass: "menu-open", reason: "the menu toggle's glyph is left out" }; + +const page = (body: string) => `t${body}`; +const icon = (alt: string, src = ICON) => `

Follow us

${alt}`; + +describe("declared differences", () => { + it("make the original show what the migration is expected to: the check passes, and each change is reported", () => { + const original = page(icon("Facebook")); + const migrated = page(icon("Facebook icon")); + expect(compareText(original, migrated).missingAttributes.length).toBeGreaterThan(0); + const expected = applyDeclared(original, [ALT]); + const check = compareText(expected.html, migrated); + expect([check.missing, check.added, check.missingAttributes, check.addedAttributes]).toEqual([[], [], [], []]); + expect(expected.applied.map(describeDeclared)).toEqual([{ reason: "social icon alt text is the editor's", detail: 'alt "Facebook" → "Facebook icon"' }]); + }); + + it("change only what they name: another alt change, another image, or other text still fails as content", () => { + // The alt text the migration writes isn't the declared one. + const wrongAlt = compareText(applyDeclared(page(icon("Facebook")), [ALT]).html, page(icon("Facebook logo"))); + expect(wrongAlt.missingAttributes).toContain("alt Facebook icon"); + expect(wrongAlt.addedAttributes).toContain("alt Facebook logo"); + // The original's alt isn't the one declared: nothing changes, and the difference stays. + const otherFrom = applyDeclared(page(icon("FB")), [ALT]); + expect(otherFrom.applied).toEqual([]); + expect(compareText(otherFrom.html, page(icon("Facebook icon"))).missingAttributes).toContain("alt FB"); + // Another image with the same alt text isn't touched. + const otherImage = applyDeclared(page(icon("Facebook", "https://cdn.example.com/photo.png")), [ALT]); + expect(otherImage.applied).toEqual([]); + // A glyph outside the element it's declared in is content. + const loose = applyDeclared(page('

Menu ☰

☰'), [GLYPH]); + expect(loose.applied).toEqual([{ declaration: GLYPH, count: 1 }]); + expect(compareText(loose.html, page("

Menu

")).missing).toEqual(["☰"]); + }); + + it("leave out a declared glyph, and keep the rest of the document byte for byte", () => { + const original = page('
Shop
'); + const { html, applied } = applyDeclared(original, [GLYPH]); + expect(html).toBe(original.replace("☰", "")); + expect(applied).toHaveLength(1); + expect(compareText(html, page('
Shop
')).missing).toEqual([]); + }); + + it("swap a declared image source, and add an alt text the original's image lacks", () => { + const swapped = applyDeclared(page(''), [ + { kind: "src", from: "/mjml/facebook.png", to: "/unlayer/facebook.png", reason: "r" }, + { kind: "alt", src: "/mjml/facebook.png", from: "", to: "Facebook icon", reason: "r" }, + ]); + expect(swapped.html).toBe(page('Facebook icon')); + expect(swapped.applied.map((a) => a.count)).toEqual([1, 1]); + }); + + it("show elements of a declared class the original hides at some widths (a menu folded into a hamburger)", () => { + const css = ""; + const original = `${css}
Shop
`; + const migrated = page('
Shop
'); + expect(compareText(original, migrated).phone.added).toEqual(["Shop"]); + const { html, applied } = applyDeclared(original, [{ kind: "shown", className: "menu-links", reason: "r" }]); + expect(html).toContain('
'); + expect(applied).toHaveLength(1); + expect(compareText(html, migrated).phone.added).toEqual([]); + }); + + it("hide elements of a declared class (a hamburger button and its icon)", () => { + const original = page('Shop'); + const migrated = page('Shop'); + expect(compareText(original, migrated).missingAttributes.length).toBeGreaterThan(0); + const { html, applied } = applyDeclared(original, [{ kind: "hidden", className: "menu-trigger", reason: "r" }]); + expect(html).toContain('