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

Theme example breaks with AppBar usage #20136

Closed
2 tasks done
matthewoates opened this issue Mar 16, 2020 · 4 comments · Fixed by #20339
Closed
2 tasks done

Theme example breaks with AppBar usage #20136

matthewoates opened this issue Mar 16, 2020 · 4 comments · Fixed by #20339
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@matthewoates
Copy link
Contributor

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

https://codesandbox.io/s/material-demo-qjiv5

This breaks with the suggested theme object:

const themeInstance = {
  background: "linear-gradient(45deg, #FE6B8B 30%, #FF8E53 90%)"
};

To fix the cannot set property type of undefined error:

const themeInstance = {
  background: "linear-gradient(45deg, #FE6B8B 30%, #FF8E53 90%)",
  palette: {
    type: "dark"
  }
};

And to fix that error:

import { grey } from '@material-ui/core/colors';

const themeInstance = {
  background: "linear-gradient(45deg, #FE6B8B 30%, #FF8E53 90%)",
  palette: {
    type: "dark",
    grey
  }
};

and I'm presented with yet another error. I'd expect the theme manager to merge the theme object correctly so that fields aren't missing.

@matthewoates
Copy link
Contributor Author

matthewoates commented Mar 16, 2020

It looks like the docs here:

and the Codesandboxes need to be updated to include createMuiTheme({ background: ... })

@eps1lon
Copy link
Member

eps1lon commented Mar 16, 2020

Yeah the example is not very helpful since it uses /core but wouldn't work with /core components. We should wrap it in createMuiTheme.

@eps1lon eps1lon added docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. labels Mar 16, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 19, 2020

Definitely, it will be clearer with something like this:

diff --git a/docs/src/pages/styles/advanced/advanced.md b/docs/src/pages/styles/advanced/advanced.md
index 53227692a..c192f2c1a 100644
--- a/docs/src/pages/styles/advanced/advanced.md
+++ b/docs/src/pages/styles/advanced/advanced.md
@@ -6,7 +6,7 @@

 Add a `ThemeProvider` to the top level of your app to pass a theme down the React component tree. Then, you can access the theme object in style functions.

-> This example creates a new theme. See the [theming section](/customization/theming/) for how to customize the default Material-UI theme.
+> This example creates a minimalist theme object. Head to the [theming section](/customization/theming/) if you intend to use some of the Material-UI's components. You need to provide a richer theme structure using the `createMuiTheme()` method.

 ```jsx
 import { ThemeProvider } from '@material-ui/core/styles';

@matthewoates do you want to submit a pull request? :)

@maksimgm
Copy link
Contributor

PR created:
#20339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants