views: add focus to TabbedPane

ui/views/controls/tabbed_pane/tabbed_pane.cc

Index: ui/views/controls/tabbed_pane/tabbed_pane.cc
diff --git a/ui/views/controls/tabbed_pane/tabbed_pane.cc b/ui/views/controls/tabbed_pane/tabbed_pane.cc
index b445db30b5cef9f364aaeb3f6be7651c51800c9f..f944b9d8128f1bd0485bdf0db9aaa41b6193931a 100644
--- a/ui/views/controls/tabbed_pane/tabbed_pane.cc
+++ b/ui/views/controls/tabbed_pane/tabbed_pane.cc
@@ -68,6 +68,8 @@ class Tab : public View {
   gfx::Size GetPreferredSize() const override;
   void Layout() override;
   const char* GetClassName() const override;
+  void OnFocus() override;
+  void OnBlur() override;
 
  protected:
   Label* title() { return title_; }
@@ -75,6 +77,9 @@ class Tab : public View {
   // Called whenever |tab_state_| changes.
   virtual void OnStateChanged();
 
+  // Returns whether the containing TabStrip has focus.
+  bool ContainerHasFocus();
+
  private:
   enum TabState {
     TAB_INACTIVE,
@@ -169,6 +174,12 @@ Tab::~Tab() {}
 void Tab::SetSelected(bool selected) {
   contents_->SetVisible(selected);
   SetState(selected ? TAB_ACTIVE : TAB_INACTIVE);
+#if defined(OS_MACOSX)
+  SetFocusBehavior(selected ? FocusBehavior::ACCESSIBLE_ONLY
+                            : FocusBehavior::NEVER);
+#else
+  SetFocusBehavior(selected ? FocusBehavior::ALWAYS : FocusBehavior::NEVER);

[Evan Stade, 2016/09/28 21:25:54]

Button::SetFocusForPlatform has to do something similar. It seems like this is a
pretty clunky little stanza of code here, can we streamline it? What if we
created a new value of FocusBehavior which acted like ACCESSIBLE_ONLY on mac and
ALWAYS elsewhere?

[Elly Fong-Jones, 2016/09/30 11:54:07]

From a cursory look, it seems that ACCESSIBLE_ONLY and ALWAYS are synonyms on
non-Mac anyway, so in principle it'd be safe to always use the OS_MACOSX code
here. I've contacted karandeepb@ (who originally introduced ACCESSIBLE_ONLY) to
ask if there's some reason we shouldn't simply do that.

[Evan Stade, 2016/09/30 17:24:41]

hmm, don't you mean to always use the non-osx code here?

+#endif
 }
 
 void Tab::OnStateChanged() {
@@ -252,6 +263,72 @@ void Tab::SetState(TabState tab_state) {
   SchedulePaint();
 }
 
+bool Tab::ContainerHasFocus() {
+  return tabbed_pane_->HasFocus();
+}
+
+void Tab::OnFocus() {
+  OnStateChanged();
+  if (contents())
+    contents()->NotifyAccessibilityEvent(ui::AX_EVENT_FOCUS, true);
+  SchedulePaint();
+}
+
+void Tab::OnBlur() {
+  OnStateChanged();
+  SchedulePaint();
+}
+
+// The border used for MdTabs when they are both focused and selected: a
+// rectangle with the bottom side in a base color and the other sides in a
+// lightened version of the base color.
+class MdTabFocusRingBorder : public Border {
+ public:
+  MdTabFocusRingBorder();
+  ~MdTabFocusRingBorder() override;
+
+ private:
+  void Paint(const View& view, gfx::Canvas* canvas) override;
+  gfx::Insets GetInsets() const override;
+  gfx::Size GetMinimumSize() const override;
+};
+
+MdTabFocusRingBorder::MdTabFocusRingBorder() {}
+MdTabFocusRingBorder::~MdTabFocusRingBorder() {}
+
+void MdTabFocusRingBorder::Paint(const View& view, gfx::Canvas* canvas) {
+  gfx::Insets insets = GetInsets();
+  // TODO(ellyjones): should this 0x66 be part of NativeTheme somehow?
+  SkColor base_color = view.GetNativeTheme()->GetSystemColor(
+      ui::NativeTheme::kColorId_FocusedBorderColor);
+  SkColor light_color = SkColorSetA(base_color, 0x66);
+  const int kBorderStrokeWidth = 2;
+
+  SkPaint paint;
+  paint.setColor(light_color);
+  paint.setStyle(SkPaint::kStroke_Style);
+  paint.setStrokeWidth(kBorderStrokeWidth);
+
+  gfx::Rect bounds = gfx::Rect(0, 0, view.width(), view.height());

[Evan Stade, 2016/09/28 21:25:54]

this is equivalent to view.GetLocalBounds()

[Elly Fong-Jones, 2016/09/30 11:54:07]

Done.

+  bounds.Inset(kBorderStrokeWidth / 2, kBorderStrokeWidth / 2);
+
+  // Draw the lighter-colored stroke first, then draw the heavier stroke over
+  // the bottom of it. This is fine because the heavier stroke has 1.0 alpha, so
+  // the lighter stroke won't show through.
+  canvas->DrawRect(bounds, paint);
+  canvas->FillRect(gfx::Rect(0, view.height() - insets.bottom(), view.width(),
+                             insets.bottom()),
+                   base_color);
+}
+
+gfx::Insets MdTabFocusRingBorder::GetInsets() const {
+  return gfx::Insets(2, 2);

[Evan Stade, 2016/09/28 21:25:54]

this 2 is the same as kBorderStrokeWidth, no?

also, I think you can just do gfx::Insets(kBorderStrokeWidth) (there's a ctor
for that)

[Elly Fong-Jones, 2016/09/30 11:54:07]

Done.

+}
+
+gfx::Size MdTabFocusRingBorder::GetMinimumSize() const {
+  return gfx::Size(GetInsets().width(), GetInsets().height());
+}
+
 MdTab::MdTab(TabbedPane* tabbed_pane,
              const base::string16& title,
              View* contents)
@@ -267,8 +344,12 @@ void MdTab::OnStateChanged() {
       selected() ? ui::NativeTheme::kColorId_FocusedBorderColor
                  : ui::NativeTheme::kColorId_UnfocusedBorderColor);
   int border_thickness = selected() ? 2 : 1;
-  SetBorder(
-      Border::CreateSolidSidedBorder(0, 0, border_thickness, 0, border_color));
+  if (HasFocus() && selected()) {
+    SetBorder(base::MakeUnique<MdTabFocusRingBorder>());

[Evan Stade, 2016/09/28 21:25:54]

Can we just always use MdTabFocusRingBorder? In the OnPaint function, check
view.HasFocus() and tab_->selected() (you would have to pass an MdTab into the
MdTabFocusRingBorder ctor)

this actually makes me think we should just override View::OnDrawBorder in
MdTab. The separate border class isn't buying us much. (The reason that Border
exists as a separate class is to share border code between views, but I don't
think we plan to share this one any time soon.)

[Elly Fong-Jones, 2016/09/30 11:54:07]

I moved all this logic into MdTabFocusRingBorder::Paint(), so this function is
now quite short. I don't really want to override OnPaintBorder, though - it
seems kind of icky to circumvent Views' builtin border drawing. If you feel
strongly I can change it, but for now I've left MdTabFocusRingBorder as-is.

[Evan Stade, 2016/09/30 17:24:41]

why is it icky? OnPaintBorder wouldn't be virtual if it weren't meant to be
overridden.

+  } else {
+    SetBorder(Border::CreateSolidSidedBorder(0, 0, border_thickness, 0,
+                                             border_color));
+  }
 
   SkColor font_color = selected()
       ? theme->GetSystemColor(ui::NativeTheme::kColorId_CallToActionColor)
@@ -386,12 +467,6 @@ TabbedPane::TabbedPane()
                      : new TabStrip(this)),
       contents_(new View()),
       selected_tab_index_(-1) {
-#if defined(OS_MACOSX)
-  SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
-#else
-  SetFocusBehavior(FocusBehavior::ALWAYS);
-#endif
-
   AddChildView(tab_strip_);
   AddChildView(contents_);
 }
@@ -403,11 +478,6 @@ int TabbedPane::GetTabCount() {
   return contents_->child_count();
 }
 
-View* TabbedPane::GetSelectedTab() {
-  return selected_tab_index() < 0 ?
-      NULL : GetTabAt(selected_tab_index())->contents();
-}
-
 void TabbedPane::AddTab(const base::string16& title, View* contents) {
   AddTabAtIndex(tab_strip_->child_count(), title, contents);
 }
@@ -435,12 +505,14 @@ void TabbedPane::SelectTabAt(int index) {
   if (index == selected_tab_index())
     return;
 
-  if (selected_tab_index() >= 0)
-    GetTabAt(selected_tab_index())->SetSelected(false);
-
-  selected_tab_index_ = index;
   Tab* tab = GetTabAt(index);
   tab->SetSelected(true);
+  if (GetSelectedTab()) {
+    if (GetSelectedTab()->HasFocus())
+      tab->RequestFocus();
+    GetSelectedTab()->SetSelected(false);
+  }
+  selected_tab_index_ = index;
   tab_strip_->SchedulePaint();
 
   FocusManager* focus_manager = tab->contents()->GetFocusManager();
@@ -473,6 +545,21 @@ Tab* TabbedPane::GetTabAt(int index) {
   return static_cast<Tab*>(tab_strip_->child_at(index));
 }
 
+Tab* TabbedPane::GetSelectedTab() {
+  return selected_tab_index() >= 0 ? GetTabAt(selected_tab_index()) : nullptr;
+}
+
+bool TabbedPane::MoveSelectionBy(int delta) {
+  const int tab_count = GetTabCount();
+  if (tab_count <= 1)
+    return false;
+  int next_selected_index = (selected_tab_index() + delta) % tab_count;
+  if (next_selected_index < 0)
+    next_selected_index += tab_count;
+  SelectTabAt(next_selected_index);
+  return true;
+}
+
 void TabbedPane::Layout() {
   const gfx::Size size = tab_strip_->GetPreferredSize();
   tab_strip_->SetBounds(0, 0, width(), size.height());
@@ -495,32 +582,22 @@ void TabbedPane::ViewHierarchyChanged(
 bool TabbedPane::AcceleratorPressed(const ui::Accelerator& accelerator) {
   // Handle Ctrl+Tab and Ctrl+Shift+Tab navigation of pages.
   DCHECK(accelerator.key_code() == ui::VKEY_TAB && accelerator.IsCtrlDown());
-  const int tab_count = GetTabCount();
-  if (tab_count <= 1)
+  return MoveSelectionBy(accelerator.IsShiftDown() ? -1 : 1);
+}
+
+bool TabbedPane::OnKeyPressed(const ui::KeyEvent& event) {
+  if (!GetSelectedTab() || !GetSelectedTab()->HasFocus())
     return false;
-  const int increment = accelerator.IsShiftDown() ? -1 : 1;
-  int next_tab_index = (selected_tab_index() + increment) % tab_count;
-  // Wrap around.
-  if (next_tab_index < 0)
-    next_tab_index += tab_count;
-  SelectTabAt(next_tab_index);
-  return true;
+  ui::KeyboardCode key = event.key_code();
+  if (key != ui::VKEY_LEFT && key != ui::VKEY_RIGHT)
+    return false;
+  return MoveSelectionBy(key == ui::VKEY_RIGHT ? 1 : -1);
 }
 
 const char* TabbedPane::GetClassName() const {
   return kViewClassName;
 }
 
-void TabbedPane::OnFocus() {
-  View::OnFocus();
-
-  View* selected_tab = GetSelectedTab();
-  if (selected_tab) {
-    selected_tab->NotifyAccessibilityEvent(
-        ui::AX_EVENT_FOCUS, true);
-  }
-}
-
 void TabbedPane::GetAccessibleState(ui::AXViewState* state) {
   state->role = ui::AX_ROLE_TAB_LIST;
 }