[Win] Add tab and keyboard navigation to the new avatar bubble

chrome/browser/ui/views/profiles/profile_chooser_view.cc

 // 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.
 
 #include "chrome/browser/ui/views/profiles/profile_chooser_view.h"
 
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
@@ skipping 123 matching lines @@
   BackgroundColorHoverButton(views::ButtonListener* listener,
                              const base::string16& text,
                              const gfx::ImageSkia& icon)
       : views::LabelButton(listener, text) {
     SetImageLabelSpacing(views::kItemLabelSpacing);
     SetBorder(views::Border::CreateEmptyBorder(
         0, views::kButtonHEdgeMarginNew, 0, views::kButtonHEdgeMarginNew));
     SetMinSize(gfx::Size(0,
         kButtonHeight + views::kRelatedControlVerticalSpacing));
     SetImage(STATE_NORMAL, icon);
+    SetFocusable(true);
   }
 
   virtual ~BackgroundColorHoverButton() {}
 
  private:
   // views::LabelButton:
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE {
     if ((state() == STATE_PRESSED) ||
         (state() == STATE_HOVERED) ||
         HasFocus()) {
@@ skipping 41 matching lines @@
     views::LabelButton::Layout();
     label()->SetHorizontalAlignment(gfx::ALIGN_CENTER);
   }
 
   DISALLOW_COPY_AND_ASSIGN(RightAlignedIconLabelButton);
 };
 
 // EditableProfilePhoto -------------------------------------------------
 
 // A custom Image control that shows a "change" button when moused over.
