[MD settings] settings icon color per spec

[MD settings] settings icon color per spec

This CL changes the default icon color to the value
specified by Alan (UX designer).

BUG=425627
Committed: https://crrev.com/71df07fb8bde5191396b08ee8d5d9323c89e33d3
Cr-Commit-Position: refs/heads/master@{#371108}

[dschuyler, 2016-01-20 01:27:14]

dschuyler@chromium.org changed reviewers:
+ dpapad@chromium.org

[dpapad, 2016-01-20 02:24:40]

lgtm

https://codereview.chromium.org/1603913003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_shared.css (right):

https://codereview.chromium.org/1603913003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared.css:42: iron-icon {
Nit (optional): Maybe group rules that directly modify default polymer styles
together (like paper-button a few lines above), instead of interleaving them
with CSS classes we have defined.

https://codereview.chromium.org/1603913003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared.css:43: color: #969696;
Does this CSS rule affect only elements that directly use iron-icon, or does it
also affect elements that indirectly use it?

I am not sure, but I think that because of CSS scoping this would only affect
the former, in which case it makes me wonder why do we need this to be on the
"global" shared style instead of smaller CSS shared styles (buttons.css,
icons.css etc). In any case, as discussed previously, we are letting those
patters emerge as we go, so no need to change anything here, mentioning it
mostly to verify if I am interpreting the CSS rule correctly.

[dschuyler, 2016-01-21 22:00:53]

https://codereview.chromium.org/1603913003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_shared.css (right):

https://codereview.chromium.org/1603913003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared.css:42: iron-icon {
On 2016/01/20 02:24:40, dpapad wrote:
> Nit (optional): Maybe group rules that directly modify default polymer styles
> together (like paper-button a few lines above), instead of interleaving them
> with CSS classes we have defined.

Done.

https://codereview.chromium.org/1603913003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared.css:43: color: #969696;
On 2016/01/20 02:24:40, dpapad wrote:
> Does this CSS rule affect only elements that directly use iron-icon, or does
it
> also affect elements that indirectly use it?
> 
> I am not sure, but I think that because of CSS scoping this would only affect
> the former, in which case it makes me wonder why do we need this to be on the
> "global" shared style instead of smaller CSS shared styles (buttons.css,
> icons.css etc). In any case, as discussed previously, we are letting those
> patters emerge as we go, so no need to change anything here, mentioning it
> mostly to verify if I am interpreting the CSS rule correctly.

If I follow what you're asking, I agree that this needs
to be looked into further.  I'd like to look into polymer
themes and such that might address this better.

[dschuyler, 2016-01-23 00:02:19]

The CQ bit was checked by dschuyler@chromium.org

[dschuyler, 2016-01-23 00:02:19]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/1603913003/#ps20001
(title: "review changes")

[commit-bot: I haz the power, 2016-01-23 00:05:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1603913003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1603913003/20001

[commit-bot: I haz the power, 2016-01-23 01:06:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-23 01:08:02]

Description was changed from

==========
[MD settings] settings icon color per spec

This CL changes the default icon color to the value
specified by Alan (UX designer).

BUG=425627
==========

to

==========
[MD settings] settings icon color per spec

This CL changes the default icon color to the value
specified by Alan (UX designer).

BUG=425627

Committed: https://crrev.com/71df07fb8bde5191396b08ee8d5d9323c89e33d3
Cr-Commit-Position: refs/heads/master@{#371108}
==========

[commit-bot: I haz the power, 2016-01-23 01:08:02]

Patchset 2 (id:??) landed as
https://crrev.com/71df07fb8bde5191396b08ee8d5d9323c89e33d3
Cr-Commit-Position: refs/heads/master@{#371108}