Skip to content

fix(heading): keep unit on margin values like mx="2rem"#3588

Open
yashs33244 wants to merge 1 commit into
resend:canaryfrom
yashs33244:fix/heading-margin-unit
Open

fix(heading): keep unit on margin values like mx="2rem"#3588
yashs33244 wants to merge 1 commit into
resend:canaryfrom
yashs33244:fix/heading-margin-unit

Conversation

@yashs33244

@yashs33244 yashs33244 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What

Heading accepts m/mx/my/mt/mr/mb/ml as number | string, but withSpace appended px to any value whose leading number parsed. So a string that already carries a unit produced invalid CSS:

<Heading mx="2rem"></Heading>   // -> margin-left:2rempx; margin-right:2rempx
<Heading my="10%"></Heading>    // -> margin-top:10%px; margin-bottom:10%px

parseFloat("2rem") is 2 (not NaN), so the existing guard let it through and ${value}px mangled it.

Fix

Suffix px only when the value is a bare number (or a unitless numeric string). Strings that already have a unit are passed through untouched. Numbers and unitless strings like mx={4} / mx="20" are unchanged. Added a regression test in utils.spec.ts.


Summary by cubic

Fixes Heading margin props to keep units (e.g., mx="2rem", my="10%") and only add "px" for bare numbers, preventing invalid CSS. Unitless numbers and numeric strings still get "px".

  • Bug Fixes
    • Updated withSpace to detect unit-bearing strings and pass them through unchanged.
    • Added regression tests for rem, %, and em (including negative values) in utils.spec.ts for react-email.

Written for commit b86d9ff. Summary will update on new commits.

Review in cubic

withSpace appended px to any value whose leading number parsed, so a
string with a unit (mx="2rem", my="10%") became invalid CSS like
"2rempx". Suffix px only for bare numbers; pass unit values through.
Copilot AI review requested due to automatic review settings June 23, 2026 06:31
@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@yashs33244 is attempting to deploy a commit to the resend Team on Vercel.

A member of the Team first needs to authorize it.

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@changeset-bot

changeset-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b86d9ff

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
react-email Patch
@react-email/editor Patch
@react-email/ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Auto-approved: Fixes a CSS unit handling bug in Heading margin props, adding a simple regex check to pass through strings with units. The change is small, well-tested, and does not affect core logic or other components.

Re-trigger cubic

@github-actions github-actions Bot added the linear-synced PR has been synced to Linear label Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

linear-synced PR has been synced to Linear

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants