Skip to content

Pin fixed-width images so they survive the design-JSON round-trip#33

Merged
ivoIturrieta merged 6 commits into
mainfrom
fix/image-fixed-width-roundtrip
Jun 28, 2026
Merged

Pin fixed-width images so they survive the design-JSON round-trip#33
ivoIturrieta merged 6 commits into
mainfrom
fix/image-fixed-width-roundtrip

Conversation

@ivoIturrieta

@ivoIturrieta ivoIturrieta commented Jun 28, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Collaborator

A fixed image width (px or number) was stored in the natural-size field with autoWidth:true, so re-opening the exported design in an editor reloaded the image's intrinsic dimensions and the explicit width was lost (the image snapped back to its original size on selection).

Treat a fixed px/number width as display intent: emit autoWidth:false with maxWidth as a percent of the column's content slot — the canonical fixed-size shape, kept independent of the natural src.width/height. The percent is computed from the same available-width geometry the renderers use (contentWidth x column share, minus paddings/borders) by a width-aware pass in both renderToHtml (via Column's threaded context) and renderToJson (via the tree walk). A percent width/maxWidth already pinned and is unchanged; a no-width image stays responsive (autoWidth:true).

  • add utils/image-sizing.ts (slot geometry + px->percent conversion)
  • Image propMapper: capture width/maxWidth as display intent, no longer polluting the natural src.width field
  • new Image.width-roundtrip tests; update stale assertions that encoded the old natural-size behavior

Verified the emitted percent matches the renderer's own available-width math and that the pin no longer jumps when intrinsic dimensions refresh.

Summary

What does this PR do? Keep it brief.

Changes

Test Plan

  • pnpm build passes
  • pnpm test passes
  • Tested in Storybook (if UI change)
  • Added/updated tests for new behavior

Notes

Any additional context for reviewers.

📖 Storybook Preview: https://unlayer.github.io/elements/pr/33/

@ivoIturrieta @claude
Pin fixed-width images so they survive the design-JSON round-trip
7fb49ba 
A fixed image width (px or number) was stored in the natural-size field
with autoWidth:true, so re-opening the exported design in an editor
reloaded the image's intrinsic dimensions and the explicit width was lost
(the image snapped back to its original size on selection).

