Updates to shelf icons for Ash material design

Updates to shelf icons for Ash material design

For the Ash material design shelf, update the
launcher icon from a magnifying glass to a
circle, and change the size of the notification
bell from 18 to 16.

BUG=634229, 641168
TEST=manual

Committed: https://crrev.com/0fc25a75528b2134e31dd1fe29c443ae4994ab82
Cr-Commit-Position: refs/heads/master@{#418304}

[tdanderson, 2016-09-08 18:51:25]

The CQ bit was checked by tdanderson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 18:52:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tdanderson, 2016-09-08 18:53:19]

tdanderson@chromium.org changed reviewers:
+ estade@chromium.org, jamescook@chromium.org

[tdanderson, 2016-09-08 18:53:19]

Evan and James, can you please take a look?

[commit-bot: I haz the power, 2016-09-08 20:14:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-08 20:14:20]

Dry run: This issue passed the CQ dry run.

[James Cook, 2016-09-08 20:23:44]

LGTM

https://codereview.chromium.org/2324913002/diff/1/ash/resources/vector_icons/...
File ash/resources/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2324913002/diff/1/ash/resources/vector_icons/...
ash/resources/vector_icons/BUILD.gn:11: "shelf_applist.1x.icon",
Thanks for migrating these into //ash.

[tdanderson, 2016-09-09 13:23:59]

The CQ bit was checked by tdanderson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 13:24:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 14:32:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 14:32:11]

Dry run: This issue passed the CQ dry run.

[tdanderson, 2016-09-09 20:25:39]

On 2016/09/08 20:23:44, James Cook wrote:
> LGTM
> 
>
https://codereview.chromium.org/2324913002/diff/1/ash/resources/vector_icons/...
> File ash/resources/vector_icons/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2324913002/diff/1/ash/resources/vector_icons/...
> ash/resources/vector_icons/BUILD.gn:11: "shelf_applist.1x.icon",
> Thanks for migrating these into //ash.

Evan, friendly ping - I need your OWNERS review of ui/gfx/vector_icons.

[Evan Stade, 2016-09-09 20:39:56]

https://codereview.chromium.org/2324913002/diff/20001/ash/common/system/web_n...
File ash/common/system/web_notification/web_notification_tray.cc (right):

https://codereview.chromium.org/2324913002/diff/20001/ash/common/system/web_n...
ash/common/system/web_notification/web_notification_tray.cc:313: bell_image =
CreateVectorIcon(kShelfNotificationsIcon, kShelfIconColor);
ternary operator?

https://codereview.chromium.org/2324913002/diff/20001/ash/resources/vector_ic...
File ash/resources/vector_icons/shelf_applist.icon (right):

https://codereview.chromium.org/2324913002/diff/20001/ash/resources/vector_ic...
ash/resources/vector_icons/shelf_applist.icon:1: // Copyright 2016 The Chromium
Authors. All rights reserved.
shouldn't this be called shelf_app_list.icon for consistency with the C++ class
names (like AppListView, etc.)

[tdanderson, 2016-09-09 21:13:40]

The CQ bit was checked by tdanderson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 21:14:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tdanderson, 2016-09-09 21:15:10]

Evan, please take another look.

https://codereview.chromium.org/2324913002/diff/20001/ash/common/system/web_n...
File ash/common/system/web_notification/web_notification_tray.cc (right):

https://codereview.chromium.org/2324913002/diff/20001/ash/common/system/web_n...
ash/common/system/web_notification/web_notification_tray.cc:313: bell_image =
CreateVectorIcon(kShelfNotificationsIcon, kShelfIconColor);
On 2016/09/09 20:39:56, Evan Stade wrote:
> ternary operator?

Done.

https://codereview.chromium.org/2324913002/diff/20001/ash/resources/vector_ic...
File ash/resources/vector_icons/shelf_applist.icon (right):

