Introduce SystemMenuButton for Ash material design

Introduce SystemMenuButton for Ash material design

Introduce a SystemMenuButton class, which defines the
size, focus rect, and ripple effect common across
all material design image buttons used in Ash UI menus.
The header row for system menu detailed views are
modified to use these buttons.

BUG=649734
Committed: https://crrev.com/fa08dbd369de855417f39522c287f8557147cc3b
Cr-Commit-Position: refs/heads/master@{#420764}

[tdanderson, 2016-09-23 18:04:24]

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

[commit-bot: I haz the power, 2016-09-23 18:04:58]

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-23 18:09:18]

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

[tdanderson, 2016-09-23 18:09:18]

James, can you please take a look?

[James Cook, 2016-09-23 18:32:17]

LGTM with nits. Nice refactor.

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/spec...
File ash/common/system/tray/special_popup_row.cc (right):

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/spec...
ash/common/system/tray/special_popup_row.cc:110: SystemMenuButton*
SpecialPopupRow::AddSystemMenuButton(
Thanks for renaming this.

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

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.cc:25: const int horizontal_padding =
(kMenuButtonSize - image.width()) / 2;
Aside: It's a bit of a bummer these constants don't have names (or a namespace)
that help you figure out which file they are in. But that's a problem for
another CL.

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

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:8: #include
"ash/resources/vector_icons/vector_icons.h"
#include macros.h

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:19: class SystemMenuButton : public
views::ImageButton {
nit: Class docs. What does this do and why would you choose to use it?

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:21:
SystemMenuButton(views::ButtonListener* listener,
nit: Document parameters. For example, what sort of ID is |accessible_name_id|?

[tdanderson, 2016-09-23 19:53:13]

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

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:8: #include
"ash/resources/vector_icons/vector_icons.h"
On 2016/09/23 18:32:16, James Cook wrote:
> #include macros.h

Done.

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:19: class SystemMenuButton : public
views::ImageButton {
On 2016/09/23 18:32:17, James Cook wrote:
> nit: Class docs. What does this do and why would you choose to use it?

Slipped my mind, sorry. Done.

https://codereview.chromium.org/2367903002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_menu_button.h:21:
SystemMenuButton(views::ButtonListener* listener,
On 2016/09/23 18:32:16, James Cook wrote:
> nit: Document parameters. For example, what sort of ID is
|accessible_name_id|?

Done.

[tdanderson, 2016-09-23 19:53:16]

The CQ bit was checked by tdanderson@chromium.org

[tdanderson, 2016-09-23 19:53:17]

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/2367903002/#ps20001
(title: "comments")

[commit-bot: I haz the power, 2016-09-23 19:53:42]

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-23 21:16:03]

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

[commit-bot: I haz the power, 2016-09-23 21:16:04]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[tdanderson, 2016-09-23 21:46:23]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2016-09-23 21:47:09]

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-23 22:54:24]

Committed patchset #2 (id:20001)

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

Description was changed from

==========
Introduce SystemMenuButton for Ash material design

Introduce a SystemMenuButton class, which defines the
size, focus rect, and ripple effect common across
all material design image buttons used in Ash UI menus.
The header row for system menu detailed views are
modified to use these buttons.

BUG=649734
==========

to

==========
Introduce SystemMenuButton for Ash material design

Introduce a SystemMenuButton class, which defines the
size, focus rect, and ripple effect common across
all material design image buttons used in Ash UI menus.
The header row for system menu detailed views are
modified to use these buttons.

BUG=649734

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

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

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