-class EditableProfilePhoto : public views::ImageView {
+class EditableProfilePhoto : public views::ImageButton {
  public:
   EditableProfilePhoto(views::ButtonListener* listener,
                        const gfx::Image& icon,
                        bool is_editing_allowed,
                        const gfx::Rect& bounds)
-      : views::ImageView(),
-        change_photo_button_(NULL) {
+      : views::ImageButton(listener),
+        photo_overlay_(NULL) {
     gfx::Image image = profiles::GetSizedAvatarIcon(
         icon, true, kLargeImageSide, kLargeImageSide);
-    SetImage(image.ToImageSkia());
+    SetImage(views::LabelButton::STATE_NORMAL, image.ToImageSkia());
     SetBoundsRect(bounds);
 
     // Calculate the circular mask that will be used to display the photo.
     circular_mask_.addCircle(SkIntToScalar(bounds.width() / 2),
                              SkIntToScalar(bounds.height() / 2),
                              SkIntToScalar(bounds.width() / 2));
 
-    if (!is_editing_allowed)
+    if (!is_editing_allowed) {
+      SetEnabled(false);
       return;
+    }
 
+    SetFocusable(true);
     set_notify_enter_exit_on_child(true);
 
-    // Button overlay that appears when hovering over the image.
-    change_photo_button_ = new views::LabelButton(listener, base::string16());
-    change_photo_button_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
-    change_photo_button_->SetBorder(views::Border::NullBorder());
+    // Photo overlay that appears when hovering over the button.
+    photo_overlay_ = new views::ImageView();
 
     const SkColor kBackgroundColor = SkColorSetARGB(65, 255, 255, 255);
-    change_photo_button_->set_background(
+    photo_overlay_->set_background(
         views::Background::CreateSolidBackground(kBackgroundColor));
-    change_photo_button_->SetImage(views::LabelButton::STATE_NORMAL,
-        *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
-            IDR_ICON_PROFILES_EDIT_CAMERA));
+    photo_overlay_->SetImage(*ui::ResourceBundle::GetSharedInstance().
+        GetImageSkiaNamed(IDR_ICON_PROFILES_EDIT_CAMERA));
 
-    change_photo_button_->SetSize(bounds.size());
-    change_photo_button_->SetVisible(false);
-    AddChildView(change_photo_button_);
+    photo_overlay_->SetSize(bounds.size());
+    photo_overlay_->SetVisible(false);
+    AddChildView(photo_overlay_);
   }
 
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE {
     // Display the profile picture as a circle.
     canvas->ClipPath(circular_mask_, true);
-    views::ImageView::OnPaint(canvas);
+    views::ImageButton::OnPaint(canvas);
   }
 
   virtual void PaintChildren(gfx::Canvas* canvas,
                              const views::CullSet& cull_set) OVERRIDE {
     // Display any children (the "change photo" overlay) as a circle.
     canvas->ClipPath(circular_mask_, true);
     View::PaintChildren(canvas, cull_set);
   }
 
-  views::LabelButton* change_photo_button() { return change_photo_button_; }
-
  private:
   // views::View:
   virtual void OnMouseEntered(const ui::MouseEvent& event) OVERRIDE {

[msw, 2014/08/20 19:29:25]

nit: maybe overload StateChanged instead of OnMouseEntered/Exited? 
(then OnFocus could SetState(views::CustomButton::STATE_HOVERED)).
You might not even need the OnBlur (and maybe OnFocus?) overrides.

[noms (inactive), 2014/08/20 20:25:32]

Still need OnFocus/OnBlur because focusing on/away doesn't trigger a state
change. But nuked the mice and went to StateChanged() for less hackiness.

On 2014/08/20 19:29:25, msw wrote:

-    if (change_photo_button_)
-      change_photo_button_->SetVisible(true);
+    if (photo_overlay_)
+      photo_overlay_->SetVisible(true);
   }
 
   virtual void OnMouseExited(const ui::MouseEvent& event) OVERRIDE {
-    if (change_photo_button_)
-      change_photo_button_->SetVisible(false);
+    if (photo_overlay_) {
+      GetFocusManager()->ClearFocus();

[msw, 2014/08/20 19:29:25]

This seems very wrong (see my other comment).

[noms (inactive), 2014/08/20 20:25:32]

Done.

+      photo_overlay_->SetVisible(false);
+    }
+  }
+
+  virtual void OnFocus() OVERRIDE {
+    views::ImageButton::OnFocus();
+    if (photo_overlay_)
+      photo_overlay_->SetVisible(true);
+  }
+
+  virtual void OnBlur() OVERRIDE {
+    views::ImageButton::OnBlur();
+    if (photo_overlay_)
+      photo_overlay_->SetVisible(false);
   }
 
   gfx::Path circular_mask_;
 
-  // Button that is shown when hovering over the image view. Can be NULL if
+  // Image that is shown when hovering over the image button. Can be NULL if
   // the photo isn't allowed to be edited (e.g. for guest profiles).
-  views::LabelButton* change_photo_button_;
+  views::ImageView* photo_overlay_;
 
   DISALLOW_COPY_AND_ASSIGN(EditableProfilePhoto);
 };
 
 // EditableProfileName -------------------------------------------------
 
 // A custom text control that turns into a textfield for editing when clicked.
 class EditableProfileName : public RightAlignedIconLabelButton,
                             public views::ButtonListener {
  public:
   EditableProfileName(views::TextfieldController* controller,
                       const base::string16& text,
                       bool is_editing_allowed)
       : RightAlignedIconLabelButton(this, text),
         profile_name_textfield_(NULL) {
     ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
     const gfx::FontList& medium_font_list =
         rb->GetFontList(ui::ResourceBundle::MediumFont);
     SetFontList(medium_font_list);
     SetHorizontalAlignment(gfx::ALIGN_CENTER);
 
     if (!is_editing_allowed) {
       SetBorder(views::Border::CreateEmptyBorder(2, 0, 2, 0));
       return;
     }
 
+    SetFocusable(true);
     // Show an "edit" pencil icon when hovering over. In the default state,
     // we need to create an empty placeholder of the correct size, so that
     // the text doesn't jump around when the hovered icon appears.
     gfx::ImageSkia hover_image =
         *rb->GetImageSkiaNamed(IDR_ICON_PROFILES_EDIT_HOVER);
     SetImage(STATE_NORMAL, CreateSquarePlaceholderImage(hover_image.width()));
     SetImage(STATE_HOVERED, hover_image);
     SetImage(STATE_PRESSED,
              *rb->GetImageSkiaNamed(IDR_ICON_PROFILES_EDIT_PRESSED));
     // To center the text, we need to offest it by the width of the icon we
@@ skipping 42 matching lines @@
     // part of the new profile name typed in the textfield.
     return false;
   }
 
   virtual void Layout() OVERRIDE {
     if (profile_name_textfield_)
       profile_name_textfield_->SetBounds(0, 0, width(), height());
     RightAlignedIconLabelButton::Layout();
   }
 
+  virtual void OnFocus() OVERRIDE {
+    RightAlignedIconLabelButton::OnFocus();
+    SetState(views::CustomButton::STATE_HOVERED);

[msw, 2014/08/20 19:29:25]

nit: This might be done for you, but I'm not sure.

[noms (inactive), 2014/08/20 20:25:32]

Sadly, no. OnFocus/Blur doesn't trigger a state change. It probably should, but
I'd rather not do it in this CL as it will get merged to M38 and break
everything. :)

On 2014/08/20 19:29:25, msw wrote:

+  }
+
+  virtual void OnBlur() OVERRIDE {
+    RightAlignedIconLabelButton::OnBlur();
+    SetState(views::CustomButton::STATE_NORMAL);

[msw, 2014/08/20 19:29:25]

nit: This may already be accomplished by CustomButton::OnBlur.

[noms (inactive), 2014/08/20 20:25:32]

Same as above.
On 2014/08/20 19:29:25, msw wrote:

+  }
+
   // Textfield that is shown when editing the profile name. Can be NULL if
   // the profile name isn't allowed to be edited (e.g. for guest profiles).
   views::Textfield* profile_name_textfield_;
 
   DISALLOW_COPY_AND_ASSIGN(EditableProfileName);
 };
 
 // A title card with one back button right aligned and one label center aligned.
 class TitleCard : public views::View {
  public:
   TitleCard(const base::string16& message, views::ButtonListener* listener,
             views::ImageButton** back_button) {
     back_button_ = new views::ImageButton(listener);
     back_button_->SetImageAlignment(views::ImageButton::ALIGN_LEFT,
                                     views::ImageButton::ALIGN_MIDDLE);
     ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
     back_button_->SetImage(views::ImageButton::STATE_NORMAL,
                            rb->GetImageSkiaNamed(IDR_BACK));
     back_button_->SetImage(views::ImageButton::STATE_HOVERED,
                            rb->GetImageSkiaNamed(IDR_BACK_H));
     back_button_->SetImage(views::ImageButton::STATE_PRESSED,
                            rb->GetImageSkiaNamed(IDR_BACK_P));
     back_button_->SetImage(views::ImageButton::STATE_DISABLED,
                            rb->GetImageSkiaNamed(IDR_BACK_D));
+    back_button_->SetFocusable(true);
     *back_button = back_button_;
 
     title_label_ = new views::Label(message);
     title_label_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
     const gfx::FontList& medium_font_list =
         rb->GetFontList(ui::ResourceBundle::MediumFont);
     title_label_->SetFontList(medium_font_list);
 
     AddChildView(back_button_);
     AddChildView(title_label_);
@@ skipping 323 matching lines @@
     std::string primary_account =
         SigninManagerFactory::GetForProfile(browser_->profile())->
         GetAuthenticatedUsername();
     // The account management view is only available with the
     // --enable-account-consistency flag.
     bool account_management_available = !primary_account.empty() &&
         switches::IsEnableAccountConsistency();
     ShowView(account_management_available ?
         profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT :
         profiles::BUBBLE_VIEW_MODE_PROFILE_CHOOSER, avatar_menu_.get());
-  } else if (current_profile_photo_ &&
-             sender == current_profile_photo_->change_photo_button()) {
+  } else if (sender == current_profile_photo_) {
     avatar_menu_->EditProfile(avatar_menu_->GetActiveProfileIndex());
     PostActionPerformed(ProfileMetrics::PROFILE_DESKTOP_MENU_EDIT_IMAGE);
   } else if (sender == signin_current_profile_link_) {
     ShowView(profiles::BUBBLE_VIEW_MODE_GAIA_SIGNIN, avatar_menu_.get());
   } else if (sender == add_person_button_) {
     ProfileMetrics::LogProfileNewAvatarMenuNotYou(
         ProfileMetrics::PROFILE_AVATAR_MENU_NOT_YOU_ADD_PERSON);
     chrome::ShowUserManager(browser_->profile()->GetPath());
   } else if (sender == disconnect_button_) {
     ProfileMetrics::LogProfileNewAvatarMenuNotYou(
@@ skipping 382 matching lines @@
             IDR_ICON_PROFILES_ACCOUNT_BUTTON_ERROR).ToImageSkia();
         auth_error_email_button_ =
             new RightAlignedIconLabelButton(this, avatar_item.sync_state);
         auth_error_email_button_->SetElideBehavior(gfx::ELIDE_EMAIL);
         auth_error_email_button_->SetBorder(views::Border::NullBorder());
         auth_error_email_button_->SetImage(
             views::LabelButton::STATE_NORMAL, warning_image);
         auth_error_email_button_->SetTextColor(
             views::LabelButton::STATE_NORMAL,
             views::Link::GetDefaultEnabledColor());
+        auth_error_email_button_->SetFocusable(true);
         layout->AddView(auth_error_email_button_);
       } else {
         views::Label* email_label = new views::Label(avatar_item.sync_state);
         email_label->SetElideBehavior(gfx::ELIDE_EMAIL);
         email_label->SetEnabled(false);
         layout->AddView(email_label);
       }
     }
   } else {
     SigninManagerBase* signin_manager = SigninManagerFactory::GetForProfile(
@@ skipping 440 matching lines @@
       IncognitoModePrefs::GetAvailability(browser_->profile()->GetPrefs()) !=
           IncognitoModePrefs::DISABLED;
   return incognito_available && !browser_->profile()->IsGuestSession();
 }
 
 void ProfileChooserView::PostActionPerformed(
     ProfileMetrics::ProfileDesktopMenu action_performed) {
   ProfileMetrics::LogProfileDesktopMenu(action_performed, gaia_service_type_);
   gaia_service_type_ = signin::GAIA_SERVICE_TYPE_NONE;
 }