Treat a fixed px/number width as display intent: emit autoWidth:false with
maxWidth as a percent of the column's content slot — the canonical
fixed-size shape, kept independent of the natural src.width/height. The
percent is computed from the same available-width geometry the renderers
use (contentWidth x column share, minus paddings/borders) by a width-aware
pass in both renderToHtml (via Column's threaded context) and renderToJson
(via the tree walk). A percent width/maxWidth already pinned and is
unchanged; a no-width image stays responsive (autoWidth:true).

- add utils/image-sizing.ts (slot geometry + px->percent conversion)
- Image propMapper: capture width/maxWidth as display intent, no longer
  polluting the natural src.width field
- new Image.width-roundtrip tests; update stale assertions that encoded
  the old natural-size behavior

Verified the emitted percent matches the renderer's own available-width
math and that the pin no longer jumps when intrinsic dimensions refresh.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 28, 2026 13:41
Copilot AI reviewed Jun 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a design-JSON round-trip issue where fixed-width images could lose their intended display size when reloaded in an editor, by encoding fixed widths in Unlayer’s canonical autoWidth:false + percent maxWidth form computed from the real column content slot geometry.

Changes:

  • Adds shared slot-geometry utilities and a px/number → percent conversion for pinned image widths.
  • Threads layout context through renderToJson and applies the pin conversion during JSON emission (and during HTML export via the item render pipeline).
  • Updates/introduces tests to lock in the fixed-width round-trip behavior and prevent regression.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/react/src/utils/render-to-json.ts Threads layout context through the JSON tree-walk and converts pinned image widths to canonical percent maxWidth.
packages/react/src/utils/image-sizing.ts New utilities for computing content-slot width and converting px pins to percent maxWidth.
packages/react/src/utils/create-component.tsx Applies the same pin conversion during export when layout geometry is available.
packages/react/src/dx-behaviors.test.tsx Updates DX behavior assertions to reflect new pinned-width semantics.
packages/react/src/components/Image.width-roundtrip.test.tsx Adds regression tests covering JSON round-trip + HTML rendering for pinned widths.
packages/react/src/components/Image.tsx Adjusts prop mapping so width/maxWidth represent display intent (not natural dimensions) and avoids polluting src.width.
packages/react/src/components/Image.test.tsx Updates expectations for pinned widths now emitted as percent maxWidth.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/react/src/utils/image-sizing.ts Outdated
ivoIturrieta and others added 2 commits June 28, 2026 15:51
@ivoIturrieta @claude
ci: raise ESM bundle budget 60KB->68KB for image width-pinning
90fcd77 
The bundle was already at 58.4KB on main (97% of the 60KB budget set when it
was ~49KB). The image width-pinning fix adds ~4.6KB of dependency-free local
geometry, so the budget no longer fits legitimate growth. Raise to 68KB; it
still flags accidental dependency bundling (any real dep is 10KB+).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ivoIturrieta @claude
Treat a bare numeric-string contentWidth as fixed px in slot geometry
5e1ab07 
fixedContentWidth only accepted numbers and px strings, but the renderer
(Row's toContentWidthPx parseInts any string) and the exporter's body-width
math treat a bare numeric string like "600" as 600px. The slot geometry
fell back to 500, producing a wrong pinned-image percent for that input.
Accept a numeric string with an optional px unit; still reject "%"/"auto".

Caught in review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ivoIturrieta ivoIturrieta requested a review from Copilot June 28, 2026 14:01
Copilot AI reviewed Jun 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread packages/react/src/utils/image-sizing.ts Outdated
Comment thread packages/react/src/utils/image-sizing.ts Outdated
@ivoIturrieta @claude
Make toPx strict + correct fixedContentWidth doc
ea651d4 
toPx() parseFloat'd any string, so a non-px maxWidth on a pinned src (e.g.
the escape hatch {autoWidth:false, maxWidth:'1.5em'}) was misread as px and
converted into a bogus percent. Accept only a number or numeric/px string;
leave other CSS units untouched. Add a test guarding it.

Also correct fixedContentWidth's doc: it mirrors the exporter's body-width
math (bare numeric string = px, %/auto -> fallback), not Row's parseInt
(which would misread '50%' as 50). Both caught in review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ivoIturrieta ivoIturrieta requested a review from Copilot June 28, 2026 14:08
Copilot AI reviewed Jun 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread packages/react/src/utils/image-sizing.ts Outdated
Comment thread packages/react/src/utils/render-to-json.ts
@ivoIturrieta @claude
Share one strict contentWidth->px parse; default row cells to Column …
0620ea6 
…count

- contentWidth parsing: Row's grid CSS and the image slot geometry had
  separate parsers that disagreed on non-px values (Row's parseInt read
  "50%" as 50px; the slot math fell back to 500). Extract one shared
  bodyContentWidthPx and use it in both, so a non-px contentWidth collapses
  to the same base everywhere. No change for px/number widths; also fixes a
  latent email-grid bug for % content widths.

- renderToJson default cells: counted all children, so a stray non-Column
  child inflated the cells array beyond the column list and distorted the
  column-share math (wrong pinned-image percent) and the row layout. Count
  only <Column> children, matching the existing comment.

Both caught in review. +4 tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ivoIturrieta ivoIturrieta requested a review from Copilot June 28, 2026 14:22
Copilot AI reviewed Jun 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread packages/react/src/utils/image-sizing.ts
@ivoIturrieta @claude
Keep geometry parsers on parseFloat to mirror the renderer
7cc9cc0 
The slot geometry must match the renderer's available-width math, and the
editor's explodePaddingsOrMargins / explodeBorder both parseFloat each token
(so '10%' is read as 10). edges() already did this; switch borderEdges() back
from strict toPx to parseFloat so the two are consistent and both mirror the
renderer. Strict px parsing (toPx) stays only for the display-pin value in
pinImageSrc, never for the geometry. Documented the rationale inline.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ivoIturrieta ivoIturrieta requested a review from Copilot June 28, 2026 15:28
Copilot AI reviewed Jun 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

@ivoIturrieta ivoIturrieta merged commit 47c61d9 into main Jun 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ivoIturrieta