Create new look of the avatar label.

Create new look of the avatar label.

The UI specs and the screenshot can be found in the bug
description.

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

[Adrian Kuegel, 2013-06-28 13:10:36]

Scott, can you please review the three files in chrome/browser/ui/views ? We got
the (hopefully final) specs for the supervised user avatar label, and this CL
does the first step towards these specs.

[sky, 2013-06-28 15:45:35]

LGTM

[Adrian Kuegel, 2013-07-01 16:58:39]

@Mitsuru: can you please review theme_resources.grd?
@Scott: I forgot to modify browser_non_client_frame_view_ash.cc and
glass_browser_frame_view.cc in my first patch. Can you please take a look at
them, too?

[oshima, 2013-07-01 17:10:16]

chrome/app/theme lgtm

[sky, 2013-07-01 20:50:52]

SLGTM

[Adrian Kuegel, 2013-07-03 16:38:53]

@Scott: I needed to do some more changes. Can you please take another look?
Before I didn't have a windows machine and could only test on Linux. Pam tested
it for me on windows and found out there were still some bugs on windows (and
helped me with the positioning of the label). Finally I also have a windows
machine and now I can test myself. Btw, how can I test the look of
browser_non_client_frame_view_ash? Is it only used in ChromeOs, or on Windows,
too?

[sky, 2013-07-03 23:17:30]

Have you tested all of this when rtl too?

https://codereview.chromium.org/18083014/diff/40001/chrome/browser/ui/views/a...
File chrome/browser/ui/views/avatar_label.cc (right):

https://codereview.chromium.org/18083014/diff/40001/chrome/browser/ui/views/a...
chrome/browser/ui/views/avatar_label.cc:65: if (font.GetFontSize() != 13)
Can't you instead align to the top so you don't have to force a size?

[Adrian Kuegel, 2013-07-05 14:22:23]

Thanks for pointing out the potential RTL issue. I just tested it, and it works
in the opaque browser, but not in the aero glass browser. This seems to be a
tricky issue to resolve, because in the aero glass browser we would probably
want to have the label on the right side of the icon, otherwise it would go too
far towards the middle and take up precious space. However, we actually don't
want to support the aero glass browser, because we want to have a default theme
for supervised users, and having a theme would make it use the opaque browser,
right?

https://codereview.chromium.org/18083014/diff/40001/chrome/browser/ui/views/a...
File chrome/browser/ui/views/avatar_label.cc (right):

https://codereview.chromium.org/18083014/diff/40001/chrome/browser/ui/views/a...
chrome/browser/ui/views/avatar_label.cc:65: if (font.GetFontSize() != 13)
On 2013/07/03 23:17:31, sky wrote:
> Can't you instead align to the top so you don't have to force a size?

In our specification we have two requirements: the label should align with the
top of the tab, and it should be 3 pixels above the line where the avatar icon
begins. So we need a fixed height, and the only two ways I see to get a fixed
height is either set the font size fixed, or depending on the font size change
the inset values. Would you prefer if I change the inset values instead?
I must agree I also don't really like the way it is right now, so if you know of
another method of selecting a font which has the same size on Linux and Windows,
I would be glad if you could tell me.

[Adrian Kuegel, 2013-07-05 14:24:03]

On 2013/07/05 14:22:23, Adrian Kuegel wrote:
> [...] because in the aero glass browser we would probably
> want to have the label on the right side of the icon

I forgot to mention that I am talking here about RTL mode. In LTR mode the label
would be on the right side of the icon, anyway.

[sky, 2013-07-08 14:32:43]

Why don't you use the same font as tabs use? Also, I think you're
right on the glass side. There is no need for the code there since it
should never be used, right?

  -Scott

On Fri, Jul 5, 2013 at 7:24 AM,  <akuegel@chromium.org> wrote:
> On 2013/07/05 14:22:23, Adrian Kuegel wrote:
>>
>> [...] because in the aero glass browser we would probably
>>
>> want to have the label on the right side of the icon
>
>
> I forgot to mention that I am talking here about RTL mode. In LTR mode the
> label
> would be on the right side of the icon, anyway.
>
> https://codereview.chromium.org/18083014/

[Adrian Kuegel, 2013-07-08 15:04:18]

On 2013/07/08 14:32:43, sky wrote:
> Why don't you use the same font as tabs use? Also, I think you're
> right on the glass side. There is no need for the code there since it
> should never be used, right?
> 
>   -Scott

Good point, I guess the UI designer intended that the font size should be the
same as for the tabs. So I will now be using just the BaseFont as it is also
done for the tabs. To make the alignment still work, I will then modify the
insets according to the actual font size.

[sky, 2013-07-08 15:07:47]

SG

On Mon, Jul 8, 2013 at 8:04 AM,  <akuegel@chromium.org> wrote:
> On 2013/07/08 14:32:43, sky wrote:
>>
>> Why don't you use the same font as tabs use? Also, I think you're
>> right on the glass side. There is no need for the code there since it
>> should never be used, right?
>
>
>>    -Scott
>
>
> Good point, I guess the UI designer intended that the font size should be
> the
> same as for the tabs. So I will now be using just the BaseFont as it is also
> done for the tabs. To make the alignment still work, I will then modify the
> insets according to the actual font size.
>
> https://codereview.chromium.org/18083014/

[Adrian Kuegel, 2013-07-08 15:59:51]

Ok, I finished this now, and removed the unnecessary code from glass browser and
browser_non_client_frame_view_ash. I also updated the bug description with the
most recent screenshot.
Sorry for the many iterations I needed for this CL, I hope this is the final one
:)

On 2013/07/08 15:07:47, sky wrote:
> SG
> 
> On Mon, Jul 8, 2013 at 8:04 AM,  <mailto:akuegel@chromium.org> wrote:
> > On 2013/07/08 14:32:43, sky wrote:
> >>
> >> Why don't you use the same font as tabs use? Also, I think you're
> >> right on the glass side. There is no need for the code there since it
> >> should never be used, right?
> >
> >
> >>    -Scott
> >
> >
> > Good point, I guess the UI designer intended that the font size should be
> > the
> > same as for the tabs. So I will now be using just the BaseFont as it is also
> > done for the tabs. To make the alignment still work, I will then modify the
> > insets according to the actual font size.
> >
> > https://codereview.chromium.org/18083014/

[sky, 2013-07-08 17:16:16]

LGTM

[commit-bot: I haz the power, 2013-07-09 07:55:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akuegel@chromium.org/18083014/53001

[commit-bot: I haz the power, 2013-07-09 14:17:57]

Change committed as 210559