-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat(browser-color): enable browser header coloring #9192
Conversation
* @param {Object} options Options object for the browser color | ||
* @returns {Function} remove function of the browser color | ||
*/ | ||
var enableBrowserColor = function (options){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Missing space here at brace
@ThomasBurleson That looks very good. LGTM. |
3207057
to
36f2260
Compare
@EladBezalel tests? |
36f2260
to
68c8b92
Compare
@ErinCoughlan done |
* | ||
* @description | ||
* | ||
* A provider and a service that simplifies meta tags access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might make a note that certain <meta>
tags (such as charset
, http-equiv="Content-Type"
, and I think name="viewport"
) cannot be added dynamically because the browser will have already decided how to render (and probably rendered) the page by the time they get added.
I could not find a complete list of the ones that were editable, but this caught me the other day so it's probably not very obvious to most devs :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Left a few comments for consideration, but looks pretty great to me! |
@@ -242,6 +270,53 @@ function ThemingProvider($mdColorPalette) { | |||
|
|||
// Default theme defined in core.js | |||
|
|||
/** | |||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot documentation
8583337
to
7c236da
Compare
expect($$mdMeta.getMeta(name)).toBe(content); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing new line
How do you feel about a directive based around a class or attribute, that when added to an element, takes it's background color or For example This is what I use to achieve a similar toolbar affect in Angular Material as done in https://inbox.google.com/ |
@clshortfuse I think providing the service is enough. Developers should be able to figure out what color/hue the toolbar will use and match that relatively easily. |
7c236da
to
df3ea14
Compare
@ThomasBurleson LGTM 👍 |
- provides an easy way to set and change the browser color according the material design spec colors and the defined themes fixes #8062
df3ea14
to
db84790
Compare
fixes #8062