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/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 * 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..26c3329 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,22 @@ 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") { + // 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; + }; + // Map shorthand icons array → exporter format if (Array.isArray(icons)) { const mapped = icons.map((icon: SocialIcon) => ({ @@ -61,7 +87,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 +98,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..a24acb6 --- /dev/null +++ b/packages/react/src/dx-footguns.test.tsx @@ -0,0 +1,112 @@ +import { describe, it, expect } from "vitest"; +import React from "react"; +import { Email, Row, Column, ColumnLayouts, Button, Social, Image, Menu, renderToHtml, renderToJson } 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"); + }); + 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", () => { + 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+"/); + }); +}); + +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"); + }); +}); + +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/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.test.ts b/packages/shared/src/utils/semantic-props.test.ts index 169a18d..bf98ddc 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", () => { @@ -289,6 +315,32 @@ describe("normalizeLinkValue", () => { expect(normalizeLinkValue({ random: "thing" })).toBeUndefined(); 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" } }) + ).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", () => { diff --git a/packages/shared/src/utils/semantic-props.ts b/packages/shared/src/utils/semantic-props.ts index e25491a..51c706f 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 @@ -333,13 +351,24 @@ 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 && + ((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; + // `||` 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;