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

feat(core): useStyle hook #758

Merged
merged 8 commits into from Oct 4, 2022

Conversation

raymondmuller
Copy link
Collaborator

Description

See feature idea discussion here: #757

@vercel
Copy link

vercel bot commented Sep 23, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
mitosis-fiddle ✅ Ready (Inspect) Visit Preview Oct 3, 2022 at 7:40PM (UTC)

@raymondmuller raymondmuller changed the title feat: useStyle poc feat(core): useStyle poc Sep 23, 2022
Copy link
Contributor

@samijaber samijaber left a comment

Choose a reason for hiding this comment

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

I think you're better off making the change once inside of collectCss, the helper responsible for aggregating all of the styles into a CSS string. You could append json.style to its output, which ought to give us support in all generators without changing the code within them.

export const collectCss = (json: MitosisComponent, options: CollectStyleOptions = {}): string => {
const styles = collectStyles(json, options);
// TODO create and use a root selector
return classStyleMapToCss(styles);
};

Mind giving that a shot and see if it works without hiccups?

@raymondmuller raymondmuller marked this pull request as ready for review September 30, 2022 14:31
@raymondmuller raymondmuller requested a review from a team as a code owner September 30, 2022 14:31
@raymondmuller raymondmuller requested review from samijaber and removed request for a team September 30, 2022 14:31
Copy link
Contributor

@samijaber samijaber left a comment

Choose a reason for hiding this comment

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

Love this! Can we add 2 component snapshot tests:

  • 1 that uses only useStyle
  • 1 that uses useStyle and the css prop, showing how they get combined.

Co-authored-by: Sami Jaber <jabersami@gmail.com>
Copy link
Contributor

@samijaber samijaber left a comment

Choose a reason for hiding this comment

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

Excellent @raymondmuller . Thank you for this new feature!! 🏆

@samijaber samijaber merged commit 916eaf9 into BuilderIO:main Oct 4, 2022
@raymondmuller raymondmuller deleted the feat/use-style-poc branch October 4, 2022 21:30
@samijaber samijaber changed the title feat(core): useStyle poc feat(core): useStyle hook Oct 5, 2022
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.

None yet

2 participants