Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Typography] Migrate styles to emotion #23841
[Typography] Migrate styles to emotion #23841
Changes from 11 commits
8bdd48c
e844fc4
565cd4f
2f95356
8ae6f0d
1d3cc1d
0cad18a
eb9e493
5540769
859ca7f
f8c9934
158a5ff
1e8567f
f518bd0
8da0c44
925f180
9910160
e89016f
f624b0d
a439583
290dd31
4a70683
a656da8
d9d73ed
5c5e764
3280345
25ec283
66a81e8
40b9ec5
c42898f
d764665
e73af0f
928f85d
ff81d3e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had conflict here with the
as
prop from emotion.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should document the props that are now intercepted by emotion. It is a breaking change if you were passing these through. Maybe add a section in the changelog that only lists affected components and once we migrated all we just give the general advise that if you had
<MuiComponent component={SomeOtherComponent} emotionProp1 emotionProp2 />
, that these are no longer passed toSomeOtherComponent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we need to update the Next.js and Gatsby examples for the conflict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eps1lon will add an entry in the
migration-v4.md
for it. @oliviertassinari good call, let me check if the examples need to be updated.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added migration entry - d764665 we can extend it with more props if there are any. At this moment this is the only one I could notice not being propagated.
Both
nextjs
examples have been updated. I didn't noticed anything worth updating in the gatsby examples.