-
Notifications
You must be signed in to change notification settings - Fork 575
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: preserve bound media query styles when converting builder to mitosis #1644
Changes from 6 commits
7745b42
a6ab66a
b0f3b44
fec169d
e3c5b10
78d5015
8d7ae81
1cb7328
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@builder.io/mitosis': patch | ||
--- | ||
|
||
[Builder] preserve bound media query styles when converting to Mitosis |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,17 +108,46 @@ const getActionBindingsFromBlock = ( | |
|
||
const getStyleStringFromBlock = (block: BuilderElement, options: BuilderToMitosisOptions) => { | ||
const styleBindings: any = {}; | ||
const responsiveStyles: Record<string, Record<string, string>> = {}; | ||
let styleString = ''; | ||
|
||
if (block.bindings) { | ||
for (const key in block.bindings) { | ||
if (key.includes('style') && key.includes('.')) { | ||
if (!key.includes('.')) { | ||
continue; | ||
} | ||
if (key.includes('style')) { | ||
const styleProperty = key.split('.')[1]; | ||
styleBindings[styleProperty] = convertExportDefaultToReturn( | ||
block.code?.bindings?.[key] || block.bindings[key], | ||
); | ||
/** | ||
* responsiveStyles that are bound need to be merged into media queries. | ||
* Example: | ||
* responsiveStyles.large.color: "state.color" | ||
* responsiveStyles.large.background: "state.background" | ||
* Should get mapped to: | ||
* @media (max-width: 1200px): { | ||
* color: "state.color", | ||
* background: "state.background" | ||
* } | ||
*/ | ||
} else if (key.includes('responsiveStyles')) { | ||
const [_, size, prop] = key.split('.'); | ||
const mediaKey = `@media (max-width: ${sizes[size as Size].max}px)`; | ||
|
||
const objKey = `'${mediaKey}'`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just out of curiosity more than anything, does wrapping this do anything as opposed to using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The key needs to be surrounded in quotes because it has spaces/special characters. Here's what it looks like without the extra quotes: - Expected
+ Received
{
"style": {
"bindingType": "expression",
- "code": "{ fontSize: state.fontSize, '@media (max-width: 640px)': {\"left\":\"state.left\",\"top\":\"state.top\"}, '@media (max-width: 1200px)': {\"color\":\"state.color\"}, }",
+ "code": "{ fontSize: state.fontSize, @media (max-width: 640px): {\"left\":\"state.left\",\"top\":\"state.top\"}, @media (max-width: 1200px): {\"color\":\"state.color\"}, }",
"type": "single",
},
} Though this is a good reminder that I should document why this is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. documented here: 8d7ae81 also switched to using double quotes for consistency with the rest of the object (shouldn't make any functional difference) |
||
responsiveStyles[objKey] = { | ||
...responsiveStyles[objKey], | ||
[prop]: block.bindings[key], | ||
}; | ||
} | ||
} | ||
|
||
// All binding values are strings, so stringify media query objects | ||
for (const key in responsiveStyles) { | ||
styleBindings[key] = JSON.stringify(responsiveStyles[key]); | ||
} | ||
} | ||
|
||
const styleKeys = Object.keys(styleBindings); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to show that the presence of
responsiveStyles.
doesn't interfere with other styles bound usingstyle.