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

[pigment-css][react] Fix sx prop transformation on Box #41705

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

brijeshb42
Copy link
Contributor

@brijeshb42 brijeshb42 commented Mar 29, 2024

This current fix is very crude and does not handle the case when a user might have their own wrapper component over Box and they'll be passing sx prop through the component. We need to have a better option to customize the condition.

Also made Box as a default import instead of name import.

Fixes #41704

@mui-bot
Copy link

mui-bot commented Mar 29, 2024

Netlify deploy preview

https://deploy-preview-41705--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 231748a

@@ -1,6 +1,6 @@
import * as React from 'react';
import { useLocation, matchRoutes, Link } from 'react-router-dom';
import { css } from '@pigment-css/react';
import Box from '@pigment-css/react/Box';
Copy link
Member

Choose a reason for hiding this comment

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

I guess there will be a question about the import in the future comparing:

import { css } from '@pigment-css/react';
import Box from '@pigment-css/react/Box';

I think we should pick one way of import.

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer import { Box } from '@pigment-css/react'; over default import.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer that everything is on a subpath. We don't want to repeat the same mistake of having barrel file exports from the start when we have control. Otherwise later, it'd harder to get the changes to be adopted by the user.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, let's use default export for Box. I added a task to move other APIs to the subpath before the stable beta release.

@zannager zannager added the package: pigment-css Specific to @pigment-css/* label Mar 29, 2024
This current fix is very crude and does not handle the case when a user
might have their own wrapper component over Box and they'll be passing
sx prop through the component.

Also made `Box` as a default import instead of name import.

Fixes mui#41704
@siriwatknp siriwatknp merged commit 5ad0e46 into mui:next Apr 2, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: pigment-css Specific to @pigment-css/*
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pigment-css] Box does not create styles
4 participants