Rework app list item drag zones.

ui/app_list/views/apps_grid_view.h

 // Copyright (c) 2012 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.
 
 #ifndef UI_APP_LIST_VIEWS_APPS_GRID_VIEW_H_
 #define UI_APP_LIST_VIEWS_APPS_GRID_VIEW_H_
 
 #include <set>
 #include <string>
 
@@ skipping 217 matching lines @@
   struct Index {
     Index() : page(-1), slot(-1) {}
     Index(int page, int slot) : page(page), slot(slot) {}
 
     bool operator==(const Index& other) const {
       return page == other.page && slot == other.slot;
     }
     bool operator!=(const Index& other) const {
       return page != other.page || slot != other.slot;
     }
+    bool operator<(const Index& other) const {
+      if (page == other.page)

[tapted, 2014/09/11 08:08:55]

optional-nit: This is fine, but I usually write these like

if (page != other.page)
  return page < other.page;

return slot < other.slot;


it means if you have more than 2 axes you don't have to nest things, and you
don't have to mix axes in each condition/statement.

[calamity, 2014/09/15 03:26:11]

Done.

+        return slot < other.slot;
+
+      return page < other.page;
+    }
 
     int page;  // Which page an item view is on.
     int slot;  // Which slot in the page an item view is in.
   };
 
   int tiles_per_page() const { return cols_ * rows_per_page_; }
 
   // Updates from model.
   void Update();
 
@@ skipping 11 matching lines @@
   // struct above with page/slot info of where to display the item.
   Index GetIndexFromModelIndex(int model_index) const;
   int GetModelIndexFromIndex(const Index& index) const;
 
   void SetSelectedItemByIndex(const Index& index);
   bool IsValidIndex(const Index& index) const;
 
   Index GetIndexOfView(const views::View* view) const;
   views::View* GetViewAtIndex(const Index& index) const;
 
+  // Gets the index of the AppListItemView in the view model.

[tapted, 2014/09/11 08:08:55]

`in the view model.` -> `at the end of the view model.`

[calamity, 2014/09/15 03:26:11]

Done.

+  Index GetLastViewIndex() const;
+
   void MoveSelected(int page_delta, int slot_x_delta, int slot_y_delta);
 
   void CalculateIdealBounds();
   void AnimateToIdealBounds();
 
   // Invoked when the given |view|'s current bounds and target bounds are on
   // different rows. To avoid moving diagonally, |view| would be put into a
   // slot prior |target| and fade in while moving to |target|. In the meanwhile,
   // a layer copy of |view| would start at |current| and fade out while moving
   // to succeeding slot of |current|. |animate_current| controls whether to run
@@ skipping 104 matching lines @@
   // If |hide| is set the view will get hidden, otherwise it gets shown.
   void SetViewHidden(views::View* view, bool hide, bool immediate);
 
   // Whether the folder drag-and-drop UI should be enabled.
   bool EnableFolderDragDropUI();
 
   // Whether target specified by |drap_target| can accept more items to be
   // dropped into it.
   bool CanDropIntoTarget(const Index& drop_target);
 
-  // Calculates the visual index of the nearest tile for which |drag_view_|
-  // enters either its re-ordering or folder dropping circle.
-  void CalculateNearestTileForDragView();
-
-  // Calculates |nearest_tile| in which |vertex| of the |drag_view| is
-  // enclosed.
-  // *|nearest_tile| and *|d_min| will be updated based on the calculation.
-  // *|d_min| is the distance between |nearest_tile| and |drag_view_|.
-  void CalculateNearestTileForVertex(
-      const gfx::Point& vertex, Index* nearest_tile, int* d_min);
-
-  // Returns the bounds of the tile in which |point| is enclosed if there
-  // is a valid item sits on the tile.
-  gfx::Rect GetTileBoundsForPoint(const gfx::Point& point, Index* tile_index);
-
   // Returns the size of the entire tile grid.
   gfx::Size GetTileGridSize() const;
 
