[Mac][MD User Menu] UI Tweaks

chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm

Index: chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
index 2b8dbf2e385f24fcc263f5e6a2abb4d259376a46..ab50cfdbbc7e6e75a2e18c7e20b495eef58f4e98 100644
--- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
+++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
@@ -571,14 +571,12 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
         [[CustomCircleImageCell alloc] init]);
     [self setCell:cell.get()];
 
-    const int imageSide =
-        switches::IsMaterialDesignUserMenu() ? kMdImageSide : kLargeImageSide;
-    [self setDefaultImage:CreateProfileImage(profileIcon, imageSide,
-                                             profiles::SHAPE_CIRCLE)];
+    [self setDefaultImage:CreateProfileImage(profileIcon, kLargeImageSide,
+                                             profiles::SHAPE_SQUARE)];

[groby-ooo-7-16, 2016/08/03 17:23:09]

I'm confused - the "after" images still show a round avatar? (Also, did we get
rid of the MD menu?)

[Jane, 2016/08/08 19:22:24]

Yes it's still a round avatar, and no we are keeping the MD menu (I mean, I'm
currently implementing it).
Explanation:
This class "EditableProfilePhoto" is actually for non-MD user menu only (the old
menu); the "kMdImageSide" that wrongly appeared here probably came from my draft
code before and didn't get cleaned up...
This class displays a round avatar by using a circular CustomCircleImageCell.
However, since profiles::GetSizedAvatarIcon can now directly return a round
avatar, in the last CL, I changed the function call to be SHAPE_CIRCLE for the
sole purpose of having more straightforward code.
However, it turns out that SHAPE_CIRCLE causes the camera icon overlay to have
pixelated border... That's why I'm switching back to SHAPE_SQUARE for now. The
avatar would still be round thanks to the cell.
It probably sounds weird and confusing, but at least we will be removing this
class later once MD menu is ready.

     [self setImagePosition:NSImageOnly];
 
     if (editingAllowed) {
-      NSRect bounds = NSMakeRect(0, 0, imageSide, imageSide);
+      NSRect bounds = NSMakeRect(0, 0, kLargeImageSide, kLargeImageSide);
       [self setTarget:self];
       [self setAction:@selector(editPhoto:)];
       changePhotoImage_.reset([[TransparentBackgroundImageView alloc]
@@ -1912,20 +1910,38 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   // Profile card button that contains the profile icon, name, and username.
   NSRect rect = NSMakeRect(0, yOffset, GetFixedMenuWidth(),
                            kMdImageSide + kVerticalSpacing);
-  NSButton* profileCard = [self hoverButtonWithRect:rect
-                                               text:[[NSString alloc] init]
-                                             action:@selector(editProfile:)];
+  NSButton* profileCard =
+      [self hoverButtonWithRect:rect
+                           text:[[NSString alloc] init]
+                          image:CreateProfileImage(item.icon, kMdImageSide,
+                                                   profiles::SHAPE_CIRCLE)
+                         action:@selector(editProfile:)];
+  [[profileCard cell] setImageDimsWhenDisabled:NO];

[groby-ooo-7-16, 2016/08/03 17:23:09]

How will the user tell it's disabled?

[Jane, 2016/08/08 19:22:24]

Double checked with UI folks about this design:
"The button has an informative (showing the avatar) function as well as being a
link to settings, so it's not "disabled" in the same sense even when it's not
linking to anything."
Hope this makes sense!

[groby-ooo-7-16, 2016/08/10 20:01:51]

Mumblegrumble UI should always indicate if there is an action behind it or not.
I get that we don't want to mess with the Avatar picture, but could you ping UI
for feedback on that (outside of this CL)?

[Jane, 2016/08/10 22:42:57]

I will bring this up for UI review (coming soon)! Thanks.

   [container addSubview:profileCard];
   if (isGuestSession_)
     [profileCard setEnabled:NO];
 
-  // Profile icon, left-aligned.
-  base::scoped_nsobject<NSImageView> iconView([[NSImageView alloc]
-      initWithFrame:NSMakeRect(xOffset, cardYOffset, kMdImageSide,
-                               kMdImageSide)]);
-  [iconView setImage:CreateProfileImage(item.icon, kMdImageSide,
-                                        profiles::SHAPE_CIRCLE)];
-  [profileCard addSubview:iconView];
+  // Profile badge for supervised account.
+  if (browser_->profile()->IsSupervised()) {
+    base::scoped_nsobject<NSImageView> supervisedIcon(
+        [[NSImageView alloc] initWithFrame:NSZeroRect]);
+    // TODO(janeliulwq): Replace the following two profile badge icons with
+    // smaller versions of them (24 x 24) to adapt to smaller profile icons.

[groby-ooo-7-16, 2016/08/03 17:23:09]

Note (not this CL): You probably want SVG images here.

[Jane, 2016/08/08 19:22:24]

Yes, I will be using SVG images! :) I'm just still waiting for new icon
resources to be made.

+    int imageId = browser_->profile()->IsChild()
+                      ? IDR_ICON_PROFILES_MENU_CHILD
+                      : IDR_ICON_PROFILES_MENU_LEGACY_SUPERVISED;
+    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+    [supervisedIcon setImage:rb.GetNativeImageNamed(imageId).ToNSImage()];
+
+    NSSize size = [[supervisedIcon image] size];
+    [supervisedIcon setFrameSize:size];
+    const int badgeSpacing = 4;
+    [supervisedIcon setFrameOrigin:NSMakePoint(xOffset + kMdImageSide -
+                                                   size.width + badgeSpacing,
+                                               cardYOffset + kMdImageSide -
+                                                   size.height + badgeSpacing)];
+    [profileCard addSubview:supervisedIcon];
+  }
 
   // Profile name, left-aligned to the right of profile icon.
   xOffset += kMdImageSide + kHorizontalSpacing;
@@ -1936,12 +1952,14 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
       NSZeroPoint, nil);
   [[profileName cell] setLineBreakMode:NSLineBreakByTruncatingTail];
   [profileName setFont:[NSFont labelFontOfSize:fontSize]];
+  [profileName sizeToFit];
+  // Adding 1px in height to avoid cutting off certain letters on the bottom.

[groby-ooo-7-16, 2016/08/03 17:23:09]

That's weird - any chance the letters are cut off because the offset was at a
half-pixel? (You might want to use std::ceil/floor for any arithmetic on coords
that's potentially non-Integer)

[Jane, 2016/08/08 19:22:24]

Woah, yes that's exactly why! Changed it to std::floor instead of having a weird
"1". Thanks!

[groby-ooo-7-16, 2016/08/10 20:01:51]

My pleasure. You have now passed the std::ceil rite-of-passage ;) (It literally
happens to pretty much everybody laying out Cocoa things the first time they get
non-integer coords. Your comment helped me realize that's what was happening!)

   [profileName
       setFrame:NSMakeRect(
                    xOffset,
                    cardYOffset +
-                       (kMdImageSide - [profileName frame].size.height) / 2,
-                   availableTextWidth, [profileName frame].size.height)];
+                       (kMdImageSide - [profileName frame].size.height - 1) / 2,
+                   availableTextWidth, [profileName frame].size.height + 1)];
   [profileCard addSubview:profileName];
 
   // Username, left-aligned to the right of profile icon and below the profile
