Adds emoji/handwriting/vocie buttons on opt-in IME menu.

Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons:
Emoji: ash/resources/vector_icons/ime_menu_emoticon.(1x.)icon
Voice: ash/resources/vector_icons/ime_menu_microphone.(1x.)icon
Handwriting: ash/resources/vector_icons/ime_menu_write.(1x.)icon

2) Update the positions of the buttons: should be emoji-handwriting-voice buttons.

3) Updates the UI based on spec: https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-%20spec.png

BUG=642385, 643304, 643305
TEST=Verified on local build.

Committed: https://crrev.com/6a18628f8826ebc484f06fcfabcfc3db0dd9d508
Cr-Commit-Position: refs/heads/master@{#419775}

[Azure Wei, 2016-08-23 02:24:23]

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

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

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-08-23 04:09:38]

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

[commit-bot: I haz the power, 2016-08-23 04:09:39]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Azure Wei, 2016-08-23 04:47:34]

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

[commit-bot: I haz the power, 2016-08-23 04:47:47]

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-08-23 05:32:25]

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

[commit-bot: I haz the power, 2016-08-23 05:32:26]

Dry run: This issue passed the CQ dry run.

[Azure Wei, 2016-08-23 06:11:37]

Description was changed from

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

BUG=570761
TEST=Verified on local build.
==========

to

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

BUG=570761
TEST=Verified on local build.
==========

[Azure Wei, 2016-08-23 06:11:38]

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

[Azure Wei, 2016-08-23 06:11:57]

Please review this cl. Thanks!

[James Cook, 2016-08-23 16:01:22]

Code LGTM, but please follow up with UX. I think we should have separate artwork
for the hover and disabled states.

https://codereview.chromium.org/2271523002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://codereview.chromium.org/2271523002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:86:
IDS_ASH_STATUS_TRAY_EMOJI, IDS_ASH_STATUS_TRAY_EMOJI, 1);
nit: Use a constant for 1, like const int right_border = 1;

https://codereview.chromium.org/2271523002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:93:
IDR_AURA_UBER_TRAY_VOICE, IDR_AURA_UBER_TRAY_VOICE,
Should there be separate artwork for the disabled and hover states?

[Azure Wei, 2016-09-18 05:32:45]

Description was changed from

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

BUG=570761
TEST=Verified on local build.
==========

to

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons: Emoji (emoticon.1x.icon &
emoticon.icon), Voice (microphone.1x.icon & microphone.icon) and Handwriting
(write.1x.icon & write.icon).

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304
TEST=Verified on local build.
==========

[Azure Wei, 2016-09-18 05:38:03]

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

[Azure Wei, 2016-09-18 05:40:03]

https://codereview.chromium.org/2271523002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://codereview.chromium.org/2271523002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:86:
IDS_ASH_STATUS_TRAY_EMOJI, IDS_ASH_STATUS_TRAY_EMOJI, 1);
On 2016/08/23 16:01:22, James Cook wrote:
> nit: Use a constant for 1, like const int right_border = 1;

Removed this border.

https://codereview.chromium.org/2271523002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:93:
IDR_AURA_UBER_TRAY_VOICE, IDR_AURA_UBER_TRAY_VOICE,
On 2016/08/23 16:01:22, James Cook wrote:
> Should there be separate artwork for the disabled and hover states?

Updated with vectorized icons instead of PNGs.

[Azure Wei, 2016-09-18 05:41:11]

R:+tdanderson@ for material design.
This cl has been updated. Please take another look. Thanks!

[Azure Wei, 2016-09-18 12:48:22]

Description was changed from

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons: Emoji (emoticon.1x.icon &
emoticon.icon), Voice (microphone.1x.icon & microphone.icon) and Handwriting
(write.1x.icon & write.icon).

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304
TEST=Verified on local build.
==========

to

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons: Emoji (emoticon.1x.icon &
emoticon.icon), Voice (microphone.1x.icon & microphone.icon) and Handwriting
(write.1x.icon & write.icon).

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304, 643305
TEST=Verified on local build.
==========

[tdanderson, 2016-09-19 15:44:34]

Thanks for taking a look at this. Please see my comments below:

https://chromiumcodereview.appspot.com/2271523002/diff/60001/ash/common/syste...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://chromiumcodereview.appspot.com/2271523002/diff/60001/ash/common/syste...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:38: const int
kButtonIconSize = 20;
There is already a constant kMenuIconSize in tray_constants that is used by
different surfaces. Can you please use that one instead?

https://chromiumcodereview.appspot.com/2271523002/diff/60001/ash/common/syste...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:41: const int
kButtonViewHeight = 56;
I have a CL in progress that will be adding kMenuButtonSize = 48 in
tray_constants. Would you mind making that change here? You can then return
kMenuButtonSize + 2 * kButtonInsideBorderSpacing in GetHeightForWidth().

