App list: Change some colours to match the new specs.

App list: Change some colours to match the new specs.

This applies to both the classic and experimental app list styles.

BUG=415483
Committed: https://crrev.com/1d8ee05eda147b10b2d493a3113c30e5d4dda166
Cr-Commit-Position: refs/heads/master@{#297625}

[Matt Giuca, 2014-09-18 08:56:07]

mgiuca@chromium.org changed reviewers:
+ calamity@chromium.org, jstark@chromium.org

[Matt Giuca, 2014-09-18 08:56:08]

Screenshots are at:
https://code.google.com/p/chromium/issues/detail?id=415483#c1

calamity: code
jstark: Could you have a look at some of the colour values (referencing the
screenshots above) and see if you agree.

We weren't sure whether your specs were supposed to be definitive regarding
colour, or if they are just sizing specs. Thanks.

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
File ui/app_list/app_list_constants.cc (right):

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
ui/app_list/app_list_constants.cc:31: const SkColor kGridTitleColor =
SkColorSetRGB(0x33, 0x33, 0x33);
The mocks say this should be #333, but this looks darker than the mocks. See
screenshots.

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
ui/app_list/app_list_constants.cc:33: const SkColor kGridTitleHoverColor =
SkColorSetRGB(0x3C, 0x3C, 0x3C);
Jeff, could you supply a new value for the text hover colour?

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
ui/app_list/app_list_constants.cc:35: const SkColor kFolderTitleColor =
SkColorSetRGB(0x33, 0x33, 0x33);
Note: This was previously just using the system default colour (which is bad
because if you have very different system colours, it will clash with the rest
of the app list). I chose #333 to match the other text.

[Matt Giuca, 2014-09-18 08:56:55]

jstark: Also please comment on whether it is appropriate to change the old app
list, or whether we should only have the new colours come up for the
experimental app list.

[calamity, 2014-09-19 05:51:14]

code lgtm.

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
File ui/app_list/app_list_constants.cc (right):

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
ui/app_list/app_list_constants.cc:36: const SkColor kFolderTitleHintTextColor =
SkColorSetRGB(0xA0, 0xA0, 0xA0);
Should these textfield colors be the same as the search box colors?

[Matt Giuca, 2014-09-22 00:34:26]

mgiuca@chromium.org changed reviewers:
+ xiyuan@chromium.org

[Matt Giuca, 2014-09-22 00:34:27]

+xiyuan: OWNERS on the css file, thanks.

(Btw, can Chris and I get owners for chrome/browser/resources/app_list? Or does
everything in chrome/browser/resources need to get approved by you guys?)

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
File ui/app_list/app_list_constants.cc (right):

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
ui/app_list/app_list_constants.cc:33: const SkColor kGridTitleHoverColor =
SkColorSetRGB(0x3C, 0x3C, 0x3C);
Hmm, I'll just use black for now (keeping with the original theme of slightly
darkening the text when hovered).

https://codereview.chromium.org/578243002/diff/1/ui/app_list/app_list_constan...
ui/app_list/app_list_constants.cc:36: const SkColor kFolderTitleHintTextColor =
SkColorSetRGB(0xA0, 0xA0, 0xA0);
I don't think they should intrinsically be the same colour, since the folder
title is on a grey background and the search box is on a white background.

[xiyuan, 2014-09-22 16:31:08]

LGTM

Sure, you can create an OWNERS file in resources/app_list and I will be happy to
approve it. Just be sure to read "Web Development Style Guide" section on
dev.chromium.org.

[Matt Giuca, 2014-09-23 00:57:01]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2014-09-23 00:57:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/578243002/60001

[Matt Giuca, 2014-09-23 01:16:15]

The CQ bit was unchecked by mgiuca@chromium.org

[Matt Giuca, 2014-09-23 01:16:46]

Actually, unchecking the commit box because this is going to change the normal
app list as well, and we should get confirmation from UI designers before making
this change.

[Matt Giuca, 2014-10-01 03:56:39]

calamity: PTAL (new colours from UI folks and new app_list_background code).

[calamity, 2014-10-01 05:00:11]

slgtm

[Matt Giuca, 2014-10-01 05:19:44]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2014-10-01 05:20:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/578243002/120001

[commit-bot: I haz the power, 2014-10-01 10:55:28]

Committed patchset #7 (id:120001) as 53b6e03218664232b57ed88ed215dc7f3902b1d0

[commit-bot: I haz the power, 2014-10-01 10:56:05]

Patchset 7 (id:??) landed as
https://crrev.com/1d8ee05eda147b10b2d493a3113c30e5d4dda166
Cr-Commit-Position: refs/heads/master@{#297625}