-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[muiStyled] Support default theme when none is available #22791
Merged
Merged
Changes from 18 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
c2d31cc
add emotion peer dependencies
mnajdova 5ae933f
fixed types & tests
mnajdova 18b0668
prettier
mnajdova f0ef95c
Merge branch 'next' of https://github.com/mui-org/material-ui into next
mnajdova c7bebb8
Merge branch 'next' of https://github.com/mui-org/material-ui into next
mnajdova 92b2d6e
Merge branch 'next' of https://github.com/mui-org/material-ui into next
mnajdova 76256f1
wip
mnajdova 8661306
added ThemeProvider
mnajdova 61d7fac
exported ThemeProvider from styled-engine-sc
mnajdova c9fcdc1
tests wip
mnajdova 48305e9
tests
mnajdova 1c04d40
prettier
mnajdova 27d9f73
Update packages/material-ui/src/styles/muiStyled.d.ts
mnajdova b149da5
Update packages/material-ui/src/styles/customStyled.d.ts
mnajdova 05217a7
renamed styled -> legacy_styled, renamed customStyled -> styled
mnajdova 3bf96aa
fixed tests
mnajdova 551157f
added migration step, exported styled with deprecation warning
mnajdova a03252c
prettier
mnajdova 179a6e2
renamed legacy_styled back to styled
mnajdova 9878176
renamed styled to experimentalStyled
mnajdova 17ce5d4
fix
mnajdova b7ccb20
specify ThemeProvider as export from emotion-theming
mnajdova 72398c8
refactored muiStyled to be used as simple styled utility too
mnajdova 58a1fb9
fix lint issues
mnajdova aac9e83
renamed muiStyled to experimentalStyled
mnajdova 5774c19
refactored to use ThemeContext
mnajdova 8016f92
Update packages/material-ui-styles/README.md
mnajdova 6bc05c1
Update packages/material-ui-styles/README.md
mnajdova 9757a00
Update packages/material-ui-styles/package.json
mnajdova bb880fe
Moved nesting ThemeProvider to core
mnajdova 73d6589
Update packages/material-ui/src/styles/experimentalStyled.d.ts
mnajdova 6e2b480
Update packages/material-ui/src/styles/experimentalStyled.d.ts
mnajdova 0c8be58
added test
mnajdova a001588
fixed docs generation
mnajdova e676780
Update packages/material-ui/src/styles/ThemeProvider.js
mnajdova 0298222
Update packages/material-ui/src/styles/experimentalStyled.d.ts
mnajdova c96062d
addressed comments
mnajdova 3e419ac
Update packages/material-ui/src/styles/ThemeProvider.js
mnajdova 4fccc26
Update packages/material-ui/src/styles/ThemeProvider.js
mnajdova dcc97fa
docs:api
mnajdova File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
export * from '@emotion/styled'; | ||
export { default } from '@emotion/styled'; | ||
export * from 'emotion-theming'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
export { default } from '@emotion/styled'; | ||
export * from 'emotion-theming'; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export { default } from './styled'; | ||
export * from './styled'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from './styled'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { | ||
StyledOptions, | ||
JSXInEl, | ||
CreateStyledComponentIntrinsic, | ||
CreateStyledComponentExtrinsic, | ||
} from '../styles/muiStyled'; | ||
|
||
export interface CreateStyled<Theme extends object = any> { | ||
<Tag extends React.ComponentType<any>, ExtraProps = {}>( | ||
tag: Tag, | ||
options?: StyledOptions | ||
): CreateStyledComponentExtrinsic<Tag, ExtraProps, Theme>; | ||
|
||
<Tag extends keyof JSXInEl, ExtraProps = {}>( | ||
tag: Tag, | ||
options?: StyledOptions | ||
): CreateStyledComponentIntrinsic<Tag, ExtraProps, Theme>; | ||
} | ||
|
||
/** | ||
* Custom styled utility that has a default MUI theme. | ||
* | ||
* @param tag HTML tag or component that should serve as base. | ||
* @param options Styled options for the created component. | ||
* @returns React component that has styles attached to it. | ||
*/ | ||
declare const styled: CreateStyled; | ||
|
||
export default styled; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import seStyled from '@material-ui/styled-engine'; | ||
oliviertassinari marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import defaultTheme from '../styles/defaultTheme'; | ||
|
||
function isEmpty(obj) { | ||
let result = true; | ||
|
||
Object.keys(obj).forEach((key) => { | ||
if (obj.hasOwnProperty(key)) { | ||
result = false; | ||
} | ||
}); | ||
|
||
return result; | ||
} | ||
|
||
export default function styled(tag, options) { | ||
const defaultStyledResolver = seStyled(tag, options); | ||
|
||
const customStyledResolver = (...styles) => { | ||
const stylesWithDefaultTheme = styles.map((stylesArg) => { | ||
return typeof stylesArg === 'function' | ||
? ({ theme: themeInput, ...rest }) => | ||
stylesArg({ theme: isEmpty(themeInput) ? defaultTheme : themeInput, ...rest }) | ||
: stylesArg; | ||
}); | ||
|
||
return defaultStyledResolver(...stylesWithDefaultTheme); | ||
}; | ||
|
||
return customStyledResolver; | ||
} |
Oops, something went wrong.
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.
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 should specify what we need in case
emotion-theming
adds new modules that we don't need.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.
What problem does
emotion-theming
solve?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.
By adding the emotion's/styled-components'
ThemeProvider
in ourThemeProvider
component, developers no longer need to specifyThemeProvider
from emotion/styled component together with ours. Also, in thestyled
utility when custom theme is used, it will automatically be available onprops.theme
. In addition to this, when noThemeProvider
is used, thestyled
utility uses thedefaultTheme
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.
What if they already use
emotion-theming
?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 that using a different ThemeProvider (enforcing the usage of two) makes incremental adoption easier. Developers don't have to worry about conflicts of theme structure with existing usage of emotion/sc.
However, for using the css prop, it seems required https://emotion.sh/docs/theming#css-prop.
It seems that theme-ui imports the ThemeContext of emotion directly (without emotion-theming), https://github.com/system-ui/theme-ui/blob/af57d4ce97c0e8a3289115f33cef1d17bc65adad/packages/core/src/index.ts#L3. They also have emotion as a direct dependency.
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.
Just a clarification, for our components, like
Slider
,Button
,muiStyled
+ ourThemeProvider
is working, it won't work for the scenarios of usingmuiStyled
+div
,span
or other primitive elements as we do not have access to change the behavior there.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.
Oh, I see, thanks for the clarification. +1 for including the styled-engine theme provider in Material-UI's ThemeProvider component. It's simpler. For the theme structure conflict, developers can workaround the issue by nesting
<MuiThemeProvider> > <Emotion ThemeProvider>
.The only point I would suggest to explore is: Using the ThemeContext exported from
@emotion/core
vs.emotion-theming
.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.
Yeah, let me spend some time on this, just to make sure I am not missing something 👍
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.
Refactored to use only the ThemeContext (both emotion & styled-components expose this). Tested with both engines, tests are passing, so I think everything is covered. No new dependencies added with this 👍
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.
Nice :).