Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow getShorthandAttrValue to parse borders #2729

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

garrettjohnson
Copy link
Contributor

@garrettjohnson garrettjohnson commented Aug 18, 2023

Fixes #2688

This fixes the column component rendering style="width:NaNInfinity;" by improving getShorthandAttrValue to use the borderParser for border and inner-border properties.

@iRyusa
Copy link
Member

iRyusa commented Aug 23, 2023

We have a dedicated getShorthandBorderValue for that can you use it for the fix instead ?

Thanks 👍

@garrettjohnson
Copy link
Contributor Author

@iRyusa I would need to modify getShorthandBorderValue to be able to parse a property (inner-border in this case) in addition to a direction https://github.com/mjmlio/mjml/blob/master/packages/mjml-core/src/createComponent.js#L131-L137

Are you ok with a breaking change and modifying the getShorthandBorderValue function to accept two parameters? Or would you want a function that breaks convention, and has the direction before an optional property name getShorthandBorderValue(direction, attribute = 'border')?

@iRyusa
Copy link
Member

iRyusa commented Aug 28, 2023

Oh. I didn't think about inner-border you're right 😬
As the project isn't typed, may be you can add an optional prefixAttribute as the second option yep to ensure retro compatibility

@garrettjohnson
Copy link
Contributor Author

@iRyusa Updated. Let me know if you need anything

@iRyusa iRyusa merged commit 50cd15b into mjmlio:master Sep 11, 2023
3 checks passed
@iRyusa
Copy link
Member

iRyusa commented Sep 11, 2023

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

style="width:NaNInfinity;" is breaking the Outlook experience
2 participants