From aaacc3030b8b46e4d0463d6d51f24341bf51ddfa Mon Sep 17 00:00:00 2001 From: noms Date: Thu, 28 Aug 2014 13:41:07 -0700 Subject: [PATCH] [Mac] Add tab and keyboard navigation to the new avatar bubble Tabbing should navigate through all the visible controls. I've changed the EditablePhotoButton to actually be a button and not just an image, so that tabbing and pressing enter actually works. BUG=405056 Review URL: https://codereview.chromium.org/489143002 Cr-Commit-Position: refs/heads/master@{#292455} --- .../profiles/profile_chooser_controller.mm | 172 ++++++++++-------- .../profile_chooser_controller_unittest.mm | 24 +-- 2 files changed, 110 insertions(+), 86 deletions(-) diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm index 3cecab24012d9..a4e8624e8adf5 100644 --- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm +++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm @@ -85,6 +85,7 @@ const int kBezelThickness = 3; // Width of the bezel on an NSButton. const int kImageTitleSpacing = 10; const int kBlueButtonHeight = 30; +const CGFloat kFocusRingLineWidth = 2; // Fixed size for embedded sign in pages as defined in Gaia. const CGFloat kFixedGaiaViewWidth = 360; @@ -410,22 +411,34 @@ - (NSSize)cellSize { return buttonSize; } -@end +- (NSFocusRingType)focusRingType { + // This is taken care of by the custom drawing code. + return NSFocusRingTypeNone; +} -// A custom button that has a transparent backround. -@interface TransparentBackgroundButton : NSButton -@end +- (void)drawWithFrame:(NSRect)frame inView:(NSView *)controlView { + [super drawInteriorWithFrame:frame inView:controlView]; -@implementation TransparentBackgroundButton -- (id)initWithFrame:(NSRect)frameRect { - if ((self = [super initWithFrame:frameRect])) { - [self setBordered:NO]; - [self setFont:[NSFont labelFontOfSize:kTextFontSize]]; - [self setButtonType:NSMomentaryChangeButton]; + // Focus ring. + if ([self showsFirstResponder]) { + NSRect focusRingRect = + NSInsetRect(frame, kFocusRingLineWidth, kFocusRingLineWidth); + // TODO(noms): When we are targetting 10.7, we should change this to use + // -drawFocusRingMaskWithFrame instead. + [[[NSColor keyboardFocusIndicatorColor] colorWithAlphaComponent:1] set]; + NSBezierPath* path = [NSBezierPath bezierPathWithRect:focusRingRect]; + [path setLineWidth:kFocusRingLineWidth]; + [path stroke]; } - return self; } +@end + +// A custom image view that has a transparent backround. +@interface TransparentBackgroundImageView : NSImageView +@end + +@implementation TransparentBackgroundImageView - (void)drawRect:(NSRect)dirtyRect { NSColor* backgroundColor = [NSColor colorWithCalibratedWhite:1 alpha:0.6f]; [backgroundColor setFill]; @@ -434,13 +447,31 @@ - (void)drawRect:(NSRect)dirtyRect { } @end +@interface CustomCircleImageCell : NSButtonCell +@end + +@implementation CustomCircleImageCell +- (void)drawWithFrame:(NSRect)frame inView:(NSView *)controlView { + // Display everything as a circle that spans the entire control. + NSBezierPath* path = [NSBezierPath bezierPathWithOvalInRect:frame]; + [path addClip]; + + [super drawImage:[self image] withFrame:frame inView:controlView]; + + // Focus ring. + if ([self showsFirstResponder]) { + [[[NSColor keyboardFocusIndicatorColor] colorWithAlphaComponent:1] set]; + [path setLineWidth:kFocusRingLineWidth]; + [path stroke]; + } +} +@end + // A custom image control that shows a "Change" button when moused over. -@interface EditableProfilePhoto : NSImageView { +@interface EditableProfilePhoto : HoverImageButton { @private AvatarMenu* avatarMenu_; // Weak; Owned by ProfileChooserController. - base::scoped_nsobject changePhotoButton_; - // Used to display the "Change" button on hover. - ui::ScopedCrTrackingArea trackingArea_; + base::scoped_nsobject changePhotoImage_; ProfileChooserController* controller_; } @@ -453,16 +484,6 @@ - (id)initWithFrame:(NSRect)frameRect // Called when the "Change" button is clicked. - (void)editPhoto:(id)sender; -// When hovering over the profile photo, show the "Change" button. -- (void)mouseEntered:(NSEvent*)event; - -// When hovering away from the profile photo, hide the "Change" button. -- (void)mouseExited:(NSEvent*)event; -@end - -@interface EditableProfilePhoto (Private) -// Create the "Change" avatar photo button. -- (TransparentBackgroundButton*)changePhotoButtonWithRect:(NSRect)rect; @end @implementation EditableProfilePhoto @@ -474,24 +495,29 @@ - (id)initWithFrame:(NSRect)frameRect if ((self = [super initWithFrame:frameRect])) { avatarMenu_ = avatarMenu; controller_ = controller; - [self setImage:CreateProfileImage( - profileIcon, kLargeImageSide).ToNSImage()]; - // Add a tracking area so that we can show/hide the button when hovering. - trackingArea_.reset([[CrTrackingArea alloc] - initWithRect:[self bounds] - options:NSTrackingMouseEnteredAndExited | NSTrackingActiveAlways - owner:self - userInfo:nil]); - [self addTrackingArea:trackingArea_.get()]; + [self setBordered:NO]; + + base::scoped_nsobject cell( + [[CustomCircleImageCell alloc] init]); + [self setCell:cell.get()]; + + [self setDefaultImage:CreateProfileImage( + profileIcon, kLargeImageSide).ToNSImage()]; + [self setImagePosition:NSImageOnly]; NSRect bounds = NSMakeRect(0, 0, kLargeImageSide, kLargeImageSide); if (editingAllowed) { - changePhotoButton_.reset([self changePhotoButtonWithRect:bounds]); - [self addSubview:changePhotoButton_]; - - // Hide the button until the image is hovered over. - [changePhotoButton_ setHidden:YES]; + [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]; } // Set the image cell's accessibility strings to be the same as the @@ -523,36 +549,15 @@ - (id)initWithFrame:(NSRect)frameRect return self; } -- (void)drawRect:(NSRect)dirtyRect { - NSRect bounds = [self bounds]; - - // Display the profile picture as a circle. - NSBezierPath* path = [NSBezierPath bezierPathWithOvalInRect:bounds]; - [path addClip]; - [self.image drawAtPoint:bounds.origin - fromRect:bounds - operation:NSCompositeSourceOver - fraction:1.0]; - -} - - (void)editPhoto:(id)sender { avatarMenu_->EditProfile(avatarMenu_->GetActiveProfileIndex()); [controller_ postActionPerformed:ProfileMetrics::PROFILE_DESKTOP_MENU_EDIT_IMAGE]; } -- (void)mouseEntered:(NSEvent*)event { - [changePhotoButton_ setHidden:NO]; -} - -- (void)mouseExited:(NSEvent*)event { - [changePhotoButton_ setHidden:YES]; -} - -// Make sure the element is focusable for accessibility. -- (BOOL)canBecomeKeyView { - return YES; +- (void)setHoverState:(HoverState)state { + [super setHoverState:state]; + [changePhotoImage_ setHidden:([self hoverState] == kHoverStateNone)]; } - (BOOL)accessibilityIsIgnored { @@ -572,16 +577,6 @@ - (void)accessibilityPerformAction:(NSString*)action { [super accessibilityPerformAction:action]; } -- (TransparentBackgroundButton*)changePhotoButtonWithRect:(NSRect)rect { - TransparentBackgroundButton* button = - [[TransparentBackgroundButton alloc] initWithFrame:rect]; - [button setImage:ui::ResourceBundle::GetSharedInstance().GetNativeImageNamed( - IDR_ICON_PROFILES_EDIT_CAMERA).AsNSImage()]; - [button setImagePosition:NSImageOnly]; - [button setTarget:self]; - [button setAction:@selector(editPhoto:)]; - return button; -} @end // A custom text control that turns into a textfield for editing when clicked. @@ -773,6 +768,26 @@ - (void)drawRect:(NSRect)dirtyRect { } @end +// A custom dummy button that is used to clear focus from the bubble's controls. +@interface DummyWindowFocusButton : NSButton +@end + +@implementation DummyWindowFocusButton +// Ignore accessibility, as this is a placeholder button. +- (BOOL)accessibilityIsIgnored { + return YES; +} + +- (id)accessibilityAttributeValue:(NSString*)attribute { + return @[]; +} + +- (BOOL)canBecomeKeyView { + return false; +} + +@end + @interface ProfileChooserController () // Builds the profile chooser view. - (NSView*)buildProfileChooserView; @@ -1127,7 +1142,15 @@ - (void)initMenuContentsWithView:(profiles::BubbleViewMode)viewToDisplay { if (viewMode_ != profiles::BUBBLE_VIEW_MODE_PROFILE_CHOOSER) tutorialMode_ = profiles::TUTORIAL_MODE_NONE; + // Add a dummy, empty element so that we don't initially display any + // focus rings. + NSButton* dummyFocusButton = + [[[DummyWindowFocusButton alloc] initWithFrame:NSZeroRect] autorelease]; + [dummyFocusButton setNextKeyView:subView]; + [[self window] makeFirstResponder:dummyFocusButton]; + [contentView addSubview:subView]; + [contentView addSubview:dummyFocusButton]; SetWindowSize([self window], NSMakeSize(NSWidth([subView frame]), NSHeight([subView frame]))); } @@ -1671,7 +1694,8 @@ - (NSView*)createGuestProfileView { - (NSButton*)createOtherProfileView:(int)itemIndex { const AvatarMenu::Item& item = avatarMenu_->GetItemAt(itemIndex); - NSRect rect = NSMakeRect(0, 0, kFixedMenuWidth, kBlueButtonHeight); + NSRect rect = NSMakeRect( + 0, 0, kFixedMenuWidth, kBlueButtonHeight + kSmallVerticalSpacing); base::scoped_nsobject profileButton( [[BackgroundColorHoverButton alloc] initWithFrame:rect diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm index 00bfd891fc133..0c7e58610e07d 100644 --- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm @@ -107,7 +107,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; // Three profiles means we should have one active card, one separator and @@ -146,7 +146,7 @@ void EnableFastUserSwitching() { // Profile icon. NSView* activeProfileImage = [activeCardSubviews objectAtIndex:2]; - EXPECT_TRUE([activeProfileImage isKindOfClass:[NSImageView class]]); + EXPECT_TRUE([activeProfileImage isKindOfClass:[NSButton class]]); // Profile name. NSView* activeProfileName = [activeCardSubviews objectAtIndex:1]; @@ -174,7 +174,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; // Three profiles means we should have one active card and a @@ -217,7 +217,7 @@ void EnableFastUserSwitching() { // Profile icon. NSView* activeProfileImage = [activeCardSubviews objectAtIndex:2]; - EXPECT_TRUE([activeProfileImage isKindOfClass:[NSImageView class]]); + EXPECT_TRUE([activeProfileImage isKindOfClass:[NSButton class]]); // Profile name. NSView* activeProfileName = [activeCardSubviews objectAtIndex:1]; @@ -248,7 +248,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; NSString* sortedNames[] = { @"Another Test", @"New Profile", @@ -280,7 +280,7 @@ void EnableFastUserSwitching() { switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess()); StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; NSArray* activeCardSubviews = [[subviews objectAtIndex:2] subviews]; NSArray* activeCardLinks = [[activeCardSubviews objectAtIndex:0] subviews]; @@ -309,7 +309,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; NSArray* activeCardSubviews = [[subviews objectAtIndex:2] subviews]; NSArray* activeCardLinks = [[activeCardSubviews objectAtIndex:0] subviews]; @@ -331,7 +331,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; NSArray* activeCardSubviews = [[subviews objectAtIndex:2] subviews]; NSArray* activeCardLinks = [[activeCardSubviews objectAtIndex:0] subviews]; @@ -366,7 +366,7 @@ void EnableFastUserSwitching() { profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT]; NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; // There should be one active card, one accounts container, two separators @@ -443,7 +443,7 @@ void EnableFastUserSwitching() { // Profile icon. NSView* activeProfileImage = [activeCardSubviews objectAtIndex:2]; - EXPECT_TRUE([activeProfileImage isKindOfClass:[NSImageView class]]); + EXPECT_TRUE([activeProfileImage isKindOfClass:[NSButton class]]); // Profile name. NSView* activeProfileName = [activeCardSubviews objectAtIndex:1]; @@ -470,7 +470,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; // Three profiles means we should have one active card, one separator, one @@ -503,7 +503,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; // Three profiles means we should have one active card, one separator, one