Move app list

ash/launcher/launcher_view.cc

 // 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.
 
 #include "ash/launcher/launcher_view.h"
 
 #include <algorithm>
 
 #include "ash/launcher/app_list_button.h"
 #include "ash/launcher/launcher_button.h"
@@ skipping 375 matching lines @@
       view_model_->set_ideal_bounds(i, gfx::Rect(x, y, 0, 0));
       continue;
     }
 
     view_model_->set_ideal_bounds(i, gfx::Rect(
         x, y, kLauncherPreferredSize, kLauncherPreferredSize));
     x = primary_axis_coordinate(x + kLauncherPreferredSize + kButtonSpacing, 0);
     y = primary_axis_coordinate(0, y + kLauncherPreferredSize + kButtonSpacing);
   }
 
-  int app_list_index = view_model_->view_size() - 1;
+  int last_view_index = view_model_->view_size() - 1;
   if (is_overflow_mode()) {
-    last_visible_index_ = app_list_index - 1;
+    last_visible_index_ = last_view_index;
     for (int i = 0; i < view_model_->view_size(); ++i) {
       view_model_->view_at(i)->SetVisible(
           i >= first_visible_index_ && i <= last_visible_index_);
     }
     return;
   }
 
   if (view_model_->view_size() > 0) {
     // Makes the first launcher button include the leading inset.
     view_model_->set_ideal_bounds(0, gfx::Rect(gfx::Size(
         primary_axis_coordinate(leading_inset() + kLauncherPreferredSize,
                                 kLauncherPreferredSize),
         primary_axis_coordinate(kLauncherPreferredSize,
                                 leading_inset() + kLauncherPreferredSize))));
   }
 
   bounds->overflow_bounds.set_size(
       gfx::Size(kLauncherPreferredSize, kLauncherPreferredSize));
   last_visible_index_ = DetermineLastVisibleIndex(
       available_size - leading_inset() - kLauncherPreferredSize -
       kButtonSpacing - kLauncherPreferredSize);
-  bool show_overflow = (last_visible_index_ + 1 < app_list_index);
+  bool show_overflow = (last_visible_index_ < last_view_index);
 
   for (int i = 0; i < view_model_->view_size(); ++i) {
-    view_model_->view_at(i)->SetVisible(
-        i == app_list_index || i <= last_visible_index_);
+    view_model_->view_at(i)->SetVisible(i <= last_visible_index_);
   }
 
   overflow_button_->SetVisible(show_overflow);
   if (show_overflow) {
     DCHECK_NE(0, view_model_->view_size());
     if (last_visible_index_ == -1) {
       x = primary_axis_coordinate(leading_inset(), 0);
       y = primary_axis_coordinate(0, leading_inset());
     } else {
       x = primary_axis_coordinate(
           view_model_->ideal_bounds(last_visible_index_).right(), 0);
       y = primary_axis_coordinate(0,
           view_model_->ideal_bounds(last_visible_index_).bottom());
     }
-    gfx::Rect app_list_bounds = view_model_->ideal_bounds(app_list_index);
+    gfx::Rect last_view_bounds = view_model_->ideal_bounds(last_view_index);
     bounds->overflow_bounds.set_x(x);
     bounds->overflow_bounds.set_y(y);
     x = primary_axis_coordinate(x + kLauncherPreferredSize + kButtonSpacing, 0);
     y = primary_axis_coordinate(0, y + kLauncherPreferredSize + kButtonSpacing);
-    app_list_bounds.set_x(x);
-    app_list_bounds.set_y(y);
-    view_model_->set_ideal_bounds(app_list_index, app_list_bounds);
+    last_view_bounds.set_x(x);
+    last_view_bounds.set_y(y);
+    view_model_->set_ideal_bounds(last_view_index, last_view_bounds);

[sky, 2012/09/11 00:18:32]

Is this code needed anymore (438 and 441-445)? It was necessary when the app
list button was at the end and we were forcing moving it's visible position.

[DaveMoore, 2012/09/11 17:07:00]

Removed
On 2012/09/11 00:18:32, sky wrote:

   } else {
     if (overflow_bubble_.get())
       overflow_bubble_->Hide();
   }
 }
 
 int LauncherView::DetermineLastVisibleIndex(int max_value) {
   int index = view_model_->view_size() - 1;
   while (index >= 0 &&
          primary_axis_coordinate(
@@ skipping 258 matching lines @@
   drag_pointer_ = NONE;
   drag_view_ = NULL;
   if (drag_view_index == modified_index) {
     // The view that was being dragged is being modified. Don't do anything.
     return modified_index;
   }
   if (!was_dragging)
     return modified_index;
 
   // Restore previous position, tracking the position of the modified view.
+  model_->Move(drag_view_index, start_drag_index_);
+
+  // Adding a new view.
+  if (modified_index == view_model_->view_size())

[sky, 2012/09/11 00:18:32]

Your comment implies that if 727 is true then we're adding a new item. But isn't
727 only true if adding an item at the end?

[DaveMoore, 2012/09/11 17:07:00]

Updated comment. There's a case that existed before where we return -1 if we're
adding a new view to the middle while dragging. @mattias, I believe this is your
code. Should it be -1?

On 2012/09/11 00:18:32, sky wrote:

[Mattias Nissler (ping if slow), 2012/09/12 12:08:18]

I fail to follow the case you're describing. If we're inserting something to the
middle, we have 0 <= modified_index < view_model_->view_size(), and the code
below will not return -1.

The return value should be the new position of the view previously at
modified_index.

The check you added is required good to cover the case of appending, in which
case we would have incorrectly returned -1, whereas the insertion position after
cancelling the drag is still at the end of the model. This hasn't been a problem
thus far since the app list was last, so we never appended.

I suggest you make the check a >= though (which should never happen, but
returning -1 in these cases is certainly not correct).

Regarding the code below, would you mind renaming |removed_view| to
|modified_view|? I should have changed it when moving the code but obviously
missed it.

+    return modified_index;
+
   views::View* removed_view =
       (modified_index >= 0) ? view_model_->view_at(modified_index) : NULL;
-  model_->Move(drag_view_index, start_drag_index_);
   return removed_view ? view_model_->GetIndexOfView(removed_view) : -1;
 }
 
 gfx::Size LauncherView::GetPreferredSize() {
   IdealBounds ideal_bounds;
   CalculateIdealBounds(&ideal_bounds);
 
-  const int app_list_index = view_model_->view_size() - 1;
-  const int last_button_index = is_overflow_mode() ?
-      last_visible_index_ : app_list_index;
   const gfx::Rect last_button_bounds =
-      last_button_index  >= first_visible_index_ ?
-          view_model_->view_at(last_button_index)->bounds() :
+      last_visible_index_  >= first_visible_index_ ?
+          view_model_->view_at(last_visible_index_)->bounds() :
           gfx::Rect(gfx::Size(kLauncherPreferredSize,
                               kLauncherPreferredSize));
 
   if (is_horizontal_alignment()) {
     return gfx::Size(last_button_bounds.right() + leading_inset(),
                      kLauncherPreferredSize);
   }
 
   return gfx::Size(kLauncherPreferredSize,
                    last_button_bounds.bottom() + leading_inset());
@@ skipping 317 matching lines @@
   FOR_EACH_OBSERVER(LauncherIconObserver, observers_,
                     OnLauncherIconPositionsChanged());
   PreferredSizeChanged();
 }
 
 void LauncherView::OnBoundsAnimatorDone(views::BoundsAnimator* animator) {
 }
 
 }  // namespace internal
 }  // namespace ash