ash: Move "CAPS LOCK is on." bubble to the notification center.

ash: Move "CAPS LOCK is on." bubble to the notification center.

When caps lock is enabled with Alt+Search, "CAPS LOCK is on." bubble
pops up. However, the bubble was not using the notification center.

BUG=686203
TEST=manually tested.

Review-Url: https://codereview.chromium.org/2820443002
Cr-Commit-Position: refs/heads/master@{#464659}
Committed: https://chromium.googlesource.com/chromium/src/+/3f0af475fd6e46a14e6a33cc538e5b24c86ab803

[tetsui2, 2017-04-13 04:54:31]

tetsui@google.com changed reviewers:
+ fukino@chromium.org

[fukino, 2017-04-13 07:27:37]

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

[fukino, 2017-04-13 07:27:38]

Looking good to me.
Let me defer to tdanderson@ for an OWNER's review.

[tetsui2, 2017-04-13 07:28:28]

Thank you for reviewing!

[tdanderson, 2017-04-13 17:01:38]

Thanks for doing this! LGTM with a few comments below.

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.cc
File ash/system/tray_caps_lock.cc (right):

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.c...
ash/system/tray_caps_lock.cc:58:
TrayPopupItemStyle::FontStyle::DEFAULT_VIEW_LABEL);
Thanks for making use of TrayPopupItemStyle. However since you are not applying
this style to any labels, I don't think you need to instantiate
|default_view_style|. Instead for the color parameter on line 67, just say:

TrayPopupItemStyle::GetIconColor(TrayPopupItemStyle::ColorStyle::ACTIVE)

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.c...
ash/system/tray_caps_lock.cc:71: message_center::RichNotificationData(), NULL));
nit: nullptr

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.c...
ash/system/tray_caps_lock.cc:170: // detailed_(nullptr),
I think you meant to delete this :)

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.c...
ash/system/tray_caps_lock.cc:209: } else {
nit: can you just make this an 'else if' here?

[tetsui2, 2017-04-14 01:54:53]

Thank you!

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.cc
File ash/system/tray_caps_lock.cc (right):

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.c...
ash/system/tray_caps_lock.cc:58:
TrayPopupItemStyle::FontStyle::DEFAULT_VIEW_LABEL);
On 2017/04/13 17:01:37, tdanderson wrote:
> Thanks for making use of TrayPopupItemStyle. However since you are not
applying
> this style to any labels, I don't think you need to instantiate
> |default_view_style|. Instead for the color parameter on line 67, just say:
> 
> TrayPopupItemStyle::GetIconColor(TrayPopupItemStyle::ColorStyle::ACTIVE)

Done.

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.c...
ash/system/tray_caps_lock.cc:71: message_center::RichNotificationData(), NULL));
On 2017/04/13 17:01:37, tdanderson wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.c...
ash/system/tray_caps_lock.cc:170: // detailed_(nullptr),
On 2017/04/13 17:01:37, tdanderson wrote:
> I think you meant to delete this :)

Done.

https://codereview.chromium.org/2820443002/diff/1/ash/system/tray_caps_lock.c...
ash/system/tray_caps_lock.cc:209: } else {
On 2017/04/13 17:01:37, tdanderson wrote:
> nit: can you just make this an 'else if' here?

Done.

[tetsui2, 2017-04-14 01:55:33]

The CQ bit was checked by tetsui@google.com

[tetsui2, 2017-04-14 01:55:34]

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/2820443002/#ps20001
(title: "Address review comments.")

[commit-bot: I haz the power, 2017-04-14 01:56:08]

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, 2017-04-14 02:30:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492134933953240,
"parent_rev": "b6c55a8e1595e5d65760be3adba1e26e0e03e95e", "commit_rev":
"3f0af475fd6e46a14e6a33cc538e5b24c86ab803"}

[commit-bot: I haz the power, 2017-04-14 02:31:38]

Description was changed from

==========
ash: Move "CAPS LOCK is on." bubble to the notification center.

When caps lock is enabled with Alt+Search, "CAPS LOCK is on." bubble
pops up. However, the bubble was not using the notification center.

BUG=686203
TEST=manually tested.
==========

to

==========
ash: Move "CAPS LOCK is on." bubble to the notification center.

When caps lock is enabled with Alt+Search, "CAPS LOCK is on." bubble
pops up. However, the bubble was not using the notification center.

BUG=686203
TEST=manually tested.

Review-Url: https://codereview.chromium.org/2820443002
Cr-Commit-Position: refs/heads/master@{#464659}
Committed:
https://chromium.googlesource.com/chromium/src/+/3f0af475fd6e46a14e6a33cc538e...
==========

[commit-bot: I haz the power, 2017-04-14 02:31:40]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/3f0af475fd6e46a14e6a33cc538e...