Fix/image objsrc width and dx footguns#34
Merged
Conversation
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) <noreply@anthropic.com>
…nt 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) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses several “type-checks but renders wrong” DX footguns across link normalization, Social sizing, and Image sizing/HTML output, and adds public type exports to make author-written helpers easier to annotate.
Changes:
- Extend link normalization to support
Href.attrs.href/targetand preserve custom attributes; add tests and cross-component regression coverage. - Improve Social DX by accepting px-string inputs for
iconSize/spacingand coercing them for exporter arithmetic; add footgun regression tests. - Adjust Image defaults and sizing intent resolution (including object-
src.widthparity with flatwidth) and update snapshots/tests accordingly.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/utils/semantic-props.ts | Link-value normalization now reads from attrs and preserves non-link attrs. |
| packages/shared/src/utils/semantic-props.test.ts | Adds unit tests for new attrs link behavior and edge cases. |
| packages/shared/src/layouts.ts | Updates JSDoc usage import path to the public React package name. |
| packages/react/src/index.ts | Re-exports additional “input building block” types from the public entry. |
| packages/react/src/dx-types.test-d.tsx | Adds a dts-only guard ensuring those input types remain publicly exported. |
| packages/react/src/dx-footguns.test.tsx | New integration-style tests covering prior silent-broken-output footguns. |
| packages/react/src/dx-behaviors.test.tsx | Updates expected Image sizing behavior in multi-column layout. |
| packages/react/src/components/Social.tsx | Accepts iconSize/spacing as SizeInput and coerces for exporter math. |
| packages/react/src/components/Image.width-roundtrip.test.tsx | Adds round-trip parity tests for object-src.width pinning behavior. |
| packages/react/src/components/Image.tsx | Drops placeholder height default and treats object-src.width as display intent. |
| packages/react/src/components/Image.test.tsx | Updates Image expectations to match new pinning semantics for object src. |
| packages/react/src/components/Button.tsx | JSDoc example updated to document href string ergonomics. |
| packages/react/src/components/snapshots/snapshots.test.tsx.snap | Snapshot updates for removed height attribute on responsive images. |
| packages/react/src/snapshots/golden-template.test.tsx.snap | Snapshot updates reflecting image height attribute omission changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ditor
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) <noreply@anthropic.com>
…ssthrough 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: <Html> renders verbatim, not sanitized — pass toSafeHtml via UnlayerProvider to sanitize like the editor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What does this PR do? Keep it brief.
Changes
Test Plan
pnpm buildpassespnpm testpassesNotes
Any additional context for reviewers.
📖 Storybook Preview: https://unlayer.github.io/elements/pr/34/