Repository navigation
fix(react-email): keep a longhand padding of 0 on Button - #3793
Conversation
🦋 Changeset detectedLatest commit: 0706e11 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
@Abhishek-B-R is attempting to deploy a commit to the resend Team on Vercel. A member of the Team first needs to authorize it. |
|
@felipefreitag friendly bump: this makes parsePadding keep an explicit 0 longhand on Button (fixes #3792), so |
commit: |
parsePadding returned undefined for a longhand set to the number 0
because it used a truthiness check before converting to px. Button
spreads the original style first and then the parsed longhands, so an
undefined paddingTop is omitted and the padding shorthand wins: with
{ padding: '12px 20px', paddingTop: 0 } the button rendered 12px of top
padding in every client, while the Outlook spacer was computed as if it
were 0.
Only skip undefined and empty strings, so 0 is passed through.
06d0954 to
0706e11
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dielduarte
left a comment
dielduarte
left a comment
There was a problem hiding this comment.
thanks @Abhishek-B-R for the contribution! I did refactor the helper to reuse the existing helper instead. approving and merging!
Fixes #3792
Intent
Bug fix.
<Button style={{ padding: '12px 20px', paddingTop: 0 }}>rendered with 12px of top padding, because the explicit0was dropped and the shorthand took over.What was going on
parsePaddingturned each side into px with a truthiness check:so the number
0came back asundefined.Buttonspreads the user'sstyle(which still containspadding: '12px 20px') and then the parsed longhands on top. React omitsundefinedstyle values, so the rendered<a>hadwith no
padding-topat all. The Outlook spacer is computed from the parsed values with?? 0, so it already treated the top as 0, which means Outlook and every other client disagreed about the button height.paddingTop: '0px'was fine because the string is truthy.Changes
parsePaddingnow only skipsundefinedand the empty string, so0is converted like any other value. The empty string case keeps its existing behaviour (there is a test for it).Verification
Two tests, both written before the fix and both failing on canary:
parsePadding({ padding: '12px 20px', paddingTop: 0 })returnedpaddingTop: undefined, now0.padding-top:0;andpadding-bottom:0;.vitest runinpackages/react-email: 304 passed. The one failing test and two failing suites (cli/commands/testing/export*.spec.ts) fail the same way on a clean canary checkout in my environment, so they are unrelated.tsc --noEmitandbiome checkare clean.Added a patch changeset for
react-email.Summary by cubic
Fixes
<Button>dropping an explicit longhand padding of0, sostyle={{ padding: '12px 20px', paddingTop: 0 }}now renders with no top padding instead of falling back to the shorthand's12px.convertToPxpreviously returned0for any falsy input, so the number0became indistinguishable from unset values and React omitted it from the style, letting the shorthand win. It now only returnsundefinedforundefinedand empty strings, and passes0through. Tests cover the parsing and the rendered output, and a patch changeset forreact-emailis included.Written for commit 0706e11. Summary will update on new commits.