Update the new avatar menu

Update the new avatar menu

1. stack the link and button in the upgrade tutorial card
2. limit the width of the title label to accomodate long names.
3. increase the height of signin view
4. adds missing 'x' button

BUG=403572, 399663, 403105
R=asvitkine@chromium.org, sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290073

[guohui, 2014-08-15 13:23:29]

Hey,

could you please review the CL?

Thanks,

Hui

[guohui, 2014-08-15 18:02:25]

+alexei for mac implementation.

[Alexei Svitkine (slow), 2014-08-15 18:44:30]

lgtm

https://codereview.chromium.org/465313003/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/465313003/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:733:
stackButton:(bool)stackButton
Nit: Use Obj-C type in Obj-C code - BOOL (all uppercase), with values YES/NO.

[guohui, 2014-08-15 18:47:21]

https://codereview.chromium.org/465313003/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/465313003/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:733:
stackButton:(bool)stackButton
On 2014/08/15 18:44:30, Alexei Svitkine wrote:
> Nit: Use Obj-C type in Obj-C code - BOOL (all uppercase), with values YES/NO.

Done.

[Alexei Svitkine (slow), 2014-08-15 18:51:13]

lgtm

https://codereview.chromium.org/465313003/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/465313003/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:724: //
|buttonMessage|. If |stackButton| is true, places the button above the link.
Nit: "is true" -> "is YES"

[guohui, 2014-08-15 18:53:42]

https://codereview.chromium.org/465313003/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/465313003/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:724: //
|buttonMessage|. If |stackButton| is true, places the button above the link.
On 2014/08/15 18:51:13, Alexei Svitkine wrote:
> Nit: "is true" -> "is YES"

Done.

[sky, 2014-08-15 19:07:58]

https://codereview.chromium.org/465313003/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/465313003/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:433: int
back_button_width = back_button_->GetPreferredSize().width();
What if back_button_width > bounds().width()? Shouldn't you cap width to width
of this?

[guohui, 2014-08-15 19:19:23]

https://codereview.chromium.org/465313003/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/465313003/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:433: int
back_button_width = back_button_->GetPreferredSize().width();
On 2014/08/15 19:07:58, sky wrote:
> What if back_button_width > bounds().width()? Shouldn't you cap width to width
> of this?

back button shows a static icon, so the width is fixed based on resolution, and
it is far less than any view we show.  Also view framework does handle negative
bounds by overwriting it with a minimum size.

[sky, 2014-08-15 19:29:53]

On 2014/08/15 19:19:23, guohui wrote:
>
https://codereview.chromium.org/465313003/diff/100001/chrome/browser/ui/views...
> File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):
> 
>
https://codereview.chromium.org/465313003/diff/100001/chrome/browser/ui/views...
> chrome/browser/ui/views/profiles/profile_chooser_view.cc:433: int
> back_button_width = back_button_->GetPreferredSize().width();
> On 2014/08/15 19:07:58, sky wrote:
> > What if back_button_width > bounds().width()? Shouldn't you cap width to
width
> > of this?
> 
> back button shows a static icon, so the width is fixed based on resolution,
and
> it is far less than any view we show.  Also view framework does handle
negative
> bounds by overwriting it with a minimum size.

It's not negative bounds, it's that you'll get clipping. But if you say that is
ok, LGTM

[guohui, 2014-08-15 19:40:53]

yes we will have clipping, but if that ever happens not sure what else we could
do, i ll add a DCHECK,

Thanks!

[guohui, 2014-08-15 20:07:01]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-08-15 20:12:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/guohui@chromium.org/465313003/120001

[guohui, 2014-08-15 21:38:14]

The CQ bit was unchecked by guohui@chromium.org

[guohui, 2014-08-16 00:04:36]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-08-16 00:06:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/guohui@chromium.org/465313003/180001

[guohui, 2014-08-16 00:18:15]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-08-16 00:20:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/guohui@chromium.org/465313003/200001

[guohui, 2014-08-16 00:58:55]

Committed patchset #7 manually as 290073 (presubmit successful).