@@ -1958,29 +1976,6 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
     [profileCard addSubview:username];
   }
 
-  // Profile badge for supervised account.
-  if (browser_->profile()->IsSupervised()) {
-    base::scoped_nsobject<NSImageView> supervisedIcon(
-        [[NSImageView alloc] initWithFrame:NSZeroRect]);
-    // TODO(janeliulwq): Replace the following two profile badge icons with
-    // smaller versions of them (24 x 24) to adapt to smaller profile icons.
-    int imageId = browser_->profile()->IsChild()
-                      ? IDR_ICON_PROFILES_MENU_CHILD
-                      : IDR_ICON_PROFILES_MENU_LEGACY_SUPERVISED;
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    [supervisedIcon setImage:rb.GetNativeImageNamed(imageId).ToNSImage()];
-
-    NSSize size = [[supervisedIcon image] size];
-    [supervisedIcon setFrameSize:size];
-    NSRect profileIconFrame = [iconView frame];
-    const int badgeSpacing = 4;
-    [supervisedIcon setFrameOrigin:NSMakePoint(NSMaxX(profileIconFrame) -
-                                                   size.width + badgeSpacing,
-                                               NSMaxY(profileIconFrame) -
-                                                   size.height + badgeSpacing)];
-    [profileCard addSubview:supervisedIcon];
-  }
-
   yOffset = NSMaxY([profileCard frame]);
   [container setFrameSize:NSMakeSize(GetFixedMenuWidth(), yOffset)];
   return container.autorelease();