Vertically centered app list page switcher

ui/app_list/views/page_switcher_vertical.cc

Index: ui/app_list/views/page_switcher_vertical.cc
diff --git a/ui/app_list/views/page_switcher.cc b/ui/app_list/views/page_switcher_vertical.cc
similarity index 69%
rename from ui/app_list/views/page_switcher.cc
rename to ui/app_list/views/page_switcher_vertical.cc
index a2c427f3e0fd35739cbf3c54a6be1be8539cb937..26457b24f29a7b71d370daf83d43a1c2ab8efde1 100644
--- a/ui/app_list/views/page_switcher.cc
+++ b/ui/app_list/views/page_switcher_vertical.cc
@@ -2,8 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ui/app_list/views/page_switcher.h"
-
+#include <ui/app_list/views/page_switcher_vertical.h>

[xiyuan, 2017/06/13 18:29:55]

Use " for chromium headers

[weidongg, 2017/06/13 21:25:00]

Done.

 #include <algorithm>
 
 #include "base/macros.h"
@@ -21,13 +20,13 @@ namespace app_list {
 
 namespace {
 
-const int kPreferredHeight = 58;
+const int kPreferredWidth = 58;
 
 const int kMaxButtonSpacing = 18;
 const int kMinButtonSpacing = 4;
-const int kMaxButtonWidth = 68;
-const int kMinButtonWidth = 28;
-const int kButtonHeight = 6;
+const int kMaxButtonHeight = 68;
+const int kMinButtonHeight = 28;
+const int kButtonWidth = 6;
 const int kButtonCornerRadius = 2;
 const int kButtonStripPadding = 20;
 
@@ -35,9 +34,8 @@ class PageSwitcherButton : public views::CustomButton {
  public:
   explicit PageSwitcherButton(views::ButtonListener* listener)
       : views::CustomButton(listener),
-        button_width_(kMaxButtonWidth),
-        selected_range_(0) {
-  }
+        button_height_(kMaxButtonHeight),
+        selected_range_(0) {}
   ~PageSwitcherButton() override {}
 
   void SetSelectedRange(double selected_range) {
@@ -48,11 +46,11 @@ class PageSwitcherButton : public views::CustomButton {
     SchedulePaint();
   }
 
-  void set_button_width(int button_width) { button_width_ = button_width; }
+  void set_button_height(int button_height) { button_height_ = button_height; }
 
   // Overridden from views::View:
   gfx::Size CalculatePreferredSize() const override {
-    return gfx::Size(button_width_, kButtonHeight);
+    return gfx::Size(kButtonWidth, button_height_);
   }
 
   void PaintButtonContents(gfx::Canvas* canvas) override {
@@ -77,7 +75,7 @@ class PageSwitcherButton : public views::CustomButton {
   // Paints a button that has two rounded corner at bottom.
   void PaintButton(gfx::Canvas* canvas, SkColor base_color) {
     gfx::Rect rect(GetContentsBounds());
-    rect.ClampToCenteredSize(gfx::Size(button_width_, kButtonHeight));
+    rect.ClampToCenteredSize(gfx::Size(kButtonWidth, button_height_));

[xiyuan, 2017/06/13 18:29:55]

This would not look the same as the UX mock. You will have follow CLs to change
this?

[weidongg, 2017/06/13 21:25:00]

Yes, I will upload additional CLs to make changes based on the specs. This CL is
building the basic framework and separate new code from old code for convenience
of removing the old code in the future.

[xiyuan, 2017/06/13 21:58:26]

Acknowledged.

 
     SkPath path;
     path.addRoundRect(gfx::RectToSkRect(rect),
@@ -90,19 +88,19 @@ class PageSwitcherButton : public views::CustomButton {
     flags.setColor(base_color);
     canvas->DrawPath(path, flags);
 
-    int selected_start_x = 0;
-    int selected_width = 0;
+    int selected_start_y = 0;
+    int selected_height = 0;
     if (selected_range_ > 0) {
-      selected_width = selected_range_ * rect.width();
+      selected_height = selected_range_ * rect.height();
     } else if (selected_range_ < 0) {
-      selected_width =  -selected_range_ * rect.width();
-      selected_start_x = rect.right() - selected_width;
+      selected_height = -selected_range_ * rect.height();
+      selected_start_y = rect.bottom() - selected_height;
     }
 
-    if (selected_width) {
+    if (selected_height) {
       gfx::Rect selected_rect(rect);
-      selected_rect.set_x(selected_start_x);
-      selected_rect.set_width(selected_width);
+      selected_rect.set_y(selected_start_y);
+      selected_rect.set_height(selected_height);
 
       SkPath selected_path;
       selected_path.addRoundRect(gfx::RectToSkRect(selected_rect),
@@ -113,11 +111,11 @@ class PageSwitcherButton : public views::CustomButton {
     }
   }
 
-  int button_width_;
+  int button_height_;
 
   // [-1, 1] range that represents the portion of the button that should be
-  // painted with kSelectedColor. Positive range starts from left side and
-  // negative range starts from the right side.
+  // painted with kSelectedColor. Positive range starts from top side and
+  // negative range starts from the bottom side.
   double selected_range_;
 
   DISALLOW_COPY_AND_ASSIGN(PageSwitcherButton);
@@ -130,9 +128,8 @@ PageSwitcherButton* GetButtonByIndex(views::View* buttons, int index) {
 
 }  // namespace
 
-PageSwitcher::PageSwitcher(PaginationModel* model)
-    : model_(model),
-      buttons_(new views::View) {
+PageSwitcherVertical::PageSwitcherVertical(PaginationModel* model)
+    : model_(model), buttons_(new views::View) {
   AddChildView(buttons_);
 
   TotalPagesChanged();
@@ -140,11 +137,11 @@ PageSwitcher::PageSwitcher(PaginationModel* model)
   model_->AddObserver(this);
 }
 
-PageSwitcher::~PageSwitcher() {
+PageSwitcherVertical::~PageSwitcherVertical() {
   model_->RemoveObserver(this);
 }
 
-int PageSwitcher::GetPageForPoint(const gfx::Point& point) const {
+int PageSwitcherVertical::GetPageForPoint(const gfx::Point& point) const {

[xiyuan, 2017/06/13 18:29:55]

If we are using the same code for both switchers, this code (, other common
parts) and |buttons_| member can probably stay in the base class.

[weidongg, 2017/06/13 21:25:00]

Ok, I see. But the both GetPageForPoint() and UpdateUIForDragPoint() use
PageSwitcherButton which is defined in anonymous namespace and will be different
for old and new code.
In the future, the old(horizontal) code will be completely removed and base code
may be not longer needed. I think it may be easier to do the refactor if we make
the base class lighter.

[xiyuan, 2017/06/13 21:58:26]

Acknowledged.

   if (!buttons_->bounds().Contains(point))
     return -1;
 
@@ -160,7 +157,7 @@ int PageSwitcher::GetPageForPoint(const gfx::Point& point) const {
   return -1;
 }
 
-void PageSwitcher::UpdateUIForDragPoint(const gfx::Point& point) {
+void PageSwitcherVertical::UpdateUIForDragPoint(const gfx::Point& point) {
   int page = GetPageForPoint(point);
 
   const int button_count = buttons_->child_count();
@@ -178,60 +175,59 @@ void PageSwitcher::UpdateUIForDragPoint(const gfx::Point& point) {
   }
 }
 
-gfx::Size PageSwitcher::CalculatePreferredSize() const {
-  // Always return a size with correct height so that container resize is not
+gfx::Size PageSwitcherVertical::CalculatePreferredSize() const {
+  // Always return a size with correct width so that container resize is not
   // needed when more pages are added.
-  return gfx::Size(buttons_->GetPreferredSize().width(),
-                   kPreferredHeight);
+  return gfx::Size(kPreferredWidth, buttons_->GetPreferredSize().height());
 }
 
-void PageSwitcher::Layout() {
+void PageSwitcherVertical::Layout() {
   gfx::Rect rect(GetContentsBounds());
 
-  CalculateButtonWidthAndSpacing(rect.width());
+  CalculateButtonHeightAndSpacing(rect.height());
 
-  // Makes |buttons_| horizontally center and vertically fill.
+  // Makes |buttons_| vertically center and horizontally fill.
   gfx::Size buttons_size(buttons_->GetPreferredSize());
-  gfx::Rect buttons_bounds(rect.CenterPoint().x() - buttons_size.width() / 2,
-                           rect.y(),
-                           buttons_size.width(),
-                           rect.height());
+  gfx::Rect buttons_bounds(rect.x(),
+                           rect.CenterPoint().y() - buttons_size.height() / 2,
+                           rect.width(), buttons_size.height());
   buttons_->SetBoundsRect(gfx::IntersectRects(rect, buttons_bounds));
 }
 
-void PageSwitcher::CalculateButtonWidthAndSpacing(int contents_width) {
+void PageSwitcherVertical::CalculateButtonHeightAndSpacing(
+    int contents_height) {
   const int button_count = buttons_->child_count();
   if (!button_count)
     return;
 
-  contents_width -= 2 * kButtonStripPadding;
+  contents_height -= 2 * kButtonStripPadding;
 
-  int button_width = kMinButtonWidth;
+  int button_height = kMinButtonHeight;
   int button_spacing = kMinButtonSpacing;
   if (button_count > 1) {
-    button_spacing = (contents_width - button_width * button_count) /
-        (button_count - 1);
+    button_spacing =
+        (contents_height - button_height * button_count) / (button_count - 1);
     button_spacing = std::min(kMaxButtonSpacing,
                               std::max(kMinButtonSpacing, button_spacing));
   }
 
-  button_width = (contents_width - (button_count - 1) * button_spacing) /
-      button_count;
-  button_width = std::min(kMaxButtonWidth,
-                          std::max(kMinButtonWidth, button_width));
+  button_height =
+      (contents_height - (button_count - 1) * button_spacing) / button_count;
+  button_height =
+      std::min(kMaxButtonHeight, std::max(kMinButtonHeight, button_height));
 
   buttons_->SetLayoutManager(new views::BoxLayout(
-      views::BoxLayout::kHorizontal, gfx::Insets(0, kButtonStripPadding),
+      views::BoxLayout::kVertical, gfx::Insets(kButtonStripPadding, 0),
       button_spacing));
   for (int i = 0; i < button_count; ++i) {
     PageSwitcherButton* button =
         static_cast<PageSwitcherButton*>(buttons_->child_at(i));
-    button->set_button_width(button_width);
+    button->set_button_height(button_height);
   }
 }
 
-void PageSwitcher::ButtonPressed(views::Button* sender,
-                                 const ui::Event& event) {
+void PageSwitcherVertical::ButtonPressed(views::Button* sender,
+                                         const ui::Event& event) {
   for (int i = 0; i < buttons_->child_count(); ++i) {
     if (sender == static_cast<views::Button*>(buttons_->child_at(i))) {
       model_->SelectPage(i, true /* animate */);
@@ -240,7 +236,7 @@ void PageSwitcher::ButtonPressed(views::Button* sender,
   }
 }
 
-void PageSwitcher::TotalPagesChanged() {
+void PageSwitcherVertical::TotalPagesChanged() {
   buttons_->RemoveAllChildViews(true);
   for (int i = 0; i < model_->total_pages(); ++i) {
     PageSwitcherButton* button = new PageSwitcherButton(this);
@@ -251,17 +247,17 @@ void PageSwitcher::TotalPagesChanged() {
   Layout();
 }
 
-void PageSwitcher::SelectedPageChanged(int old_selected, int new_selected) {
+void PageSwitcherVertical::SelectedPageChanged(int old_selected,
+                                               int new_selected) {
   if (old_selected >= 0 && old_selected < buttons_->child_count())
     GetButtonByIndex(buttons_, old_selected)->SetSelectedRange(0);
   if (new_selected >= 0 && new_selected < buttons_->child_count())
     GetButtonByIndex(buttons_, new_selected)->SetSelectedRange(1);
 }
 
-void PageSwitcher::TransitionStarted() {
-}
+void PageSwitcherVertical::TransitionStarted() {}
 
-void PageSwitcher::TransitionChanged() {
+void PageSwitcherVertical::TransitionChanged() {
   const int current_page = model_->selected_page();
   const int target_page = model_->transition().target_page;