From b3b8180e7ccb8d25d8ee75ad051b5778327c689f Mon Sep 17 00:00:00 2001 From: Ivo Date: Sun, 28 Jun 2026 19:11:46 +0200 Subject: [PATCH 1/5] Complete image width round-trip (object-src form) + fix DX footguns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Image round-trip: a fixed width now pins whether set via the flat `width` prop OR the object-src / documented `values` full-control form (`src={{ width: 300 }}`). Previously only the flat prop pinned; the object form serialized autoWidth:true and still resized to the original on click in the Builder. Both now emit the editor's canonical pin (autoWidth:false + percent of the column slot), verified against the editor's own imageRendering functions (holds across the on-click natural-dimension reload). An explicit `autoWidth` on `src` is still honored. DX footguns surfaced by cold-start agent testing (valid, type-checking input that produced broken output): - Button: `{ name, attrs: { href } }` (the shape the canonical Href type advertises) rendered href="" — normalizeLinkValue now reads href/target from `attrs` too, and `||` lets the schema's empty default href fall through. Genuine custom attrs are still spread. - Social: `iconSize`/`spacing` as px strings ("34px") rendered max-width:NaNpx — the exporter does arithmetic on them; relax the type to number|string and coerce to a number in the mapper. - Image: a string-url image inherited the placeholder's 1600x400 aspect and emitted a wrong height attr; drop the default height so it's height:auto. Polish: - Export the input building-block types (SizeInput, BorderInput, TextStyleProps, FontFamilyInput, FontWeightInput, HeadingLevel, ImageSrcInput) — referenced by public prop types but not importable (TS2459). - Fix a JSDoc import example (@unlayer-internal/shared-elements -> the public package) and add a Button href example. Tests: new dx-footguns + object-src round-trip cases; updated the two tests that encoded the old object-src=natural behavior; type guards for the exports; refreshed two snapshots (only the wrong height attr removed). 369 pass, typecheck clean, bundle 63.6KB < 68KB. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../golden-template.test.tsx.snap | 8 +-- packages/react/src/components/Button.tsx | 3 +- packages/react/src/components/Image.test.tsx | 11 +-- packages/react/src/components/Image.tsx | 35 ++++++---- .../components/Image.width-roundtrip.test.tsx | 34 +++++++++ packages/react/src/components/Social.tsx | 39 ++++++++--- .../__snapshots__/snapshots.test.tsx.snap | 2 +- packages/react/src/dx-behaviors.test.tsx | 10 +-- packages/react/src/dx-footguns.test.tsx | 69 +++++++++++++++++++ packages/react/src/dx-types.test-d.tsx | 20 ++++++ packages/react/src/index.ts | 9 +++ packages/shared/src/layouts.ts | 2 +- packages/shared/src/utils/semantic-props.ts | 20 ++++-- 13 files changed, 218 insertions(+), 44 deletions(-) create mode 100644 packages/react/src/dx-footguns.test.tsx diff --git a/packages/react/src/__snapshots__/golden-template.test.tsx.snap b/packages/react/src/__snapshots__/golden-template.test.tsx.snap index ca192c0..306834e 100644 --- a/packages/react/src/__snapshots__/golden-template.test.tsx.snap +++ b/packages/react/src/__snapshots__/golden-template.test.tsx.snap @@ -46,7 +46,7 @@ exports[`Golden Template: Marketing Email > snapshot locks the full email HTML 1 - Acme Co + Acme Co @@ -183,7 +183,7 @@ exports[`Golden Template: Marketing Email > snapshot locks the full email HTML 1 - Spring Collection + Spring Collection @@ -346,7 +346,7 @@ exports[`Golden Template: Marketing Email > snapshot locks the full email HTML 1 - Product A + Product A @@ -408,7 +408,7 @@ exports[`Golden Template: Marketing Email > snapshot locks the full email HTML 1 - Product B + Product B diff --git a/packages/react/src/components/Button.tsx b/packages/react/src/components/Button.tsx index 70cdfd6..7a825ed 100644 --- a/packages/react/src/components/Button.tsx +++ b/packages/react/src/components/Button.tsx @@ -44,7 +44,8 @@ const DEFAULT_VALUES = { * * @example Flat Props (Simple - most common) * ```tsx - * * ``` diff --git a/packages/react/src/components/Image.test.tsx b/packages/react/src/components/Image.test.tsx index eb2117d..788c2fc 100644 --- a/packages/react/src/components/Image.test.tsx +++ b/packages/react/src/components/Image.test.tsx @@ -129,13 +129,16 @@ describe("Image Component", () => { return (json as any).body.rows[0].columns[0].contents[0].values.src; } - it("a natural width (object src) stays responsive — not pinned", () => { + it("an object-src width pins as display intent (like the flat width prop)", () => { + // The documented full-control form `src={{ width }}` must pin and survive a + // round-trip, same as `width={…}` — not serialize autoWidth:true. const src = imageSrc( ); - expect(src.autoWidth).toBe(true); - expect(src.width).toBe(300); - expect(src.maxWidth).toBe("100%"); + expect(src.autoWidth).toBe(false); + expect(src.maxWidth).toBe("62.5%"); // 300 / (500 default − 20px slot) + expect(src.width).toBe(300); // natural/aspect fields preserved + expect(src.height).toBe(200); }); it("no width / string src stays responsive (autoWidth:true)", () => { diff --git a/packages/react/src/components/Image.tsx b/packages/react/src/components/Image.tsx index f2722d8..515283a 100644 --- a/packages/react/src/components/Image.tsx +++ b/packages/react/src/components/Image.tsx @@ -24,12 +24,18 @@ type ImageSemanticProps = Omit< export interface ImageProps extends ItemComponentProps {} -// Defaults from the editor schema, plus React-specific overrides +// Defaults from the editor schema, plus React-specific overrides. +// Drop the schema placeholder's `height` so an image with no explicit dimensions +// doesn't inherit its 4:1 aspect ratio: the email exporter then omits the `height` +// attribute (height:auto), letting the real image keep its own ratio instead of +// being letterboxed (Outlook honors the height attribute). `width` is kept — it +// drives the responsive display width attribute. Real dimensions, when provided +// via an object `src`, override these. +const { height: _placeholderHeight, ...defaultSrc } = ImageDefaults.src as Record; const DEFAULT_VALUES = { ...ImageDefaults, - // Override src with autoWidth/maxWidth for responsive rendering src: { - ...ImageDefaults.src, + ...defaultSrc, autoWidth: true, maxWidth: "100%", }, @@ -112,14 +118,17 @@ const Image = createItemComponent({ : { ...DEFAULT_VALUES.src }; const merged = { ...start, ...userSrc } as Record; - // In Unlayer's value model, `src.width`/`height` are the NATURAL image size - // and never set the display size. Display size = autoWidth + maxWidth: the - // default (and "100%") is responsive (autoWidth:true); a fixed display size - // is autoWidth:false + `maxWidth` as a PERCENT of the column's content slot - // (see image-sizing.ts for why a percent, not the natural-size field). A - // px/number pin is kept here as a placeholder and converted to that percent - // by the width-aware pass in renderToHtml / renderToJson, where the column - // geometry is known. An explicit autoWidth is honored. + // Display size = autoWidth + maxWidth: the default is responsive + // (autoWidth:true); a fixed display size is autoWidth:false + `maxWidth` as a + // PERCENT of the column's content slot (see image-sizing.ts for why a + // percent, not a raw px). A px/number pin is kept here as a placeholder and + // converted to that percent by the width-aware pass in renderToHtml / + // renderToJson, where the column geometry is known. + // + // A `width` the author provides — the flat `width` prop OR `src.width` + // (the documented full-control form) — is treated as that display intent and + // pins. This is the canonical pinned shape the editor stores, so it survives + // the round-trip; an `autoWidth` set explicitly on `src` is honored as-is. const pctRe = /^\d+(?:\.\d+)?%$/; const asPercent = (v: unknown): string | undefined => typeof v === "string" && pctRe.test(v.trim()) ? v.trim() : undefined; @@ -135,10 +144,10 @@ const Image = createItemComponent({ }; // Resolve display intent in priority order: flat `width` → flat `maxWidth` - // → escape-hatch values.src.maxWidth. + // → object src `maxWidth` → object src `width`. let displayPct: string | undefined; let displayPx: number | undefined; - for (const candidate of [widthProp, maxWidthProp, userSrc.maxWidth]) { + for (const candidate of [widthProp, maxWidthProp, userSrc.maxWidth, userSrc.width]) { if (candidate === undefined) continue; const pct = asPercent(candidate); if (pct) { diff --git a/packages/react/src/components/Image.width-roundtrip.test.tsx b/packages/react/src/components/Image.width-roundtrip.test.tsx index bec3f6c..ef9a7d1 100644 --- a/packages/react/src/components/Image.width-roundtrip.test.tsx +++ b/packages/react/src/components/Image.width-roundtrip.test.tsx @@ -280,3 +280,37 @@ describe("renderToJson row cells default to the Column count", () => { expect(src.maxWidth).toBe("42.86%"); }); }); + +describe("object-src width pins identically to the flat width prop (round-trip parity)", () => { + const inOneCol = (img: React.ReactElement) => ( + + + {img} + + + ); + + it("src={{ width }} pins (autoWidth:false + percent), not autoWidth:true", () => { + const src = imgSrc(inOneCol()); + expect(src.autoWidth).toBe(false); + expect(src.maxWidth).toBe("51.72%"); + }); + + it("the documented values={{ src: { width } }} full-control form pins too", () => { + const src = imgSrc(inOneCol()); + expect(src.autoWidth).toBe(false); + expect(src.maxWidth).toBe("51.72%"); + }); + + it("src width + height pins and keeps the height for aspect", () => { + const src = imgSrc(inOneCol()); + expect(src.autoWidth).toBe(false); + expect(src.maxWidth).toBe("51.72%"); + expect(src.height).toBe(200); + }); + + it("an explicit autoWidth on src is honored (escape hatch stays responsive)", () => { + const src = imgSrc(inOneCol()); + expect(src.autoWidth).toBe(true); + }); +}); diff --git a/packages/react/src/components/Social.tsx b/packages/react/src/components/Social.tsx index 074b5ba..04f2c43 100644 --- a/packages/react/src/components/Social.tsx +++ b/packages/react/src/components/Social.tsx @@ -1,18 +1,28 @@ import { SocialExporters, SocialDefaults } from "@unlayer/exporters"; -import type { SocialValues, SocialIcon } from "../types"; +import type { SocialValues, SocialIcon, SizeInput } from "../types"; import { createItemComponent, type ItemComponentProps } from "../utils/create-component"; import { mapSemanticProps, type SemanticProps } from "../utils/semantic-props"; -type SocialSemanticProps = SemanticProps & { +// `iconSize`/`spacing` are pixel counts the exporter does arithmetic on, so they +// must reach it as numbers. Accept a number or a px string for DX and coerce in +// the mapper — a raw "34px" would otherwise render `max-width:NaNpx`. +type SocialSemanticProps = Omit, "iconSize" | "spacing"> & { /** Social icons shorthand (array of {name, url}) */ icons?: SocialIcon[]; /** Icon shape */ iconType?: "circle" | "rounded" | "squared"; + /** Icon size in px — a number (34) or px string ("34px"). */ + iconSize?: SizeInput; + /** Gap between icons in px — a number or px string. */ + spacing?: SizeInput; }; -export interface SocialProps extends Omit>, "icons"> { +export interface SocialProps + extends Omit>, "icons" | "iconSize" | "spacing"> { icons?: SocialIcon[] | SocialValues["icons"]; iconType?: "circle" | "rounded" | "squared"; + iconSize?: SizeInput; + spacing?: SizeInput; } // Defaults from the editor schema @@ -46,6 +56,19 @@ const Social = createItemComponent({ propMapper: (props) => { const { icons, iconType, ...rest } = props; + // The exporter does arithmetic on iconSize/spacing (icon box sizing and the + // container max-width), so coerce a px string or number down to a number. + const coerceSizes = (base: Partial): Partial => { + for (const key of ["iconSize", "spacing"] as const) { + const v = (base as Record)[key]; + if (typeof v === "string") { + const n = parseFloat(v); + if (!Number.isNaN(n)) (base as Record)[key] = n; + } + } + return base; + }; + // Map shorthand icons array → exporter format if (Array.isArray(icons)) { const mapped = icons.map((icon: SocialIcon) => ({ @@ -61,7 +84,7 @@ const Social = createItemComponent({ iconType: iconType ?? base.icons?.iconType ?? "circle", icons: mapped, }; - return base; + return coerceSizes(base); } // If iconType passed without shorthand icons, thread it into nested group @@ -72,13 +95,11 @@ const Social = createItemComponent({ "Social" ); base.icons = { ...DEFAULT_ICONS, ...base.icons, iconType }; - return base; + return coerceSizes(base); } - return mapSemanticProps( - props as SemanticProps, - DEFAULT_VALUES, - "Social" + return coerceSizes( + mapSemanticProps(props as SemanticProps, DEFAULT_VALUES, "Social") ); }, displayName: "Social", diff --git a/packages/react/src/components/__snapshots__/snapshots.test.tsx.snap b/packages/react/src/components/__snapshots__/snapshots.test.tsx.snap index 7673922..549e02d 100644 --- a/packages/react/src/components/__snapshots__/snapshots.test.tsx.snap +++ b/packages/react/src/components/__snapshots__/snapshots.test.tsx.snap @@ -274,7 +274,7 @@ exports[`Render Snapshots > Image > email 1`] = ` - + diff --git a/packages/react/src/dx-behaviors.test.tsx b/packages/react/src/dx-behaviors.test.tsx index d74a962..08f2548 100644 --- a/packages/react/src/dx-behaviors.test.tsx +++ b/packages/react/src/dx-behaviors.test.tsx @@ -93,10 +93,10 @@ describe("DX: image sizing (a fixed width pins to the editor's canonical percent expect(src.maxWidth).toBe("62.5%"); }); - // Guards the column-overflow regression: a dimensioned image (object src.width - // = natural size) inside a multi-column row must stay responsive, never forced - // to its natural width — which would overflow the narrow column. - it("a dimensioned image stays responsive inside a multi-column row", () => { + // Guards the column-overflow regression: an object-src width inside a narrow + // multi-column row pins (display intent) but clamps to 100% — so it fills the + // column responsively via a percent, never a fixed px that overflows. + it("a dimensioned image in a multi-column row pins but clamps to 100% (no overflow)", () => { const json: any = renderToJson( @@ -107,7 +107,7 @@ describe("DX: image sizing (a fixed width pins to the editor's canonical percent ); const src = json.body.rows[0].columns[0].contents[0].values.src; - expect(src.autoWidth).toBe(true); + expect(src.autoWidth).toBe(false); expect(src.maxWidth).toBe("100%"); expect(src.width).toBe(400); }); diff --git a/packages/react/src/dx-footguns.test.tsx b/packages/react/src/dx-footguns.test.tsx new file mode 100644 index 0000000..67a333a --- /dev/null +++ b/packages/react/src/dx-footguns.test.tsx @@ -0,0 +1,69 @@ +import { describe, it, expect } from "vitest"; +import React from "react"; +import { Email, Row, Column, ColumnLayouts, Button, Social, Image, renderToHtml } from "./index"; + +// Guards for "valid, type-checking input silently produced broken output" footguns. + +const wrap = (el: React.ReactElement) => + renderToHtml( + + + {el} + + + ); +const firstHref = (html: string) => (html.match(/]*\bhref="([^"]*)"/i) || [, null])[1]; + +describe("Button href shapes all resolve (attrs is not dropped)", () => { + it("plain string href", () => { + expect(firstHref(wrap())).toBe("https://x.com/s"); + }); + it("{ name, values: { href } } (canonical storage shape)", () => { + expect( + firstHref(wrap()) + ).toBe("https://x.com/v"); + }); + it("{ name, attrs: { href } } — the type advertises attrs, so it must render", () => { + expect( + firstHref(wrap()) + ).toBe("https://x.com/a"); + }); + it("genuine custom attrs are still spread onto the anchor", () => { + const html = wrap( + + ); + expect(firstHref(html)).toBe("https://x.com/v"); + expect(html).toContain('class="cta"'); + }); +}); + +describe("Social iconSize/spacing accept px strings without NaN", () => { + const social = (props: any) => + wrap(); + + it("a px-string iconSize coerces to a number (no NaNpx)", () => { + const html = social({ iconSize: "34px" }); + expect(html).not.toContain("NaN"); + expect(html).toContain("34px"); // the icon box uses the coerced 34 + }); + it("a px-string spacing does not produce NaN", () => { + expect(social({ spacing: "8px" })).not.toContain("NaN"); + }); + it("a numeric iconSize still works", () => { + expect(social({ iconSize: 40 })).not.toContain("NaN"); + }); +}); + +describe("Image with no explicit dimensions does not force a wrong aspect", () => { + it("a string-url image emits no height attribute (height:auto keeps its real ratio)", () => { + const html = wrap(); + const img = (html.match(/]*>/i) || [""])[0]; + expect(img).toMatch(/width="\d+"/); // responsive width attr is present + expect(img).not.toMatch(/height="\d+"/); // but no guessed height + }); + it("an object src with real dimensions keeps its height", () => { + const html = wrap(); + const img = (html.match(/]*>/i) || [""])[0]; + expect(img).toMatch(/height="\d+"/); + }); +}); diff --git a/packages/react/src/dx-types.test-d.tsx b/packages/react/src/dx-types.test-d.tsx index 1383da0..45f1c29 100644 --- a/packages/react/src/dx-types.test-d.tsx +++ b/packages/react/src/dx-types.test-d.tsx @@ -80,3 +80,23 @@ export const _image_percent: ImageProps["maxWidth"] = "50%"; // @ts-expect-error fontWeight does not accept arbitrary words export const _fontWeight_reject: HeadingProps["fontWeight"] = "heavy"; + +// ── input building blocks must stay exported from the public entry ─────────── +// Agents factor shared helpers and want to annotate them; these were referenced +// by the public prop types but not exported (TS2459) until this guard. +import type { + SizeInput, + BorderInput as PublicBorderInput, + TextStyleProps, + FontFamilyInput, + FontWeightInput, + HeadingLevel, + ImageSrcInput, +} from "./index"; +export const _pub_size: SizeInput = "50%"; +export const _pub_border: PublicBorderInput = { borderBottomWidth: "1px" }; +export const _pub_textStyle: TextStyleProps = { fontSize: 16, color: "#222" }; +export const _pub_fontFamily: FontFamilyInput = "Georgia"; +export const _pub_fontWeight: FontWeightInput = 700; +export const _pub_headingLevel: HeadingLevel = "h2"; +export const _pub_imageSrc: ImageSrcInput = "https://x/p.png"; diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 6dfe7e6..fcb43f4 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -54,6 +54,15 @@ export type { // Shorthand types SocialIcon, MenuItem, + // Agent-friendly input building blocks — exported so authors can annotate + // their own factored-out helpers/consts (e.g. a shared border or text style). + SizeInput, + BorderInput, + TextStyleProps, + FontFamilyInput, + FontWeightInput, + HeadingLevel, + ImageSrcInput, } from "./types"; // Design JSON types (re-export from shared) diff --git a/packages/shared/src/layouts.ts b/packages/shared/src/layouts.ts index 0504d1c..23cf303 100644 --- a/packages/shared/src/layouts.ts +++ b/packages/shared/src/layouts.ts @@ -5,7 +5,7 @@ * configurations that match exactly what the Unlayer editor supports. * * Usage: - * import { ColumnLayouts } from '@unlayer-internal/shared-elements'; + * import { ColumnLayouts } from '@unlayer/react-elements'; * * ... * ... diff --git a/packages/shared/src/utils/semantic-props.ts b/packages/shared/src/utils/semantic-props.ts index e25491a..58e47be 100644 --- a/packages/shared/src/utils/semantic-props.ts +++ b/packages/shared/src/utils/semantic-props.ts @@ -333,13 +333,21 @@ export function normalizeLinkValue(value: unknown): Record | undefi const v = value as Record; // Already render-shape (has url) — pass through. if ("url" in v) return v; - // Storage shape from the schema: { name, values: { href, target } }. - if ("name" in v && v.values && typeof v.values === "object") { - const inner = v.values as Record; + // Storage shape from the schema: { name, values: { href, target }, attrs? }. + // The canonical Href type also surfaces `attrs: { href, target }`, so authors + // reasonably put the link there — read href/target from `values` first, then + // fall back to `attrs`, and spread any remaining `attrs` as custom attributes. + // (Without this, `{ name, attrs: { href } }` type-checks but renders href="".) + if ("name" in v) { + const inner = v.values && typeof v.values === "object" ? (v.values as Record) : {}; + const attrs = v.attrs && typeof v.attrs === "object" ? (v.attrs as Record) : {}; + const { href: attrsHref, target: attrsTarget, ...customAttrs } = attrs; + // `||` not `??`: the schema default merges in `values: { href: "" }`, so an + // empty `values.href` must fall through to the `attrs` href, not win over it. return { - url: inner.href ?? "", - target: inner.target ?? "_blank", - ...(v.attrs ?? {}), + url: inner.href || attrsHref || "", + target: inner.target || attrsTarget || "_blank", + ...customAttrs, }; } return undefined; From 08e67c40bcebd7b25212d2f904140f74de52e9ef Mon Sep 17 00:00:00 2001 From: Ivo Date: Sun, 28 Jun 2026 19:16:51 +0200 Subject: [PATCH 2/5] Add tests: normalizeLinkValue attrs/empty-fallthrough + cross-component link consistency - shared: normalizeLinkValue reads href from attrs, an empty values.href falls through to attrs (the || vs ?? fix), custom attrs preserved. - react: the attrs-href fix works for Image action + Menu, not just Button. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/react/src/dx-footguns.test.tsx | 22 ++++++++++++++++++- .../shared/src/utils/semantic-props.test.ts | 20 +++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/packages/react/src/dx-footguns.test.tsx b/packages/react/src/dx-footguns.test.tsx index 67a333a..ed359fc 100644 --- a/packages/react/src/dx-footguns.test.tsx +++ b/packages/react/src/dx-footguns.test.tsx @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest"; import React from "react"; -import { Email, Row, Column, ColumnLayouts, Button, Social, Image, renderToHtml } from "./index"; +import { Email, Row, Column, ColumnLayouts, Button, Social, Image, Menu, renderToHtml } from "./index"; // Guards for "valid, type-checking input silently produced broken output" footguns. @@ -67,3 +67,23 @@ describe("Image with no explicit dimensions does not force a wrong aspect", () = expect(img).toMatch(/height="\d+"/); }); }); + +describe("the attrs-href fix is consistent across all link-bearing components", () => { + const allHrefs = (html: string) => [...html.matchAll(/]*\bhref="([^"]*)"/gi)].map((m) => m[1]); + + it("Image action { name, attrs: { href } } wraps the image in a working anchor", () => { + const html = wrap( + + ); + expect(allHrefs(html)).toContain("https://x.com/ia"); + }); + + it("existing link forms still resolve (Menu items, Image action string) — no regression", () => { + const menu = wrap( + + ); + expect(allHrefs(menu)).toContain("https://x.com/m1"); + const img = wrap(); + expect(allHrefs(img)).toContain("https://x.com/is"); + }); +}); diff --git a/packages/shared/src/utils/semantic-props.test.ts b/packages/shared/src/utils/semantic-props.test.ts index 169a18d..dc2de62 100644 --- a/packages/shared/src/utils/semantic-props.test.ts +++ b/packages/shared/src/utils/semantic-props.test.ts @@ -289,6 +289,26 @@ describe("normalizeLinkValue", () => { expect(normalizeLinkValue({ random: "thing" })).toBeUndefined(); expect(normalizeLinkValue(42)).toBeUndefined(); }); + + it("reads href/target from `attrs` (the alias the canonical Href type exposes)", () => { + expect( + normalizeLinkValue({ name: "web", attrs: { href: "https://x.com/a", target: "_self" } }) + ).toEqual({ url: "https://x.com/a", target: "_self" }); + }); + + it("an empty `values.href` (the schema default) falls through to `attrs.href`", () => { + // mergeValues merges the schema default `values: { href: "" }` in, so an empty + // values.href must NOT win over an attrs href — guards the `||` vs `??` fix. + expect( + normalizeLinkValue({ name: "web", values: { href: "" }, attrs: { href: "https://x.com/a" } }) + ).toEqual({ url: "https://x.com/a", target: "_blank" }); + }); + + it("keeps genuine custom attrs (non href/target) on the render value", () => { + expect( + normalizeLinkValue({ name: "web", values: { href: "https://x.com/v" }, attrs: { class: "cta", "data-id": "7" } }) + ).toEqual({ url: "https://x.com/v", target: "_blank", class: "cta", "data-id": "7" }); + }); }); describe("normalizeValuesForExporter", () => { From d6a498ddcf2327c4b8807b8c8afcea8cd0e27c43 Mon Sep 17 00:00:00 2001 From: Ivo Date: Sun, 28 Jun 2026 19:30:37 +0200 Subject: [PATCH 3/5] Canonicalize attrs href into values.href so it round-trips into the editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The render-time fix made { name, attrs:{ href } } render a working anchor, but renderToJson preserved the storage shape (empty values.href + attrs), and the Builder reads values.href — so the link was lost on import. Move an attrs href/target into values.href/target at the mapper (both flat prop and values escape hatch), keeping genuine custom attrs. Found while loading a test design into the live editor. +4 tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/react/src/dx-footguns.test.tsx | 20 +++++++++- .../shared/src/utils/semantic-props.test.ts | 28 ++++++++++++- packages/shared/src/utils/semantic-props.ts | 40 ++++++++++++++----- 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/packages/react/src/dx-footguns.test.tsx b/packages/react/src/dx-footguns.test.tsx index ed359fc..9de2231 100644 --- a/packages/react/src/dx-footguns.test.tsx +++ b/packages/react/src/dx-footguns.test.tsx @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest"; import React from "react"; -import { Email, Row, Column, ColumnLayouts, Button, Social, Image, Menu, renderToHtml } from "./index"; +import { Email, Row, Column, ColumnLayouts, Button, Social, Image, Menu, renderToHtml, renderToJson } from "./index"; // Guards for "valid, type-checking input silently produced broken output" footguns. @@ -87,3 +87,21 @@ describe("the attrs-href fix is consistent across all link-bearing components", expect(allHrefs(img)).toContain("https://x.com/is"); }); }); + +describe("attrs href round-trips into the editor (renderToJson stores values.href)", () => { + const buttonHref = (el: React.ReactElement) => + (renderToJson( + {el} + ) as any).body.rows[0].columns[0].contents[0].values.href; + + it("a Button attrs href is stored in values.href (where the Builder reads it)", () => { + const href = buttonHref(); + expect(href.values.href).toBe("https://x.com/cta"); + expect(href.attrs).toBeUndefined(); + }); + + it("a string href is stored canonically too", () => { + const href = buttonHref(); + expect(href.values.href).toBe("https://x.com/s"); + }); +}); diff --git a/packages/shared/src/utils/semantic-props.test.ts b/packages/shared/src/utils/semantic-props.test.ts index dc2de62..8c69717 100644 --- a/packages/shared/src/utils/semantic-props.test.ts +++ b/packages/shared/src/utils/semantic-props.test.ts @@ -212,7 +212,7 @@ describe("mapSemanticProps", () => { }); }); - it("passes object href through unchanged", () => { + it("passes a canonical object href ({name, values}) through unchanged", () => { const hrefObj = { name: "email", values: { href: "mailto:test@test.com" } }; const result = mapSemanticProps( { href: hrefObj }, @@ -221,6 +221,32 @@ describe("mapSemanticProps", () => { ); expect(result.href).toEqual(hrefObj); }); + + it("canonicalizes an attrs href into values.href so the editor reads it", () => { + // The link must land in `values.href` (where the Builder reads it), not be + // left in `attrs` with an empty values.href — otherwise renderToJson → + // editor loses the link even though renderToHtml renders it. + const result = mapSemanticProps( + { href: { name: "web", attrs: { href: "https://x.com/cta", target: "_self" } } }, + BUTTON_DEFAULTS, + "Button" + ); + expect((result.href as any).values).toEqual({ + href: "https://x.com/cta", + target: "_self", + }); + expect((result.href as any).attrs).toBeUndefined(); + }); + + it("keeps genuine custom attrs while lifting the href into values", () => { + const result = mapSemanticProps( + { href: { name: "web", values: { href: "https://x.com/v" }, attrs: { class: "cta" } } }, + BUTTON_DEFAULTS, + "Button" + ); + expect((result.href as any).values.href).toBe("https://x.com/v"); + expect((result.href as any).attrs).toEqual({ class: "cta" }); + }); }); describe("edge cases", () => { diff --git a/packages/shared/src/utils/semantic-props.ts b/packages/shared/src/utils/semantic-props.ts index 58e47be..979e443 100644 --- a/packages/shared/src/utils/semantic-props.ts +++ b/packages/shared/src/utils/semantic-props.ts @@ -202,19 +202,37 @@ export function mapSemanticProps>( delete result.html; } - // String shorthand for link/action fields (`href="https://..."` or - // `action="https://..."`) → wrap into the schema's storage shape so that - // the rest of the pipeline (mergeValues, renderToJson) sees a consistent - // type. The render-time exporter handoff later resolves this into the - // exporter's `{ url, target }` shape via `normalizeValuesForExporter`. - for (const key of ["href", "action"] as const) { - const v = userProps[key]; + // Canonicalize link/action fields to the schema's storage shape so the rest of + // the pipeline (mergeValues, renderToJson, AND the editor) sees a consistent + // type: + // - `"https://…"` string shorthand → `{ name:"web", values:{ href, target } }`. + // - a link placed in `attrs` (the canonical Href type also surfaces + // `attrs:{ href, target }`) is moved into `values.href`/`target`, where the + // editor reads it — so the link round-trips into the Builder, not just into + // renderToHtml. Genuine custom attrs (class, data-*, …) are preserved. + // Applied to both the flat prop and the `values` escape hatch. (Render-time + // `normalizeLinkValue` also reads `attrs` as a defensive fallback.) + const canonicalizeLink = (v: any): any => { if (typeof v === "string") { - userProps[key] = { - name: "web", - values: { href: v, target: "_blank" }, - }; + return { name: "web", values: { href: v, target: "_blank" } }; + } + if (v && typeof v === "object" && "name" in v && v.attrs && typeof v.attrs === "object") { + const { href: attrsHref, target: attrsTarget, ...customAttrs } = v.attrs; + if (attrsHref !== undefined || attrsTarget !== undefined) { + const linkValues = { ...(v.values as Record | undefined) }; + if (!linkValues.href && attrsHref !== undefined) linkValues.href = attrsHref; + if (!linkValues.target && attrsTarget !== undefined) linkValues.target = attrsTarget; + const next: Record = { ...v, values: linkValues }; + if (Object.keys(customAttrs).length) next.attrs = customAttrs; + else delete next.attrs; + return next; + } } + return v; + }; + for (const key of ["href", "action"] as const) { + if (userProps[key] !== undefined) userProps[key] = canonicalizeLink(userProps[key]); + if (result[key] !== undefined) result[key] = canonicalizeLink(result[key]); } // Normalize CSS-idiom prop values to the shapes the exporters expect. Authors From c2f4544313e1a7ce0452c960ac9ed876bf671675 Mon Sep 17 00:00:00 2001 From: Ivo Date: Sun, 28 Jun 2026 19:47:23 +0200 Subject: [PATCH 4/5] Fix CallToAction story (malformed SVG data URI) + note Html is raw passthrough MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The story embedded an inline-SVG data URI with double quotes (xmlns="…") inside a double-quoted style="…", so the first inner quote closed the attribute and '); opacity: 0.1; "> leaked as visible text. Replaced the grain with a valid CSS radial-gradient dot pattern and dropped the onmouseover/out inline JS (can't run in a rendered email; XSS pattern) from the affected stories. Added a guard test over all Html story HTML (no inline handlers, no url() with a raw double quote) and a security note: renders verbatim, not sanitized — pass toSafeHtml via UnlayerProvider to sanitize like the editor. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/components/Html.stories.test.tsx | 32 +++++++++++++++++++ .../react/src/components/Html.stories.tsx | 11 ++++--- packages/react/src/components/Html.tsx | 7 ++++ 3 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 packages/react/src/components/Html.stories.test.tsx diff --git a/packages/react/src/components/Html.stories.test.tsx b/packages/react/src/components/Html.stories.test.tsx new file mode 100644 index 0000000..2abe199 --- /dev/null +++ b/packages/react/src/components/Html.stories.test.tsx @@ -0,0 +1,32 @@ +import { describe, it, expect } from "vitest"; +import * as stories from "./Html.stories"; + +// The component is a raw passthrough (no default sanitizer), so its story +// HTML must itself be valid and safe. Two things have bitten us: +// - inline event handlers (onmouseover, …) — never run in a rendered email and +// are an XSS pattern; bad examples to ship. +// - a `url('…')` whose content has a raw `"` (e.g. an SVG data URI with +// xmlns="…") sits inside a double-quoted style="…", so the first inner `"` +// closes the attribute and the tail (`'); opacity: …">`) leaks as visible +// text. URL-encode the data URI or avoid raw double quotes. +const htmlStories = Object.entries(stories).filter( + ([, s]) => s && typeof s === "object" && (s as any).args && typeof (s as any).args.html === "string" +) as [string, { args: { html: string } }][]; + +describe("Html stories ship valid, safe HTML", () => { + it("there are several html stories to check", () => { + expect(htmlStories.length).toBeGreaterThan(3); + }); + + for (const [name, story] of htmlStories) { + const html = story.args.html; + + it(`${name}: has no inline event handlers`, () => { + expect(html).not.toMatch(/\son[a-z]+\s*=/i); + }); + + it(`${name}: has no url() containing a raw double quote (style break-out)`, () => { + expect(html).not.toMatch(/url\(\s*'[^']*"[^']*'\s*\)/); + }); + } +}); diff --git a/packages/react/src/components/Html.stories.tsx b/packages/react/src/components/Html.stories.tsx index 992173a..dee6409 100644 --- a/packages/react/src/components/Html.stories.tsx +++ b/packages/react/src/components/Html.stories.tsx @@ -88,7 +88,7 @@ export const StyledCard: Story = { font-weight: 600; cursor: pointer; transition: all 0.3s ease; - " onmouseover="this.style.background='rgba(255,255,255,0.3)'" onmouseout="this.style.background='rgba(255,255,255,0.2)'"> + "> Learn More @@ -172,8 +172,9 @@ export const CallToAction: Story = { left: 0; right: 0; bottom: 0; - background: url('data:image/svg+xml,'); - opacity: 0.1; + background-image: radial-gradient(rgba(255, 255, 255, 0.18) 1px, transparent 1px); + background-size: 18px 18px; + opacity: 0.5; ">
@@ -213,7 +214,7 @@ export const CallToAction: Story = { cursor: pointer; transition: transform 0.2s ease; box-shadow: 0 4px 14px rgba(59, 130, 246, 0.4); - " onmouseover="this.style.transform='translateY(-2px)'" onmouseout="this.style.transform='translateY(0)'"> + "> Start Free Trial @@ -227,7 +228,7 @@ export const CallToAction: Story = { font-size: 16px; cursor: pointer; transition: all 0.2s ease; - " onmouseover="this.style.borderColor='rgba(255,255,255,0.6)'; this.style.background='rgba(255,255,255,0.1)'" onmouseout="this.style.borderColor='rgba(255,255,255,0.3)'; this.style.background='transparent'"> + "> Learn More
diff --git a/packages/react/src/components/Html.tsx b/packages/react/src/components/Html.tsx index 854d335..46e4bcc 100644 --- a/packages/react/src/components/Html.tsx +++ b/packages/react/src/components/Html.tsx @@ -13,6 +13,13 @@ const DEFAULT_VALUES = { /** * Html - Universal SSR/Client Component for custom HTML with Automatic Semantic Props * + * ⚠️ Renders the HTML verbatim — it is NOT sanitized by default. Only pass HTML + * you trust, and make sure it is valid: notably, an inline SVG inside a `url(...)` + * must be URL-encoded, because a raw `"` inside a double-quoted `style="…"` + * closes the attribute and the rest leaks out as text. To sanitize (matching the + * editor's HTML block, which strips scripts/event handlers), pass a `toSafeHtml` + * function via the `UnlayerProvider` config. + * * @example Flat Props * ```tsx * From 163f979aeb17e8011d51eb473a03859bf80cd0d5 Mon Sep 17 00:00:00 2001 From: Ivo Date: Sun, 28 Jun 2026 19:47:23 +0200 Subject: [PATCH 5/5] Review: tighten normalizeLinkValue guard + strict Social size coercion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - normalizeLinkValue: only normalize a {name} object when it actually carries values or attrs, so a bare {name} (or accidental {name:…}) falls through to undefined per the documented contract instead of becoming {url:""}. - Social coerceSizes: parse iconSize/spacing strictly (number or px string); drop a non-px unit ("50%", "1.5em") so it falls back to the schema default rather than being silently parseFloat-ed to a wrong px count. Both caught in review. +2 tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/react/src/components/Social.tsx | 7 +++++-- packages/react/src/dx-footguns.test.tsx | 5 +++++ packages/shared/src/utils/semantic-props.test.ts | 6 ++++++ packages/shared/src/utils/semantic-props.ts | 5 ++++- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/react/src/components/Social.tsx b/packages/react/src/components/Social.tsx index 04f2c43..26c3329 100644 --- a/packages/react/src/components/Social.tsx +++ b/packages/react/src/components/Social.tsx @@ -62,8 +62,11 @@ const Social = createItemComponent({ for (const key of ["iconSize", "spacing"] as const) { const v = (base as Record)[key]; if (typeof v === "string") { - const n = parseFloat(v); - if (!Number.isNaN(n)) (base as Record)[key] = n; + // px count only — a non-px unit ("50%", "1.5em") is invalid here, so + // drop it and let mergeValues fall back to the schema default. + const m = /^(\d+(?:\.\d+)?)(?:px)?$/.exec(v.trim()); + if (m) (base as Record)[key] = parseFloat(m[1]); + else delete (base as Record)[key]; } } return base; diff --git a/packages/react/src/dx-footguns.test.tsx b/packages/react/src/dx-footguns.test.tsx index 9de2231..a24acb6 100644 --- a/packages/react/src/dx-footguns.test.tsx +++ b/packages/react/src/dx-footguns.test.tsx @@ -52,6 +52,11 @@ describe("Social iconSize/spacing accept px strings without NaN", () => { it("a numeric iconSize still works", () => { expect(social({ iconSize: 40 })).not.toContain("NaN"); }); + it("a non-px iconSize (\"50%\") is dropped → falls back to the default, no NaN", () => { + const html = social({ iconSize: "50%" }); + expect(html).not.toContain("NaN"); + expect(html).toContain("32px"); // schema default, not a coerced 50 + }); }); describe("Image with no explicit dimensions does not force a wrong aspect", () => { diff --git a/packages/shared/src/utils/semantic-props.test.ts b/packages/shared/src/utils/semantic-props.test.ts index 8c69717..bf98ddc 100644 --- a/packages/shared/src/utils/semantic-props.test.ts +++ b/packages/shared/src/utils/semantic-props.test.ts @@ -316,6 +316,12 @@ describe("normalizeLinkValue", () => { expect(normalizeLinkValue(42)).toBeUndefined(); }); + it("returns undefined for a bare {name} with no values/attrs (not a link)", () => { + // Honor the contract: only the actual storage variants (values or attrs) + // normalize; a name-only object falls through so the caller keeps it. + expect(normalizeLinkValue({ name: "web" })).toBeUndefined(); + }); + it("reads href/target from `attrs` (the alias the canonical Href type exposes)", () => { expect( normalizeLinkValue({ name: "web", attrs: { href: "https://x.com/a", target: "_self" } }) diff --git a/packages/shared/src/utils/semantic-props.ts b/packages/shared/src/utils/semantic-props.ts index 979e443..51c706f 100644 --- a/packages/shared/src/utils/semantic-props.ts +++ b/packages/shared/src/utils/semantic-props.ts @@ -356,7 +356,10 @@ export function normalizeLinkValue(value: unknown): Record | undefi // reasonably put the link there — read href/target from `values` first, then // fall back to `attrs`, and spread any remaining `attrs` as custom attributes. // (Without this, `{ name, attrs: { href } }` type-checks but renders href="".) - if ("name" in v) { + if ( + "name" in v && + ((v.values && typeof v.values === "object") || (v.attrs && typeof v.attrs === "object")) + ) { const inner = v.values && typeof v.values === "object" ? (v.values as Record) : {}; const attrs = v.attrs && typeof v.attrs === "object" ? (v.attrs as Record) : {}; const { href: attrsHref, target: attrsTarget, ...customAttrs } = attrs;