feat(mdThemeProvider): register custom theme styles #7864
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
1 similar comment
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it! |
|
CLAs look good, thanks! |
1 similar comment
|
CLAs look good, thanks! |
| @@ -153,6 +161,9 @@ function ThemingProvider($mdColorPalette) { | |||
| extendPalette: extendPalette, | |||
| theme: registerTheme, | |||
|
|
|||
| registerStyles: function(styles) { | |||
There was a problem hiding this comment.
I initially thought, that there will be a function, which compiles the provided style on demand as well? Thoughts?
There was a problem hiding this comment.
Could you clarify what you mean by compile? I would expect the provided styles are valid CSS just like $MD_THEME_CSS and will just be appended to the constant before the themes are generated.
There was a problem hiding this comment.
@Emeegeemee Sorry, I got some different thoughts, which are not valid anymore, since $mdColors will come.
Just thought again about your method, and it looks good.
|
ThomasBurleson
added
needs: team discussion
|
I've updated the tests to be more specific and added a few missing checks, please let me know if you want something more specific tested. As for a demo I'm not really sure where to start, from my perspective all of the currently implemented components serve as great demo's to the power of using the theming system. Would a document explaining how to create and register custom styles be better? I'm thinking of updating and moving this guide into the theme section of the documentation if that make sense. As for your "too risky" comment could you give some examples that I could help reduce or mitigate. I definitely see the potential for developers making a mistake with registering styles. Doing so would cause the rest of the theming to break, but I believe that you would just get components without theming similar to if the themes were disabled. |
ThomasBurleson
added
ui: theme
pr: merge ready
|
@devversion - can you rebase this please and force push to this PR? |
ThomasBurleson
added
the
needs: rebase
|
@devversion - ping! |
* Add a public method, which allows developers to register their own theme styles for the app theming * Adds descriptions and examples to the service documentation. Credits to @Emeegeemee for implementing the `registerStyles` functionality. Closes angular#7708. Closes angular#7864.
|
@Emeegeemee Thanks for the great PR again - We have a new PR, which includes your changes and gives deservedly credit to you :) |
* Add a public method, which allows developers to register their own theme styles for the app theming * Adds descriptions and examples to the service documentation. Credits to @Emeegeemee for implementing the `registerStyles` functionality. Closes angular#7708. Closes angular#7864.
* Add a public method, which allows developers to register their own theme styles for the app theming * Adds descriptions and examples to the service documentation. Credits to @Emeegeemee for implementing the `registerStyles` functionality. Closes #7708. Closes #7864. Closes #8641
Closes #7708