-
Notifications
You must be signed in to change notification settings - Fork 67
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
Deeply merging enhancement #19
Comments
I never thought about combining themes from an upper context. Right now the code assumes that you are using just one provider. Since usually providers are defined on the top of your app, maybe you can just merge both themes using Javascript? A deep merge from lodash should be enough. Apart from that, maybe I'm lost in translation :/ How is this related with postcss-apply?
By priority I mean context < default < prop |
I'll try again via another example(ignore nested ThemeProviders) :
In this example I expect the default An example attribute I would like to override could be: Default
Override by global
In this case they both get applied to the browser, but the Default takes priority. I definitely want to deeply merge, because I don't want to start from scratch with the react toolbox components. Hope this makes more sense! |
Oh I get it now. The thing is that we are getting modules but not the actual CSS. Processing the css is therefore out of the scope of this library. I know see what you mean with apply. It would work as something complementary but it has an important caveat (once this is resolved). Doing: @import 'react-toolbox/lib/button/theme.css';
:root {
--button-content: {
color: red;
}
} And then importing that file would work like a charm... We could even expose apply for every node same as we do with classes. But you are importing the whole theme.css so your styles will be duplicated :( I don't see a way of achieving this right now if it's not with what some people call "functional styles". That means rewriting the styles thought... |
@javivelasco No worries, Like I mentioned. One Work around is to include the !important flag. Thats usually a big no no, but In this case since we are keeping things modularized, I think I am okay with it. I'll keep an eye out on the |
In any case it's better if you check the priority of the selector in the browser and boost your own by replicating the same plus a classname for example. I have to revisit react toolbox styles to try to reduce specific selectors as much as possible. Anyway apply will solve some issues soon :) |
@javivelasco any news here? I believe pascalduez/postcss-apply#10 has been closed. I haven't had time to test. |
Not actually @gharwood. Honestly I think we can do nothing with ordering. This issue should be solved by controlling how modules are bundled together from your bundler config and overriding defaults with postcss plugins. Otherwise boosting priority is the only solution I think |
@javivelasco can we supply an example of how to override defaults with postcss plugins? |
Of course @petemill |
I would actually be super interested in merging themes in ThemeProvider if there was already ThemeProvider higher in component tree. We have a multi-module application where you would define styles in app root (first ThemeProvider) and then modify them on per module basis. Would you be ok with such change? I am willing to make a pr, if needed. |
@Podlas29 Actually I also need such feature but just do not have time to make a PR :) |
While this does offer the most flexibility, I think it can be improved to override styles set by parent themes/defaults.
Take this example. I think it would be cool to see specific nested styles defined in global overwritten if also defined by specialCase.
@javivelasco If I am understanding correctly, this might be pending work on postcss-apply?
Post Edit:
Note current work arounds are to use selectors that are more specific and/or override using
!important
.The text was updated successfully, but these errors were encountered: