OneClickSigninBubbleView: Update the bubble form to reflect new bubble/

OneClickSigninBubbleView: Update the bubble form to reflect new bubble/
dialog UI design paradigm [Mac/Views]:

- Add a title ("You're now signed in to Chromium!")
- Modify "OK" button to say "OK, got it!" instead
- Adjust margins and gaps according to spec.

XIB changes:
* Added a new NSTextField for the “You’re now signed in to Chromium” title.
* Made the entire bubble taller (167 height) and wider (350 width) to accommodate new UI spec and to make room for the title text field.
* Repositioned the Advanced link and the OK button in order to have 10 view units margin from the bottom and left/right edges (also for new UI spec).

Screenshots:
* Mac: http://i.imgur.com/qKWKwR4.png
* Views: http://i.imgur.com/6fYmi42.png

BUG=322593
R=thakis@chromium.org

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

[Nico, 2013-12-20 20:39:23]

Hi,

when changing xib files, please add a "xib changes" section that describes what
you changed, see
http://svnsearch.org/svnsearch/repos/CHROMIUM/search?logMessage=xib%20changes
for examples.

Including a link to a screenshot (on imgur.com or on the bug or whatevs) is
useful too.

(I'll look at the CL now :-) )

[Nico, 2013-12-20 20:42:18]

almost lgtm, one tiny change please:

https://codereview.chromium.org/107673002/diff/190001/chrome/app/chromium_str...
File chrome/app/chromium_strings.grd (right):

https://codereview.chromium.org/107673002/diff/190001/chrome/app/chromium_str...
chrome/app/chromium_strings.grd:874: Your bookmarks, history, and other settings
will be synced to your Google Account.
Since this no longer refers to Chromium / Google Chrome, it can now move to the
regular grd file, right?

[Oren Blasberg, 2013-12-20 22:51:27]

On 2013/12/20 20:42:18, Nico wrote:
> almost lgtm, one tiny change please:
> 
>
https://codereview.chromium.org/107673002/diff/190001/chrome/app/chromium_str...
> File chrome/app/chromium_strings.grd (right):
> 
>
https://codereview.chromium.org/107673002/diff/190001/chrome/app/chromium_str...
> chrome/app/chromium_strings.grd:874: Your bookmarks, history, and other
settings
> will be synced to your Google Account.
> Since this no longer refers to Chromium / Google Chrome, it can now move to
the
> regular grd file, right?

Thanks!
Done.

[Oren Blasberg, 2013-12-20 22:52:07]

https://codereview.chromium.org/107673002/diff/190001/chrome/app/chromium_str...
File chrome/app/chromium_strings.grd (right):

https://codereview.chromium.org/107673002/diff/190001/chrome/app/chromium_str...
chrome/app/chromium_strings.grd:874: Your bookmarks, history, and other settings
will be synced to your Google Account.
On 2013/12/20 20:42:18, Nico wrote:
> Since this no longer refers to Chromium / Google Chrome, it can now move to
the
> regular grd file, right?

Done.

[Nico, 2013-12-20 23:03:22]

lgtm, thanks!

(nit: xib files are in "view units" (following android parlance, we also say
"dips", not in "pixels")

[commit-bot: I haz the power, 2014-01-06 22:03:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/orenb@chromium.org/107673002/280001

[noms (inactive), 2014-01-06 22:22:23]

Some drive-by comments on the views code.

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
File chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc (right):

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:35: const int
kMinimumDialogLabelWidth = 400;
What's the difference between kMinBubbleWidth and kMinDialogLabelWidth? I see
that the dialog message label is sized to fit kMinDialogLabelWidth -- does this
still make sense? It's bigger than the min bubble width. Perhaps only one of the
two should be used.

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:37: const int
kBubbleMargin = 20;
You don't need to redefine this, you can just use kButtonVEdgeMarginNew (or
HEdge) from layout_constants.h

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:307:
(*ok_button)->SetText(ok_label);
You can move this (and above) to right under the ok_label definition, as it no
longer needs to be if/elsed

[Oren Blasberg, 2014-01-07 00:12:19]

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
File chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc (right):

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:35: const int
kMinimumDialogLabelWidth = 400;
On 2014/01/06 22:22:23, Monica Dinculescu wrote:
> What's the difference between kMinBubbleWidth and kMinDialogLabelWidth? I see
> that the dialog message label is sized to fit kMinDialogLabelWidth -- does
this
> still make sense? It's bigger than the min bubble width. Perhaps only one of
the
> two should be used.

The bubble and dialog modes of the view aren't necessarily supposed to have the
same widths -- I wanted to leave the dialog mode untouched, so I just added a
new constant for the bubble.
The kMinimumDialogLabelWidth constant is still used, but only when this view is
initialized as a dialog.

BTW, how do I get the dialog mode to show up from within the browser? It used to
be that if I signed in and intentionally mistyped my password then typed it
correctly the second time, it would show this mode. However that doesn't seem to
be working anymore.

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:37: const int
kBubbleMargin = 20;
On 2014/01/06 22:22:23, Monica Dinculescu wrote:
> You don't need to redefine this, you can just use kButtonVEdgeMarginNew (or
> HEdge) from layout_constants.h

Done.

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:307:
(*ok_button)->SetText(ok_label);
On 2014/01/06 22:22:23, Monica Dinculescu wrote:
> You can move this (and above) to right under the ok_label definition, as it no
> longer needs to be if/elsed

Ah. Duh. Thanks. :)

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:307:
(*ok_button)->SetText(ok_label);
On 2014/01/06 22:22:23, Monica Dinculescu wrote:
> You can move this (and above) to right under the ok_label definition, as it no
> longer needs to be if/elsed

