[Mac][MD User Menu] UI Tweaks

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/profiles/profile_chooser_controller.h"
 
 #import <Carbon/Carbon.h>  // kVK_Return.
 #import <Cocoa/Cocoa.h>
 #include <stddef.h>
 
@@ skipping 553 matching lines @@
   if ((self = [super initWithFrame:frameRect])) {
     avatarMenu_ = avatarMenu;
     controller_ = controller;
 
     [self setBordered:NO];
 
     base::scoped_nsobject<CustomCircleImageCell> cell(
         [[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]
           initWithFrame:bounds]);
       [changePhotoImage_ setImage:ui::ResourceBundle::GetSharedInstance().
           GetNativeImageNamed(IDR_ICON_PROFILES_EDIT_CAMERA).AsNSImage()];
       [self addSubview:changePhotoImage_];
 
       // Hide the image until the button is hovered over.
       [changePhotoImage_ setHidden:YES];
@@ skipping 1313 matching lines @@
   if (!NSIsEmptyRect(profileLinksBound)) {
     NSView* linksContainer =
         [self createCurrentProfileLinksForItem:item rect:profileLinksBound];
     [container addSubview:linksContainer];
     yOffset = NSMaxY([linksContainer frame]);
   }
 
   // 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;
   CGFloat fontSize = kTextFontSize + 1.0;
   NSTextField* profileName = BuildLabel(
       base::SysUTF16ToNSString(
           profiles::GetAvatarNameForProfile(browser_->profile()->GetPath())),
       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
   // name.
   if (item.signed_in && !switches::IsEnableAccountConsistency()) {
     // Adjust the y-position of profile name to leave space for username.
     cardYOffset += kMdImageSide / 2 - [profileName frame].size.height;
     [profileName setFrameOrigin:NSMakePoint(xOffset, cardYOffset)];
 
     NSTextField* username = BuildLabel(
         ElideEmail(base::UTF16ToUTF8(item.username), availableTextWidth),
         NSZeroPoint, skia::SkColorToSRGBNSColor(SK_ColorGRAY));
     [username setFrameOrigin:NSMakePoint(xOffset, NSMaxY([profileName frame]))];
     [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();
 }
 
 - (NSView*)createCurrentProfileLinksForItem:(const AvatarMenu::Item&)item
                                        rect:(NSRect)rect {
   if (switches::IsMaterialDesignUserMenu()) {
     // The branch is empty for material design user menu in non-account-
     // consistency mode, because in that case, the username would appear in the
@@ skipping 760 matching lines @@
 }
 
 - (bool)shouldShowGoIncognito {
   bool incognitoAvailable =
       IncognitoModePrefs::GetAvailability(browser_->profile()->GetPrefs()) !=
           IncognitoModePrefs::DISABLED;
   return incognitoAvailable && !browser_->profile()->IsGuestSession();
 }
 
 @end