[ash-md] Changed the style of ink drop applied to system menu icon buttons.

[ash-md] Changed the style of ink drop applied to system menu icon buttons.

Applied new spec changes to the ink drop which include:
- inset the size of the ink drop by 4dp on each side
- use a circle ink drop style for the detailed row header buttons
- updated highlight color to BLACK with 8% opacity
- updated ripple color to BLACK with 6% opacity
- disabled ink drop highlight for focus & hover

BUG=658731, 649734
Committed: https://crrev.com/f6be14bed96dd7e9078d7aee52f43000106ca9af
Cr-Commit-Position: refs/heads/master@{#428170}

[bruthig, 2016-10-27 20:01:37]

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

[bruthig, 2016-10-27 20:02:56]

bruthig@chromium.org changed reviewers:
+ tdanderson@chromium.org

[bruthig, 2016-10-27 20:02:57]

Terry, can you PTAL?

[commit-bot: I haz the power, 2016-10-27 20:03:14]

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

[bruthig, 2016-10-27 20:03:47]

Description was changed from

==========
[ash-md] Changed the style of ink drop applied to system menu icon buttons.

Applied new spec changes to the ink drop which include:
- inset the size of the ink drop by 4dp on each side
- use a circle ink drop style for the detailed row header buttons
- updated highlight color to BLACK with 8% opacity
- updated ripple color to BLACK with 6% opacity
- disabled ink drop highlight for focus & hover

BUG=658731
==========

to

==========
[ash-md] Changed the style of ink drop applied to system menu icon buttons.

Applied new spec changes to the ink drop which include:
- inset the size of the ink drop by 4dp on each side
- use a circle ink drop style for the detailed row header buttons
- updated highlight color to BLACK with 8% opacity
- updated ripple color to BLACK with 6% opacity
- disabled ink drop highlight for focus & hover

BUG=658731, 649734
==========

[commit-bot: I haz the power, 2016-10-27 21:15:34]

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

[commit-bot: I haz the power, 2016-10-27 21:15:35]

Dry run: This issue passed the CQ dry run.

[tdanderson, 2016-10-27 21:21:41]

LGTM with a few comments below.

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
File ash/common/system/tray/system_menu_button.cc (right):

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.cc:82: switch (ink_drop_style_) {
optional: Consider replacing 81-89 with just:

int highlight_radius = ink_drop_style_ == SQUARE ? width/2 : 0;

(Though I suspect you will elect to leave as-is.)

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
File ash/common/system/tray/system_menu_button.h (right):

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:45: // Defines the flavor of ink
drop ripple/highlight that should be constructed.
Consider including a brief mention of why one would want to choose one over the
other (e.g., flood fill for separator-bounded buttons, square otherwise)

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:46: InkDropStyle ink_drop_style_ =
InkDropStyle::SQUARE;
nit: I don't think initialization here is necessary. There is only one
constructor, and it takes in and initializes the ink drop style.

[bruthig, 2016-10-27 21:34:31]

Updated, no need to review but do so if you wish.

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
File ash/common/system/tray/system_menu_button.cc (right):

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.cc:82: switch (ink_drop_style_) {
On 2016/10/27 21:21:40, tdanderson wrote:
> optional: Consider replacing 81-89 with just:
> 
> int highlight_radius = ink_drop_style_ == SQUARE ? width/2 : 0;
> 
> (Though I suspect you will elect to leave as-is.)

You're right ;) I prefer switch cases when enumerating enums because it will
show up as a compiler error if another value is added.

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
File ash/common/system/tray/system_menu_button.h (right):

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:45: // Defines the flavor of ink
drop ripple/highlight that should be constructed.
On 2016/10/27 21:21:41, tdanderson wrote:
> Consider including a brief mention of why one would want to choose one over
the
> other (e.g., flood fill for separator-bounded buttons, square otherwise)

Done, added to the enum.

https://codereview.chromium.org/2456783005/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:46: InkDropStyle ink_drop_style_ =
InkDropStyle::SQUARE;
On 2016/10/27 21:21:41, tdanderson wrote:
> nit: I don't think initialization here is necessary. There is only one
> constructor, and it takes in and initializes the ink drop style.

Removed.

[bruthig, 2016-10-27 21:34:34]

The CQ bit was checked by bruthig@chromium.org

[bruthig, 2016-10-27 21:34:35]

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

[commit-bot: I haz the power, 2016-10-27 21:35:11]

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-10-27 22:45:09]

Description was changed from

==========
[ash-md] Changed the style of ink drop applied to system menu icon buttons.

Applied new spec changes to the ink drop which include:
- inset the size of the ink drop by 4dp on each side
- use a circle ink drop style for the detailed row header buttons
- updated highlight color to BLACK with 8% opacity
- updated ripple color to BLACK with 6% opacity
- disabled ink drop highlight for focus & hover

BUG=658731, 649734
==========

to

==========
[ash-md] Changed the style of ink drop applied to system menu icon buttons.

Applied new spec changes to the ink drop which include:
- inset the size of the ink drop by 4dp on each side
- use a circle ink drop style for the detailed row header buttons
- updated highlight color to BLACK with 8% opacity
- updated ripple color to BLACK with 6% opacity
- disabled ink drop highlight for focus & hover

BUG=658731, 649734
==========

[commit-bot: I haz the power, 2016-10-27 22:45:10]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-27 22:47:59]

Description was changed from

==========
[ash-md] Changed the style of ink drop applied to system menu icon buttons.

Applied new spec changes to the ink drop which include:
- inset the size of the ink drop by 4dp on each side
- use a circle ink drop style for the detailed row header buttons
- updated highlight color to BLACK with 8% opacity
- updated ripple color to BLACK with 6% opacity
- disabled ink drop highlight for focus & hover

BUG=658731, 649734
==========

to

==========
[ash-md] Changed the style of ink drop applied to system menu icon buttons.

Applied new spec changes to the ink drop which include:
- inset the size of the ink drop by 4dp on each side
- use a circle ink drop style for the detailed row header buttons
- updated highlight color to BLACK with 8% opacity
- updated ripple color to BLACK with 6% opacity
- disabled ink drop highlight for focus & hover

BUG=658731, 649734

Committed: https://crrev.com/f6be14bed96dd7e9078d7aee52f43000106ca9af
Cr-Commit-Position: refs/heads/master@{#428170}
==========

[commit-bot: I haz the power, 2016-10-27 22:48:00]

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