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.

Sign up for GitHub

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

Jump to bottom

[material-ui][InputBase] Use globalCss for Pigment integration #42431

Merged
merged 8 commits into from
Jun 11, 2024
Merged

[material-ui][InputBase] Use globalCss for Pigment integration #42431

merged 8 commits into from
Jun 11, 2024

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented May 28, 2024 •
edited
Loading

Need mui/pigment-css#31 for the test to pass.

Summary

export globalCss from ../zero-styled to let Pigment CSS replace the call.

For default behavior, the globalCss will return <GlobalStyles /> as usual.

@siriwatknp siriwatknp requested review from mnajdova and brijeshb42 May 28, 2024 14:33
@mui-bot
Copy link

mui-bot commented May 28, 2024 •
edited
Loading

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 4a3aeec

@siriwatknp siriwatknp mentioned this pull request May 28, 2024
Merged
1 task
@siriwatknp
Copy link
Member Author

Tested this PR and mui/pigment-css#31 with Material UI template, it's working correctly through Pigment CSS:

image

mnajdova
mnajdova reviewed May 29, 2024
View reviewed changes
packages/mui-material/src/InputBase/InputBase.js
}}
/>
);
const InputGlobalStyles = globalCss({
Copy link
Member

Choose a reason for hiding this comment

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

We should include this change in the API in the migration guide, if we haven't done it already.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you explain more? I don't see any migration for InputBase. If you mean globalCss, I plan to merge this one first and then open a new PR for Pigment CSS opt-in guide.

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind, this is used internally, I was thinking this will be the API we document for developers. As long as we have the same public API, it should be fine

Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed that the same function is called injectGlobal in emotion. It should be fine though since we don't expose this through @mui/system

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 3, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 5, 2024
@siriwatknp siriwatknp requested a review from mnajdova June 5, 2024 16:35
@siriwatknp
Copy link
Member Author

@mnajdova @brijeshb42 I think this PR is ready for another review.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 6, 2024
@siriwatknp siriwatknp mentioned this pull request Jun 9, 2024
Closed
62 tasks
@siriwatknp siriwatknp requested a review from DiegoAndai June 11, 2024 03:51
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 11, 2024
mnajdova
mnajdova approved these changes Jun 11, 2024
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@siriwatknp siriwatknp merged commit d569d3f into mui:next Jun 11, 2024
19 checks passed
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Jun 12, 2024
@siriwatknp @mnajdova
[material-ui][InputBase] Use globalCss for Pigment integration (mui…
397a10a 
…#42431)
@o-alexandrov o-alexandrov mentioned this pull request Jun 15, 2024
Closed
joserodolfofreitas pushed a commit to joserodolfofreitas/material-ui that referenced this pull request Jul 29, 2024
@siriwatknp @joserodolfofreitas
[material-ui][InputBase] Use globalCss for Pigment integration (mui…
ef5001a 
…#42431)
@oliviertassinari oliviertassinari added the component: text field This is the name of the generic UI component, not the React module! label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnajdova mnajdova mnajdova approved these changes

@brijeshb42 brijeshb42 brijeshb42 approved these changes

@DiegoAndai DiegoAndai Awaiting requested review from DiegoAndai

Assignees
No one assigned
Labels
component: text field This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material v6.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@siriwatknp @mui-bot @brijeshb42 @mnajdova @oliviertassinari