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

[system] Add experimental_sx utility #29833

Merged
merged 11 commits into from Nov 25, 2021
Merged

Conversation

mnajdova
Copy link
Member

Fixes #28678
Related to #29816

The goal of this new API, is to allow users to use the sx syntax inside the styled() utility primerly.

Why?

  • it would allow developers to easily move styles between the sx and the styled() utility without having surprise as to why some styles are different/not working
  • it would unlock people who would like to have "one way" of defining styles, not having to choose one based on where it is used.

How?

I would like to start with minimal and most simple implementation, which would be defining the styles and propagating the theme. This should allow developers to use this syntax wherever they want, for example, in the styled() utility, when defining the variants for the component, when defining mixins etc.

Some alternatives proposed: #28678 (comment)

@mui-pr-bot
Copy link

mui-pr-bot commented Nov 22, 2021

Details of bundle changes

Generated by 🚫 dangerJS against 1f16e9c

@petermiles
Copy link

petermiles commented Nov 23, 2021

It'd be nice if we could use the theme within unstable_sx without having to pass it in. Isn't the whole point of sx mapping styles directly to theme values?

@mnajdova

This comment has been minimized.

@mnajdova mnajdova changed the title [system] Add unstable_sx utility [system] Add experimental_sx utility Nov 23, 2021
@mnajdova mnajdova marked this pull request as ready for review November 23, 2021 09:19
@mnajdova mnajdova requested a review from a team November 23, 2021 09:19
Comment on lines 3 to 5
function sx(styles, theme) {
return styleFunctionSx({ sx: styles, theme });
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you try with:

const sx = (styles) => ({ theme }) => styleFunctionSx({ sx: styles, theme });

From what I check in emotion & styled-components, they support nested function.

I prefer this because I don't need to get the theme if we don't need it (mostly we don't need the theme if we want to use sx).

const MyThemeComponent = styled('div')(
  sx({
      color: 'primary.contrastText',
      backgroundColor: 'primary.main',
      padding: 1,
      borderRadius: 1,
    })
);

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to create an API not tightly related to the styled(), but maybe this is the only use-case, so yeah, let me update

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. I've tested it using in the styled() utility and in the variants in side the theme. I can't think of any other use-cases at this moment.

Comment on lines +19 to +24
sx({
color: 'primary.contrastText',
backgroundColor: 'primary.main',
padding: 1,
borderRadius: 1,
}),
Copy link
Member

Choose a reason for hiding this comment

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

One question, does sx work without ThemeProvider?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's just that I could not really test that the values are applied :) I will add test for it too, just to ensure it does not throw

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

🎉🎉🎉

Copy link
Member

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

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

I love this new utility :)

@mnajdova mnajdova merged commit 8c0f63e into mui:master Nov 25, 2021
@mbrookes mbrookes added the package: system Specific to @mui/system label Nov 28, 2021
@oliviertassinari oliviertassinari added the new feature New feature or request label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[system] styled should accept sx syntax
7 participants