Repository navigation
fix(react-email): keep a longhand padding of 0 on Button - #3793
Open
Abhishek-B-R wants to merge 1 commit into
Open
Abhishek-B-R wants to merge 1 commit into
Abhishek-B-R wants to merge 1 commit into
Conversation
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.
馃 Changeset detectedLatest commit: 537ac04 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 |
Contributor
|
@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. |
This branch has not been deployed
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.
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.parsePaddingskipped falsy values, so0becameundefinedand React omitted it from the style, letting the shorthand win. It now only skipsundefinedand empty strings. Tests cover the parsing and the rendered output, and a patch changeset forreact-emailis included.Written for commit 537ac04. Summary will update on new commits.