This is a part of #3670. After having a conversation with @mbrookes we came to the conclusion that lightBaseTheme and darkBaseTheme are unnecessary for the following reasons:
lightBaseTheme acts as the default value for getMuiTheme function, which can just go into that function. all in one place: getMuiTheme!darkBaseTheme only overrides some of the values, it can be a simple copy/paste-able example in the Theme section!@callemall/material-ui Lets discuss this,
I'm good with this. Another options is we could make another repo with themes that users can contribute or just create some public snippets and link to them on the website. I think it's easy enough to override :+1:
@newoga That's also a good idea :+1: :+1: Another package under packages/ :sunglasses:
@newoga That's also a good idea :+1: :+1: Another package under packages/ :sunglasses:
Yeah exactly, material-ui-themes :smile:
@newoga The one thing I wish the theming system here could do is accept basic color options and automatically create a palette that fits the spec guidelines.
for eg:
const theme = getMuiTheme({ primary: 'indigo', accent: 'pink' });
There we go, brand spankin new theme.
@newoga The one thing I wish the theming system here could do is accept basic color options and automatically create a palette that fits the spec guidelines.
I think we could already do that like this:
const theme = getMuiTheme({pallete: { primary: 'indigo', accent: 'pink' }});
Except the keys are different. Though it would be cool to find a way to improve the default color matching, or fill it better defaults for the other color values based on just primary and accent. I'm not sure if that's possible though.
@newoga they do it at angular/material and it turns out pretty good IIRC
(there are recommendations in the spec for what shades/tones/accent/whatever to use and where)
angular.module('myApp', ['ngMaterial'])
.config(function($mdThemingProvider) {
$mdThemingProvider.theme('default')
.primaryPalette('pink')
.accentPalette('orange');
});
@nathanmarks That looks like a utility function to me: getMuiTheme(buildThemeFromColor(primary, accent)). And we can provide that feature easily :grin: maybe you're right, we might not really need another package for themes as they will be so easy to create.
@alitaheri RE typography, I actually think that MUI is lacking there. I had to create my own typography.js that exports the styles I need to recreate the typography set outlined in the spec.
If we're exporting individual shades of colors, why is typography getting bundled into a monster object? We need to add to it if anything so users can use material design spec type styles easily.
@nathanmarks Yeah @newoga has the same concerns. I'm not a big fan how our typography is right now. But I do know it's best place is within that function. so users can just read that function and see what they can override.
@alitaheri I'm thinking less along the lines of overrides and more to do with accessing the different type styles in your application outside of the theme. Or are you suggesting that they access the theme instead to get their type styles?
@nathanmarks typography is like a reference, same as colors. but lightBaseTheme isn't a reference its the default values! I don't think the default values should be in a different file. or if it is is shouldn't be a theme object! I'm just saying themes are unnecessary and confusing. but typography is another thing. We can provide categories and stuff, or even a build logic to dynamically build the object according to some input like primary color and locale. But the main goal of this discussion is to see if we really need the themes. Do you think having that buildThemeFromColor is enough?
@alitaheri Yeah I don't really touch the theme stuff if I can avoid it.
However, I find myself having to override the theme sometimes because otherwise the deeply nested styles are too much of a pain in the ass to reach for, and I hate littering my code with verbose style props (somethingOnTheTopLeftStyle)
and I hate littering my code with verbose style props (somethingOnTheTopLeftStyle)
I feel ya :laughing: :laughing: This happens because our components aren't composable enough!
Some of these ideas made it into next. I think we can close this now.
Most helpful comment
@nathanmarks That looks like a utility function to me:
getMuiTheme(buildThemeFromColor(primary, accent)). And we can provide that feature easily :grin: maybe you're right, we might not really need another package for themes as they will be so easy to create.