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] Fix transform not firing when theme provided #24010

Merged
merged 5 commits into from Dec 14, 2020
Merged

[system] Fix transform not firing when theme provided #24010

merged 5 commits into from Dec 14, 2020

Conversation

ZovcIfzm
Copy link
Contributor

@ZovcIfzm ZovcIfzm commented Dec 14, 2020

Fixed the bug where transform was not fired if a theme was provided. Similarly, added tests to test this. Tests were provided from the changes in this thread (https://github.com/akomm/material-ui/blob/a4a394ff5420d29311ce1edef2bf20b51b507954/packages/material-ui-system/src/style.test.js)

And the change to style.js was provided in the issue discussion.

Fixes #23496

fixed the bug where transform was not fired if a theme was provided. Similarly, added tests to test this. Tests were provided from the changes in this thread (https://github.com/akomm/material-ui/blob/a4a394ff5420d29311ce1edef2bf20b51b507954/packages/material-ui-system/src/style.test.js)

And the change to style.js was provided in the issue discussion.
@mui-pr-bot
Copy link

mui-pr-bot commented Dec 14, 2020

Details of bundle changes

Generated by 🚫 dangerJS against 34295a2

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.

Changes look good, let's fix the tests and I believe it should be good to go :) Thanks for the contribution @ZovcIfzm

@oliviertassinari oliviertassinari changed the title [system] Fixed transform not firing when theme provided [system] Fix transform not firing when theme provided Dec 14, 2020
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work package: system Specific to @mui/system labels Dec 14, 2020
ZovcIfzm and others added 3 commits December 14, 2020 13:18
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
@ZovcIfzm
Copy link
Contributor Author

Thank you! All my tests are passing except Vercel (which I think is a permission based check?). This will also be my very first open source contribution ever! I'm very excited. =)

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.

@ZovcIfzm this is a great first pull request! Congrats 🥳

The Vercel build is currently experimental on the repo, so don't worry about it! Everything looks good from my end :)

@ZovcIfzm
Copy link
Contributor Author

Completed! All tests except Vercel pass.

@mnajdova mnajdova merged commit fe14d4b into mui:next Dec 14, 2020
@ZovcIfzm ZovcIfzm deleted the transform-with-theme branch December 14, 2020 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[system] transform and themeKey do not work together when using style from material-ui/system
4 participants