Done.

[sky, 2014-01-07 21:39:13]

https://codereview.chromium.org/107673002/diff/500001/chrome/browser/ui/views...
File chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc (right):

https://codereview.chromium.org/107673002/diff/500001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:301:
(*ok_button)->SetText(undo_label);
Is this needed anymore? And can the text be passed to the constructor?

[Oren Blasberg, 2014-01-08 21:43:59]

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
File chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc (right):

https://codereview.chromium.org/107673002/diff/280001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:35: const int
kMinimumDialogLabelWidth = 400;
On 2014/01/07 00:12:20, Oren Blasberg wrote:
> On 2014/01/06 22:22:23, Monica Dinculescu wrote:
> > What's the difference between kMinBubbleWidth and kMinDialogLabelWidth? I
see
> > that the dialog message label is sized to fit kMinDialogLabelWidth -- does
> this
> > still make sense? It's bigger than the min bubble width. Perhaps only one of
> the
> > two should be used.
> 
> The bubble and dialog modes of the view aren't necessarily supposed to have
the
> same widths -- I wanted to leave the dialog mode untouched, so I just added a
> new constant for the bubble.
> The kMinimumDialogLabelWidth constant is still used, but only when this view
is
> initialized as a dialog.
> 
> BTW, how do I get the dialog mode to show up from within the browser? It used
to
> be that if I signed in and intentionally mistyped my password then typed it
> correctly the second time, it would show this mode. However that doesn't seem
to
> be working anymore.



As discussed offline I've moved the layout->set_minimum_size call into the
InitBubbleContent (so we only set the bubble width when in bubble mode).  This
is a no op change visually but makes the code more sensible. Thanks!

https://codereview.chromium.org/107673002/diff/500001/chrome/browser/ui/views...
File chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc (right):

https://codereview.chromium.org/107673002/diff/500001/chrome/browser/ui/views...
chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc:301:
(*ok_button)->SetText(undo_label);
On 2014/01/07 21:39:13, sky wrote:
> Is this needed anymore?

Apparently not, since things look fine without it. I simplified this.

> And can the text be passed to the constructor?

And yes, the label string can be passed to the constructor. Thanks!

Done.

[sky, 2014-01-08 23:36:52]

views LGTM

[commit-bot: I haz the power, 2014-01-08 23:41:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/orenb@chromium.org/107673002/590001

[commit-bot: I haz the power, 2014-01-09 02:16:30]

Change committed as 243744