CrOS MD Menu - Some more user row tweaks.

CrOS MD Menu - Some more user row tweaks.

As per Sebastien,
- more padding for "No more users" case.
- update text styles for both "Add user" and "No more users".

Also,
- fix active user name font weight (Medium->Normal)
- don't make the "no more users" label a button (it already didn't do
  anything when you clicked it, but there's no sense in making it a
  button).

BUG=667942
Committed: https://crrev.com/2029f37011b0506fc0d824b9d62f63b940048cec
Cr-Commit-Position: refs/heads/master@{#434874}

[Evan Stade, 2016-11-28 18:31:01]

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

[tdanderson, 2016-11-28 20:55:30]

LGTM

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
File ash/common/system/user/user_card_view.cc (right):

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
ash/common/system/user/user_card_view.cc:507: nullptr,
TrayPopupItemStyle::FontStyle::DEFAULT_VIEW_LABEL);
nit: Use the single-parameter TPIS constructor.

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
File ash/common/system/user/user_view.cc (right):

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
ash/common/system/user/user_view.cc:9: 
Also specified in the bug is "'Sign-out' left and right padding should be 20dp"
- was that fixed in another CL / are you fixing that in a follow-up CL?

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
ash/common/system/user/user_view.cc:154: vertical_padding +=
kMenuSeparatorVerticalPadding;
From the bug, Sebastien has specified "the entire additional row ("sign in
another user" or "All available...) need to be moved down 8dp" but here it looks
like you're giving 4dp of extra padding for "Sign in another user" but 8dp extra
padding to "All available...". Just wanted to double check this is intentional.

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
ash/common/system/user/user_view.cc:167: return button;
Thanks for making this a button only when it needs to be.

[Evan Stade, 2016-11-29 02:30:33]

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
File ash/common/system/user/user_card_view.cc (right):

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
ash/common/system/user/user_card_view.cc:507: nullptr,
TrayPopupItemStyle::FontStyle::DEFAULT_VIEW_LABEL);
On 2016/11/28 20:55:30, tdanderson wrote:
> nit: Use the single-parameter TPIS constructor.

Done.

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
File ash/common/system/user/user_view.cc (right):

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
ash/common/system/user/user_view.cc:9: 
On 2016/11/28 20:55:30, tdanderson wrote:
> Also specified in the bug is "'Sign-out' left and right padding should be
20dp"
> - was that fixed in another CL / are you fixing that in a follow-up CL?

I didn't actually notice that part of the bug --- guess it wasn't part of the
email thread? The change to the user name text was something I noticed
incidentally when trying to figure out the font styles for the add user label.

For some reason I changed the sign out padding from 20 to 8 in
https://codereview.chromium.org/2498293003 but I can't dig up the reasoning for
why I did that, so I've reverted that change (and fixed the height of the button
as well).

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
ash/common/system/user/user_view.cc:154: vertical_padding +=
kMenuSeparatorVerticalPadding;
On 2016/11/28 20:55:30, tdanderson wrote:
> From the bug, Sebastien has specified "the entire additional row ("sign in
> another user" or "All available...) need to be moved down 8dp" but here it
looks
> like you're giving 4dp of extra padding for "Sign in another user" but 8dp
extra
> padding to "All available...". Just wanted to double check this is
intentional.

I added 4 with add_user_padding (because that 4 is not supposed to be part of
the ripple). I added 4 here for "sign in another", making 8. I added 4 more top
and bottom for "all available" which is supposed to be 8dp taller (not
explicitly mentioned in the bug, but this is where the previous 56 came from).

https://codereview.chromium.org/2535003002/diff/1/ash/common/system/user/user...
ash/common/system/user/user_view.cc:167: return button;
On 2016/11/28 20:55:30, tdanderson wrote:
> Thanks for making this a button only when it needs to be.

Acknowledged.

[Evan Stade, 2016-11-29 02:30:46]

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

[commit-bot: I haz the power, 2016-11-29 02:31:34]

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

[Evan Stade, 2016-11-29 02:33:47]

The CQ bit was unchecked by estade@chromium.org

[Evan Stade, 2016-11-29 02:33:59]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-11-29 02:33:59]

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/2535003002/#ps20001
(title: "sign out button improvements")

[commit-bot: I haz the power, 2016-11-29 02:34:51]

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-11-29 03:49:25]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480386839526550,
"parent_rev": "2ab93925b61373d731eebd7c8fe76493f9d55af8", "commit_rev":
"ee37595b556177cedff75b1f7b21aeb7a2c9e6c8"}

[commit-bot: I haz the power, 2016-11-29 03:49:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-29 03:51:39]

Description was changed from

==========
CrOS MD Menu - Some more user row tweaks.

As per Sebastien,
- more padding for "No more users" case.
- update text styles for both "Add user" and "No more users".

Also,
- fix active user name font weight (Medium->Normal)
- don't make the "no more users" label a button (it already didn't do
  anything when you clicked it, but there's no sense in making it a
  button).

BUG=667942
==========

to

==========
CrOS MD Menu - Some more user row tweaks.

As per Sebastien,
- more padding for "No more users" case.
- update text styles for both "Add user" and "No more users".

Also,
- fix active user name font weight (Medium->Normal)
- don't make the "no more users" label a button (it already didn't do
  anything when you clicked it, but there's no sense in making it a
  button).

BUG=667942

Committed: https://crrev.com/2029f37011b0506fc0d824b9d62f63b940048cec
Cr-Commit-Position: refs/heads/master@{#434874}
==========

[commit-bot: I haz the power, 2016-11-29 03:51:40]

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