https://codereview.chromium.org/2324913002/diff/20001/ash/resources/vector_ic...
ash/resources/vector_icons/shelf_applist.icon:1: // Copyright 2016 The Chromium
Authors. All rights reserved.
On 2016/09/09 20:39:56, Evan Stade wrote:
> shouldn't this be called shelf_app_list.icon for consistency with the C++
class
> names (like AppListView, etc.)

Done.

[commit-bot: I haz the power, 2016-09-09 22:22:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 22:23:00]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-09-12 21:28:28]

https://codereview.chromium.org/2324913002/diff/40001/ash/common/system/web_n...
File ash/common/system/web_notification/web_notification_tray.cc (right):

https://codereview.chromium.org/2324913002/diff/40001/ash/common/system/web_n...
ash/common/system/web_notification/web_notification_tray.cc:246: gfx::Size size,
nit: const ref on size.

but better yet, why not remove size? ImageSkia has a size and it looks like it's
always the same here. You can probably just remove size and SetImageSize?

[tdanderson, 2016-09-13 16:46:59]

The CQ bit was checked by tdanderson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-13 16:47:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tdanderson, 2016-09-13 16:48:04]

Evan, please take another look.

cc yoshiki@ as FYI.

https://codereview.chromium.org/2324913002/diff/40001/ash/common/system/web_n...
File ash/common/system/web_notification/web_notification_tray.cc (right):

https://codereview.chromium.org/2324913002/diff/40001/ash/common/system/web_n...
ash/common/system/web_notification/web_notification_tray.cc:246: gfx::Size size,
On 2016/09/12 21:28:28, Evan Stade wrote:
> nit: const ref on size.
> 
> but better yet, why not remove size? ImageSkia has a size and it looks like
it's
> always the same here. You can probably just remove size and SetImageSize?

Done.

[Evan Stade, 2016-09-13 17:29:45]

lgtm

[commit-bot: I haz the power, 2016-09-13 17:53:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 17:53:53]

Dry run: This issue passed the CQ dry run.

[tdanderson, 2016-09-13 18:19:54]

The CQ bit was checked by tdanderson@chromium.org

[tdanderson, 2016-09-13 18:19:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2324913002/#ps60001
(title: "remove size parameter")

[commit-bot: I haz the power, 2016-09-13 18:20:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 18:26:23]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-13 18:28:30]

Description was changed from

==========
Updates to shelf icons for Ash material design

For the Ash material design shelf, update the
launcher icon from a magnifying glass to a
circle, and change the size of the notification
bell from 18 to 16.

BUG=634229,641168
TEST=manual
==========

to

==========
Updates to shelf icons for Ash material design

For the Ash material design shelf, update the
launcher icon from a magnifying glass to a
circle, and change the size of the notification
bell from 18 to 16.

BUG=634229,641168
TEST=manual

Committed: https://crrev.com/0fc25a75528b2134e31dd1fe29c443ae4994ab82
Cr-Commit-Position: refs/heads/master@{#418304}
==========

[commit-bot: I haz the power, 2016-09-13 18:28:31]

Patchset 4 (id:??) landed as
https://crrev.com/0fc25a75528b2134e31dd1fe29c443ae4994ab82
Cr-Commit-Position: refs/heads/master@{#418304}

[yoshiki, 2016-09-20 11:32:02]

On 2016/09/12 21:28:28, Evan Stade wrote:
>
https://codereview.chromium.org/2324913002/diff/40001/ash/common/system/web_n...
> File ash/common/system/web_notification/web_notification_tray.cc (right):
> 
>
https://codereview.chromium.org/2324913002/diff/40001/ash/common/system/web_n...
> ash/common/system/web_notification/web_notification_tray.cc:246: gfx::Size
size,
> nit: const ref on size.
> 
> but better yet, why not remove size? ImageSkia has a size and it looks like
it's
> always the same here. You can probably just remove size and SetImageSize?

Sorry, I missed this CL. 

This is necessary and please not remove this. Some icons come from external
application or extension, so we need to set the image size to align the icon
sizes.