https://chromiumcodereview.appspot.com/2271523002/diff/60001/ash/common/syste...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:45: const int
kButtonInsideBorderSpacing = 4;
This padding constant of 4 will be seen a lot in Ash MD. Can you please add a
TODO(tdanderson): move this constant into tray_constants and share across
different UI surfaces?

https://chromiumcodereview.appspot.com/2271523002/diff/60001/ash/common/syste...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:83:
CreateVectorIcon(vector_id, kButtonIconSize, gfx::kChromeIconGrey),
Please use kMenuIconColor (found in tray_constants) instead of
gfx::kChromeIconGrey here.

https://chromiumcodereview.appspot.com/2271523002/diff/60001/ash/common/syste...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:181: settings_button_ =
CreateImeMenuButton(this, gfx::VectorIconId::SETTINGS,
The preexisting settings icon actually doesn't look very good when drawn on a
20x20 pixel grid. I'm in the process of modifying this icon and can update this
call site myself - can you please add a TODO(tdanderson): update this icon ?

https://chromiumcodereview.appspot.com/2271523002/diff/60001/ui/gfx/vector_ic...
File ui/gfx/vector_icons/BUILD.gn (right):

https://chromiumcodereview.appspot.com/2271523002/diff/60001/ui/gfx/vector_ic...
ui/gfx/vector_icons/BUILD.gn:38: "emoticon.1x.icon",
Since these are being used specifically for Ash, so all six of these new icons
should instead live in ash/resources/vector_icons (this is a somewhat recent
change, and I have updated the documentation to explain:
https://www.chromium.org/developers/how-tos/vectorized-icons-in-native-chrome-ui)

Also please name these system_menu_emoticon (or ime_menu_emoticon if you wish)
to stay consistent with the other icons that are on a 20x20 pixel grid.

[Azure Wei, 2016-09-20 01:05:45]

https://codereview.chromium.org/2271523002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://codereview.chromium.org/2271523002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:38: const int
kButtonIconSize = 20;
On 2016/09/19 15:44:34, tdanderson wrote:
> There is already a constant kMenuIconSize in tray_constants that is used by
> different surfaces. Can you please use that one instead?

Done.

https://codereview.chromium.org/2271523002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:41: const int
kButtonViewHeight = 56;
On 2016/09/19 15:44:34, tdanderson wrote:
> I have a CL in progress that will be adding kMenuButtonSize = 48 in
> tray_constants. Would you mind making that change here? You can then return
> kMenuButtonSize + 2 * kButtonInsideBorderSpacing in GetHeightForWidth().

Done.

https://codereview.chromium.org/2271523002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:45: const int
kButtonInsideBorderSpacing = 4;
On 2016/09/19 15:44:34, tdanderson wrote:
> This padding constant of 4 will be seen a lot in Ash MD. Can you please add a
> TODO(tdanderson): move this constant into tray_constants and share across
> different UI surfaces?

Done.

https://codereview.chromium.org/2271523002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:83:
CreateVectorIcon(vector_id, kButtonIconSize, gfx::kChromeIconGrey),
On 2016/09/19 15:44:34, tdanderson wrote:
> Please use kMenuIconColor (found in tray_constants) instead of
> gfx::kChromeIconGrey here.

Done.

https://codereview.chromium.org/2271523002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:181: settings_button_ =
CreateImeMenuButton(this, gfx::VectorIconId::SETTINGS,
On 2016/09/19 15:44:34, tdanderson wrote:
> The preexisting settings icon actually doesn't look very good when drawn on a
> 20x20 pixel grid. I'm in the process of modifying this icon and can update
this
> call site myself - can you please add a TODO(tdanderson): update this icon ?

Done.

https://codereview.chromium.org/2271523002/diff/60001/ui/gfx/vector_icons/BUI...
File ui/gfx/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2271523002/diff/60001/ui/gfx/vector_icons/BUI...
ui/gfx/vector_icons/BUILD.gn:38: "emoticon.1x.icon",
On 2016/09/19 15:44:34, tdanderson wrote:
> Since these are being used specifically for Ash, so all six of these new icons
> should instead live in ash/resources/vector_icons (this is a somewhat recent
> change, and I have updated the documentation to explain:
>
https://www.chromium.org/developers/how-tos/vectorized-icons-in-native-chrome-ui)
> 
> Also please name these system_menu_emoticon (or ime_menu_emoticon if you wish)
> to stay consistent with the other icons that are on a 20x20 pixel grid.

Done. Thanks for the doc.

[Azure Wei, 2016-09-20 01:07:34]

Description was changed from

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons: Emoji (emoticon.1x.icon &
emoticon.icon), Voice (microphone.1x.icon & microphone.icon) and Handwriting
(write.1x.icon & write.icon).

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304, 643305
TEST=Verified on local build.
==========

to

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons:
Emoji: ash/resources/vector_icons/ime_menu_emoticon.(1x.)icon
Voice: ash/resources/vector_icons/ime_menu_microphone.(1x.)icon
Handwriting: ash/resources/vector_icons/ime_menu_write.(1x.)icon

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304, 643305
TEST=Verified on local build.
==========

[tdanderson, 2016-09-20 13:11:12]

lgtm with two comments below.

https://chromiumcodereview.appspot.com/2271523002/diff/100001/ash/common/syst...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://chromiumcodereview.appspot.com/2271523002/diff/100001/ash/common/syst...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:30: #include
"ui/gfx/vector_icons_public.h"
nit: this is not needed since you're including vector_icons.h

https://chromiumcodereview.appspot.com/2271523002/diff/100001/ash/common/syst...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:160: if
(!MaterialDesignController::IsShelfMaterial()) {
Since the changes here are for the buttons inside the IME menu, this should
instead be checking IsSystemTrayMenuMaterial(). That's what we're using to
develop the changes to the system menu, including the IME detailed view (which
the opt-in menu shares the input method rows with).

Line 80 is fine as-is since IsShelfMaterial() is used for the appearance of the
app buttons and tray buttons within the bounds of the shelf.

[Azure Wei, 2016-09-20 14:43:58]

https://codereview.chromium.org/2271523002/diff/100001/ash/common/system/chro...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://codereview.chromium.org/2271523002/diff/100001/ash/common/system/chro...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:30: #include
"ui/gfx/vector_icons_public.h"
On 2016/09/20 13:11:12, tdanderson wrote:
> nit: this is not needed since you're including vector_icons.h

This is needed by settings button.

https://codereview.chromium.org/2271523002/diff/100001/ash/common/system/chro...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:160: if
(!MaterialDesignController::IsShelfMaterial()) {
On 2016/09/20 13:11:12, tdanderson wrote:
> Since the changes here are for the buttons inside the IME menu, this should
> instead be checking IsSystemTrayMenuMaterial(). That's what we're using to
> develop the changes to the system menu, including the IME detailed view (which
> the opt-in menu shares the input method rows with).
> 
> Line 80 is fine as-is since IsShelfMaterial() is used for the appearance of
the
> app buttons and tray buttons within the bounds of the shelf.

Done.

[Azure Wei, 2016-09-20 14:44:05]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-09-20 14:44:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, tdanderson@chromium.org
Link to the patchset: https://codereview.chromium.org/2271523002/#ps120001
(title: "Addressed comments and sync.")

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

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-20 15:46:54]

Description was changed from

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons:
Emoji: ash/resources/vector_icons/ime_menu_emoticon.(1x.)icon
Voice: ash/resources/vector_icons/ime_menu_microphone.(1x.)icon
Handwriting: ash/resources/vector_icons/ime_menu_write.(1x.)icon

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304, 643305
TEST=Verified on local build.
==========

to

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons:
Emoji: ash/resources/vector_icons/ime_menu_emoticon.(1x.)icon
Voice: ash/resources/vector_icons/ime_menu_microphone.(1x.)icon
Handwriting: ash/resources/vector_icons/ime_menu_write.(1x.)icon

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304, 643305
TEST=Verified on local build.
==========

[commit-bot: I haz the power, 2016-09-20 15:46:55]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-20 15:49:12]

Description was changed from

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons:
Emoji: ash/resources/vector_icons/ime_menu_emoticon.(1x.)icon
Voice: ash/resources/vector_icons/ime_menu_microphone.(1x.)icon
Handwriting: ash/resources/vector_icons/ime_menu_write.(1x.)icon

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304, 643305
TEST=Verified on local build.
==========

to

==========
Adds emoji/handwriting/vocie buttons on opt-in IME menu.

1) Add the Vectorized icons for buttons:
Emoji: ash/resources/vector_icons/ime_menu_emoticon.(1x.)icon
Voice: ash/resources/vector_icons/ime_menu_microphone.(1x.)icon
Handwriting: ash/resources/vector_icons/ime_menu_write.(1x.)icon

2) Update the positions of the buttons: should be emoji-handwriting-voice
buttons.

3) Updates the UI based on spec:
https://folio.googleplex.com/cros-panel-ime/Previews%20and%20specs#%2FIME%20-...

BUG=642385, 643304, 643305
TEST=Verified on local build.

Committed: https://crrev.com/6a18628f8826ebc484f06fcfabcfc3db0dd9d508
Cr-Commit-Position: refs/heads/master@{#419775}
==========

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

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