-  // Gets the expected bounds of a tile located at |row| and |col| on the
-  // current page.
+  // Returns the slot number which the given |point| falls into.

[tapted, 2014/09/11 08:08:55]

nit: This comment suggests to me that if |point| is not over a slot it will
return -1 or something. Still reading through the CL. I came back: could this
return an AppsGridView::Index with page == current page? - might neaten things
up a bit

[calamity, 2014/09/15 03:26:11]

Returns an Index, changed description.

+  int GetNearestTileSlotForPoint(const gfx::Point& point);

[tapted, 2014/09/11 08:08:55]

nit: declare const?

[calamity, 2014/09/15 03:26:11]

Done.

+
+  // Gets the bounds of the tile located at |slot| on current page.
+  gfx::Rect GetExpectedTileBounds(int slot) const;

[tapted, 2014/09/11 08:08:55]

maybe take an AppsGridView::Index ? looks like it's always called with
index.slot

[calamity, 2014/09/15 03:26:11]

I think slot is better since using an Index kind of implies that this will
consider the page when calculating the bounds.

+
+  // Gets the bounds of the tile located at |row| and |col| on current page.

[tapted, 2014/09/11 08:08:55]

nit: on -> on the

[calamity, 2014/09/15 03:26:11]

Done.

   gfx::Rect GetExpectedTileBounds(int row, int col) const;
 
-  // Returns true if the slot of |index| is the last possible slot to drop
-  // an item, i.e. first empty slot next to the last item on the last page.
-  bool IsLastPossibleDropTarget(const Index& index) const;
-
   // Gets the item view located at |slot| on the current page. If there is
   // no item located at |slot|, returns NULL.
   views::View* GetViewAtSlotOnCurrentPage(int slot);
 
   // Sets state of the view with |target_index| to |is_target_folder| for
   // dropping |drag_view_|.
   void SetAsFolderDroppingTarget(const Index& target_index,
                                  bool is_target_folder);
 
   // Invoked when |reorder_timer_| fires to show re-order preview UI.
@@ skipping 77 matching lines @@
 #if defined(OS_WIN)
   // Created when a drag is started (ie: drag exceeds the drag threshold), but
   // not Run() until supplied with a shortcut path.
   scoped_refptr<SynchronousDrag> synchronous_drag_;
 
   // Whether to use SynchronousDrag to support dropping to task bar etc.
   bool use_synchronous_drag_;
 #endif
 
   Pointer drag_pointer_;
+
+  // The index where a folder drop is being attempted.
+  Index folder_drop_target_;
+
+  // The index where a reorder is being attempted.
   Index reorder_drop_target_;
-  Index folder_drop_target_;
+
+  // The index where a reorder's blank placeholder is located.
+  Index reorder_placeholder_;

[tapted, 2014/09/11 08:08:55]

Can you say more about this in the comment - I'm not sure why the placeholder is
needed. And, e.g., how is this different to drag_view_init_index_?

[calamity, 2014/09/15 03:26:11]

Done.

+
+  // The type of drop being attempted.
   DropAttempt drop_attempt_;
 
   // Timer for re-ordering the |drop_target_| and |drag_view_|.
   base::OneShotTimer<AppsGridView> reorder_timer_;
 
   // Timer for dropping |drag_view_| into the folder containing
   // the |drop_target_|.
   base::OneShotTimer<AppsGridView> folder_dropping_timer_;
 
   // Timer for dragging a folder item out of folder container ink bubble.
@@ skipping 30 matching lines @@
 
   // True if the drag_view_ item is a folder item being dragged for reparenting.
   bool dragging_for_reparent_item_;
 
   DISALLOW_COPY_AND_ASSIGN(AppsGridView);
 };
 
 }  // namespace app_list
 
 #endif  // UI_APP_LIST_VIEWS_APPS_GRID_VIEW_H_