Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

add the reload function inside the themingProvider #5074

Closed
wants to merge 1 commit into from

Conversation

fmarcelo
Copy link

@fmarcelo fmarcelo commented Oct 8, 2015

We were trying to register theme on the fly and the only way to do it is assign $mdThemingProvider as $provide.value('themeService', $mdThemingProvider) inside the angular config.

In this case the themeService will be accessible to angular controllers and we can registerTheme with $http resolve values.

To apply the theme we call themeService.reload($injector);

add the reload function inside the themingProvider
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case 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!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@ThomasBurleson ThomasBurleson added the pr: merge ready This PR is ready for a caretaker to review label Oct 12, 2015
@ThomasBurleson ThomasBurleson added this to the 0.12.0 milestone Oct 12, 2015
@ThomasBurleson ThomasBurleson self-assigned this Oct 12, 2015
@breity
Copy link

breity commented Oct 23, 2015

This will be very useful for us too! One question:

Shouldn't the reload function be reload: generateThemes instead, so that the new themes actually get generated and output as css in the head? registerTheme doesn't actually output the css, right? Am I missing something here?

Thanks!

@Frank3K
Copy link
Contributor

Frank3K commented Oct 24, 2015

+1

@ThomasBurleson
Copy link
Contributor

@rschmukler, @jelbourn - can you review plz.

@ThomasBurleson ThomasBurleson added needs: review This PR is waiting on review from the team and removed pr: merge ready This PR is ready for a caretaker to review labels Nov 13, 2015
@ThomasBurleson ThomasBurleson modified the milestones: 1.0-rc2, 1.0-rc4 Nov 13, 2015
@jelbourn
Copy link
Member

This needs documentation and at least one unit test making sure that calling reload does what you want it to do.

@JeJan
Copy link

JeJan commented Dec 18, 2015

in 1.0-rc6 it has to be "reload: generateAllThemes" to work, but this still throws some warning logs

@chenav
Copy link

chenav commented Jan 14, 2016

+1 (a real important feature for multi-sites angular-based CMS initiatives, where theming is dynamic and set at run time)

@ThomasBurleson ThomasBurleson modified the milestones: 1.0-rc4, 1.0.6 Feb 5, 2016
@EladBezalel
Copy link
Member

@jelbourn i remember that registering themes on the fly was an idea,
If it's not valid please close it.

@jelbourn
Copy link
Member

The change seems okay, but it still needs unit testing.

@ThomasBurleson ThomasBurleson modified the milestones: 1.0.6, 1.0.8 Apr 4, 2016
@EladBezalel
Copy link
Member

@fmarcelo please add tests

@ThomasBurleson ThomasBurleson removed the needs: review This PR is waiting on review from the team label Apr 19, 2016
@ThomasBurleson ThomasBurleson modified the milestones: 1.0.8, Backlog Apr 21, 2016
@CodyMorris
Copy link

Would this also allow dynamic registration of palettes ? In my use case I need to get hex values from a backend service and use that to create palettes. No luck with any of these changes

@ThomasBurleson
Copy link
Contributor

This issue is closed as part of our ‘Surge Focus on Material 2' efforts.
For details, see our forum posting @ http://bit.ly/1UhZyWs.

@Splaktar Splaktar added the needs: unit tests This PR needs unit tests to cover the changes being proposed label Mar 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: unit tests This PR needs unit tests to cover the changes being proposed
Projects
None yet
Development

Successfully merging this pull request may close these issues.