Adjust BrowserActionsContainer drag and drop to work for overflow

chrome/browser/ui/views/toolbar/browser_actions_container.cc

 // Copyright 2013 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/toolbar/browser_actions_container.h"
 
 #include "base/compiler_specific.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ skipping 84 matching lines @@
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ChevronMenuButton);
 };
 
 }  // namespace
 
 // static
 bool BrowserActionsContainer::disable_animations_during_testing_ = false;
 
+struct BrowserActionsContainer::DropPosition {
+  DropPosition(size_t row, size_t icon_in_row);
+  ~DropPosition();
+
+  // The (0-indexed) row into which the action will be dropped.
+  size_t row;
+
+  // The (0-indexed) icon in the row before the action will be dropped.
+  size_t icon_in_row;
+};
+
+BrowserActionsContainer::DropPosition::DropPosition(
+    size_t row, size_t icon_in_row)
+    : row(row), icon_in_row(icon_in_row) {
+}
+
+BrowserActionsContainer::DropPosition::~DropPosition() {
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserActionsContainer
 
 BrowserActionsContainer::BrowserActionsContainer(
     Browser* browser,
     View* owner_view,
     BrowserActionsContainer* main_container)
     : profile_(browser->profile()),
       browser_(browser),
       owner_view_(owner_view),
       main_container_(main_container),
       popup_(NULL),
       popup_button_(NULL),
       model_(NULL),
       container_width_(0),
       resize_area_(NULL),
       chevron_(NULL),
       overflow_menu_(NULL),
       suppress_chevron_(false),
       resize_amount_(0),
       animation_target_size_(0),
-      drop_indicator_position_(-1),
       task_factory_(this),
       show_menu_task_factory_(this) {
   set_id(VIEW_ID_BROWSER_ACTION_TOOLBAR);
 
   model_ = extensions::ExtensionToolbarModel::Get(browser->profile());
   if (model_)
     model_->AddObserver(this);
 
   bool overflow_experiment =
       extensions::FeatureSwitch::extension_action_redesign()->IsEnabled();
@@ skipping 215 matching lines @@
       // Ensure that any browser actions shown in the main view are hidden in
       // the overflow view.
       browser_action_views_[i]->SetVisible(false);
     }
 
     for (size_t i = main_container_->VisibleBrowserActionsAfterAnimation();
          i < browser_action_views_.size(); ++i) {
       BrowserActionView* view = browser_action_views_[i];
       size_t index = i - main_container_->VisibleBrowserActionsAfterAnimation();
       int row_index = static_cast<int>(index) / kIconsPerMenuRow;
-      int x = (index * IconWidth(true)) -
+      int x = kItemSpacing + (index * IconWidth(true)) -

[Devlin, 2014/07/17 22:26:30]

We need this so that we can see the drop indicator to the left of the first icon
in a row.

           (row_index * IconWidth(true) * kIconsPerMenuRow);
       gfx::Rect rect_bounds(
           x, IconHeight() * row_index, icon_width, IconHeight());
       view->SetBoundsRect(rect_bounds);
       view->SetVisible(true);
     }
   } else {
     for (BrowserActionViews::const_iterator it = browser_action_views_.begin();
          it < browser_action_views_.end(); ++it) {
       BrowserActionView* view = *it;
@@ skipping 27 matching lines @@
 int BrowserActionsContainer::OnDragUpdated(
     const ui::DropTargetEvent& event) {
   // First check if we are above the chevron (overflow) menu.
   if (GetEventHandlerForPoint(event.location()) == chevron_) {
     if (!show_menu_task_factory_.HasWeakPtrs() && !overflow_menu_)
       StartShowFolderDropMenuTimer();
     return ui::DragDropTypes::DRAG_MOVE;
   }
   StopShowFolderDropMenuTimer();
 
-  // TODO(devlin): This calculation needs to take 'overflow' mode into account
-  // once the wrench menu becomes a drag target for browser action icons.
-
   // Figure out where to display the indicator.  This is a complex calculation:
 
   // First, we figure out how much space is to the left of the icon area, so we
-  // can calculate the true offset into the icon area.
-  int width_before_icons = ToolbarView::kStandardSpacing;
-  if (chevron_ && base::i18n::IsRTL()) {
-    width_before_icons +=
-        chevron_->GetPreferredSize().width() + kChevronSpacing;
-  }
+  // can calculate the true offset into the icon area. The easiest way to do
+  // this is to just find where the first icon starts.
+  int width_before_icons =
+      browser_action_views_[GetFirstVisibleIconIndex()]->x();

[Finnur, 2014/07/18 10:52:41]

Will this work in RTL?

[Devlin, 2014/07/18 17:58:20]

Nope!  But it does now. :)

   int offset_into_icon_area = event.x() - width_before_icons;
 
+  // Next, figure out what row we're on. This only matters for overflow mode,
+  // but the calculation is the same for both.
+  int row_index = event.y() / IconHeight();
+
+  // Sanity check - we should never be on a different row in the main container.
+  DCHECK(in_overflow_mode() || row_index == 0);
+
   // Next, we determine which icon to place the indicator in front of.  We want
   // to place the indicator in front of icon n when the cursor is between the
   // midpoints of icons (n - 1) and n.  To do this we take the offset into the
   // icon area and transform it as follows:
   //
   // Real icon area:
   //   0   a     *  b        c
   //   |   |        |        |
   //   |[IC|ON]  [IC|ON]  [IC|ON]
   // We want to be before icon 0 for 0 < x <= a, icon 1 for a < x <= b, etc.
   // Here the "*" represents the offset into the icon area, and since it's
   // between a and b, we want to return "1".
   //
   // Transformed "icon area":
   //   0        a     *  b        c
   //   |        |        |        |
   //   |[ICON]  |[ICON]  |[ICON]  |
   // If we shift both our offset and our divider points later by half an icon
   // plus one spacing unit, then it becomes very easy to calculate how many
   // divider points we've passed, because they're the multiples of "one icon
   // plus padding".
   int before_icon_unclamped = (offset_into_icon_area + (IconWidth(false) / 2) +
       kItemSpacing) / IconWidth(true);
 
+  // We need to figure out how many icons are visible on the relevant row.
+  // In the main container, this will just be the visible actions.
+  int visible_icons_on_row = VisibleBrowserActionsAfterAnimation();
+  if (in_overflow_mode()) {
+    // If this is the final row of the overflow, then this is the remainder of
+    // visible icons. Otherwise, it's a full row (kIconsPerRow).
+    visible_icons_on_row =
+        row_index == (visible_icons_on_row / kIconsPerMenuRow) ?
+            visible_icons_on_row % kIconsPerMenuRow :
+            kIconsPerMenuRow;
+  }
+
   // Because the user can drag outside the container bounds, we need to clamp to
   // the valid range.  Note that the maximum allowable value is (num icons), not
   // (num icons - 1), because we represent the indicator being past the last
   // icon as being "before the (last + 1) icon".
-  int before_icon = std::min(std::max(before_icon_unclamped, 0),
-                             static_cast<int>(VisibleBrowserActions()));
+  int before_icon_in_row =
+      std::min(std::max(before_icon_unclamped, 0), visible_icons_on_row);

[Devlin, 2014/07/17 22:26:30]

These calculations plus using a row/icon_on_row struct have the nice trait of
"just working" in funny edge cases, like how dropping an icon to the right of
the last icon in row 1 is actionably the same as dropping it to the left of the
first icon in row 2, but needs to be displayed differently with the drop
indicator.

 
-  // Now we convert back to a pixel offset into the container.  We want to place
-  // the center of the drop indicator at the midpoint of the space before our
-  // chosen icon.
-  SetDropIndicator(width_before_icons + (before_icon * IconWidth(true)) -
-      (kItemSpacing / 2));
+  SetDropPosition(row_index, before_icon_in_row);
 
   return ui::DragDropTypes::DRAG_MOVE;
 }
 
 void BrowserActionsContainer::OnDragExited() {
   StopShowFolderDropMenuTimer();
-  drop_indicator_position_ = -1;
+  ResetDropPosition();
   SchedulePaint();
 }
 
 int BrowserActionsContainer::OnPerformDrop(
     const ui::DropTargetEvent& event) {
   BrowserActionDragData data;
   if (!data.Read(event.data()))
     return ui::DragDropTypes::DRAG_NONE;
 
   // Make sure we have the same view as we started with.
   DCHECK_EQ(browser_action_views_[data.index()]->button()->extension()->id(),
             data.id());
   DCHECK(model_);
 
-  size_t i = 0;
-  for (; i < browser_action_views_.size(); ++i) {
-    int view_x = browser_action_views_[i]->GetMirroredBounds().x();
-    if (!browser_action_views_[i]->visible() ||
-        (base::i18n::IsRTL() ? (view_x < drop_indicator_position_) :
-            (view_x >= drop_indicator_position_))) {
-      // We have reached the end of the visible icons or found one that has a
-      // higher x position than the drop point.
-      break;
-    }
-  }
-
+  size_t i =
+      drop_position_->row * kIconsPerMenuRow + drop_position_->icon_in_row;
+  if (in_overflow_mode())
+    i += GetFirstVisibleIconIndex();
   // |i| now points to the item to the right of the drop indicator*, which is
   // correct when dragging an icon to the left. When dragging to the right,
   // however, we want the icon being dragged to get the index of the item to
   // the left of the drop indicator, so we subtract one.
   // * Well, it can also point to the end, but not when dragging to the left. :)
   if (i > data.index())
     --i;
 
   if (profile_->IsOffTheRecord())
     i = model_->IncognitoIndexToOriginal(i);
@@ skipping 180 matching lines @@
 
   for (BrowserActionViews::iterator it = browser_action_views_.begin();
        it != browser_action_views_.end(); ++it) {
     BrowserActionButton* button = (*it)->button();
     if (button && button->extension() == extension)
       return ShowPopup(button, ExtensionPopup::SHOW, should_grant);
   }
   return false;
 }
 
+size_t BrowserActionsContainer::GetFirstVisibleIconIndex() const {
+  return in_overflow_mode() ? model_->GetVisibleIconCount() : 0;
+}
+
 void BrowserActionsContainer::HidePopup() {
   // Remove this as an observer and clear |popup_| and |popup_button_| here,
   // since we might change them before OnWidgetDestroying() gets called.
   if (popup_) {
     popup_->GetWidget()->RemoveObserver(this);
     popup_->GetWidget()->Close();
     popup_ = NULL;
   }
   if (popup_button_) {
     popup_button_->SetButtonNotPushed();
@@ skipping 17 matching lines @@
 void BrowserActionsContainer::OnPaint(gfx::Canvas* canvas) {
   // If the views haven't been initialized yet, wait for the next call to
   // paint (one will be triggered by entering highlight mode).
   if (model_->is_highlighting() && !browser_action_views_.empty()) {
     views::Painter::PaintPainterAt(
         canvas, highlight_painter_.get(), GetLocalBounds());
   }
 
   // TODO(sky/glen): Instead of using a drop indicator, animate the icons while
   // dragging (like we do for tab dragging).
-  if (drop_indicator_position_ > -1) {
+  if (drop_position_.get()) {
     // The two-pixel width drop indicator.
     static const int kDropIndicatorWidth = 2;
-    gfx::Rect indicator_bounds(
-        drop_indicator_position_ - (kDropIndicatorWidth / 2),
-        0,
-        kDropIndicatorWidth,
-        height());
+
+    // Convert back to a pixel offset into the container. We want to place
+    // the center of the drop indicator at the midpoint of the space before our
+    // chosen icon.

[Finnur, 2014/07/18 10:52:41]

Nit: "Chosen icon" is a bit weird here, because you are choosing an empty space
between two icons, or in the case of edges (before/after the first/last icon). 

[Devlin, 2014/07/18 17:58:20]

Done.

+    // This is:
+    //     The width before the first icon, plus
+    //     The width of all icons before the target icon in the row, minus
+    //     Half the spacing between the target icon and the previous icon, minus
+    //     Half the width of the drop indicator, so it's centered.
+    int drop_indicator_x =
+        browser_action_views_[GetFirstVisibleIconIndex()]->x() +
+        drop_position_->icon_in_row * IconWidth(true) -
+        kItemSpacing / 2 -
+        kDropIndicatorWidth / 2;
+
+    // The y is simply the start of the row (which for the main container is
+    // always 0).
+    int drop_indicator_y = IconHeight() * drop_position_->row;
+
+    gfx::Rect indicator_bounds(drop_indicator_x,
+                               drop_indicator_y,
+                               kDropIndicatorWidth,
+                               IconHeight());
 
     // Color of the drop indicator.
     static const SkColor kDropIndicatorColor = SK_ColorBLACK;
     canvas->FillRect(indicator_bounds, kDropIndicatorColor);
   }
 }
 
 void BrowserActionsContainer::OnThemeChanged() {
   LoadImages();
 }
@@ skipping 190 matching lines @@
 void BrowserActionsContainer::StartShowFolderDropMenuTimer() {
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&BrowserActionsContainer::ShowDropFolder,
                  show_menu_task_factory_.GetWeakPtr()),
       base::TimeDelta::FromMilliseconds(views::GetMenuShowDelay()));
 }
 
 void BrowserActionsContainer::ShowDropFolder() {
   DCHECK(!overflow_menu_);
-  SetDropIndicator(-1);
+  ResetDropPosition();
   overflow_menu_ =
       new BrowserActionOverflowMenuController(this,
                                               browser_,
                                               chevron_,
                                               browser_action_views_,
                                               VisibleBrowserActions(),
                                               true);
   overflow_menu_->set_observer(this);
   overflow_menu_->RunMenu(GetWidget());
 }
 
-void BrowserActionsContainer::SetDropIndicator(int x_pos) {
-  if (drop_indicator_position_ != x_pos) {
-    drop_indicator_position_ = x_pos;
+void BrowserActionsContainer::SetDropPosition(size_t row, size_t icon_in_row) {
+  if (!drop_position_.get() ||
+      !(drop_position_->row == row &&
+        drop_position_->icon_in_row == icon_in_row)) {
+    drop_position_.reset(new DropPosition(row, icon_in_row));
     SchedulePaint();
   }
 }
 
+void BrowserActionsContainer::ResetDropPosition() {
+  drop_position_.reset();
+}
+
 int BrowserActionsContainer::IconCountToWidth(int icons,
                                               bool display_chevron) const {
   if (icons < 0)
     icons = browser_action_views_.size();
   if ((icons == 0) && !display_chevron)
     return ToolbarView::kStandardSpacing;
   int icons_size =
       (icons == 0) ? 0 : ((icons * IconWidth(true)) - kItemSpacing);
   int chevron_size = chevron_ && display_chevron ?
       (kChevronSpacing + chevron_->GetPreferredSize().width()) : 0;
@@ skipping 87 matching lines @@
                                      views::BubbleBorder::TOP_RIGHT,
                                      show_action);
   popup_->GetWidget()->AddObserver(this);
   popup_button_ = button;
 
   // Only set button as pushed if it was triggered by a user click.
   if (should_grant)
     popup_button_->SetButtonPushed();
   return true;
 }