Add MD ink drop ripple to shelf app items

ash/shelf/shelf_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/shelf/shelf_view.h"
 
 #include <algorithm>
 #include <memory>
 
 #include "ash/ash_constants.h"
@@ skipping 400 matching lines @@
 void ShelfView::Init() {
   model_->AddObserver(this);
 
   const ShelfItems& items(model_->items());
   for (ShelfItems::const_iterator i = items.begin(); i != items.end(); ++i) {
     views::View* child = CreateViewForItem(*i);
     child->SetPaintToLayer(true);
     view_model_->Add(child, static_cast<int>(i - items.begin()));
     AddChildView(child);
   }
-  overflow_button_ = new OverflowButton(this, shelf_);
+  overflow_button_ = new OverflowButton(this);
   overflow_button_->set_context_menu_controller(this);
   ConfigureChildView(overflow_button_);
   AddChildView(overflow_button_);
 
   // We'll layout when our bounds change.
 }
 
 void ShelfView::OnShelfAlignmentChanged() {
   overflow_button_->OnShelfAlignmentChanged();
   LayoutToIdealBounds();
@@ skipping 274 matching lines @@
       // to animate back to its correct location.
       AnimateToIdealBounds();
     } else {
       drag_and_drop_view->SetSize(pre_drag_and_drop_size_);
     }
   }
 
   drag_and_drop_shelf_id_ = 0;
 }
 
+bool ShelfView::WillIgnoreEventOnButton(View* view, const ui::Event& event) {
+  if (dragging())
+    return true;
+
+  // Ignore if we are already in a pointer event sequence started with a repost
+  // event.
+  if (is_repost_event_)

[bruthig, 2016/06/09 03:26:00]

The interplay of |is_repost_event_|, |last_pressed_index_|,
|closing_event_time|, and IsRepostEvent() is very difficult to follow.  I'm sure
this could be made much simpler, but perhaps it should not be included in this
change.

WDYT?

[mohsen, 2016/06/09 21:24:51]

Probably, similar names of |is_repost_event_| and IsRepostEvent() which have
different meanings make it confusing. Changed the former to
|is_repost_event_on_same_item_|. Does it look better?

[bruthig, 2016/06/10 15:25:19]

Definitely looks better!

+    return true;
+
+  if (event.flags() & ui::EF_IS_DOUBLE_CLICK)
+    return true;
+
+  // Ignore if this is a repost event on the last pressed shelf item.
+  int index = view_model_->GetIndexOfView(view);
+  if (index == -1)
+    return true;
+  return IsRepostEvent(event) && (last_pressed_index_ == index);
+}
+
 void ShelfView::PointerPressedOnButton(views::View* view,
                                        Pointer pointer,
                                        const ui::LocatedEvent& event) {
   if (drag_view_)
     return;
 
   int index = view_model_->GetIndexOfView(view);
-  if (index == -1)
-    return;
+  if (index == -1 || view_model_->view_size() <= 1)
+    return;  // View is being deleted, ignore request.
 
   ShelfItemDelegate* item_delegate =
       item_manager_->GetShelfItemDelegate(model_->items()[index].id);
-  if (view_model_->view_size() <= 1 || !item_delegate->IsDraggable())
-    return;  // View is being deleted or not draggable, ignore request.
+  if (!item_delegate->IsDraggable())
+    return;  // View is not draggable, ignore request.
 
   // Only when the repost event occurs on the same shelf item, we should ignore
   // the call in ShelfView::ButtonPressed(...).
   is_repost_event_ = IsRepostEvent(event) && (last_pressed_index_ == index);
 
   CHECK_EQ(ShelfButton::kViewClassName, view->GetClassName());
   drag_view_ = static_cast<ShelfButton*>(view);
   drag_origin_ = gfx::Point(event.x(), event.y());
   UMA_HISTOGRAM_ENUMERATION("Ash.ShelfAlignmentUsage",
                             shelf_->SelectValueForShelfAlignment(
@@ skipping 29 matching lines @@
     FinalizeRipOffDrag(false);
     drag_pointer_ = NONE;
     AnimateToIdealBounds();
   }
   // If the drag pointer is NONE, no drag operation is going on and the
   // drag_view can be released.
   if (drag_pointer_ == NONE)
     drag_view_ = nullptr;
 }
 
+void ShelfView::ButtonPressed(views::Button* sender,
+                              const ui::Event& event,
+                              views::InkDrop* ink_drop) {
+  if (!ButtonPressedImpl(sender, event, ink_drop) && !is_repost_event_)
+    ink_drop->AnimateToState(views::InkDropState::HIDDEN);
+}
+
 void ShelfView::LayoutToIdealBounds() {
   if (bounds_animator_->IsAnimating()) {
     AnimateToIdealBounds();
     return;
   }
 
   IdealBounds ideal_bounds;
   CalculateIdealBounds(&ideal_bounds);
   views::ViewModelUtils::SetViewBoundsToIdealBounds(*view_model_);
   overflow_button_->SetBoundsRect(ideal_bounds.overflow_bounds);
@@ skipping 248 matching lines @@
   ShelfItemDelegate* item_delegate = item_manager_->GetShelfItemDelegate(
       model_->items()[start_drag_index_].id);
   if (!item_delegate->IsDraggable()) {
     CancelDrag(-1);
     return;
   }
 
   // Move the view to the front so that it appears on top of other views.
   ReorderChildView(drag_view_, -1);
   bounds_animator_->StopAnimatingView(drag_view_);
+
+  drag_view_->OnDragStarted();
 }
 
 void ShelfView::ContinueDrag(const ui::LocatedEvent& event) {
   // Due to a syncing operation the application might have been removed.
   // Bail if it is gone.
   int current_index = view_model_->GetIndexOfView(drag_view_);
   DCHECK_NE(-1, current_index);
 
   ShelfItemDelegate* item_delegate =
       item_manager_->GetShelfItemDelegate(model_->items()[current_index].id);
@@ skipping 332 matching lines @@
 void ShelfView::UpdateOverflowRange(ShelfView* overflow_view) const {
   const int first_overflow_index = last_visible_index_ + 1;
   const int last_overflow_index = last_hidden_index_;
   DCHECK_LE(first_overflow_index, last_overflow_index);
   DCHECK_LT(last_overflow_index, view_model_->view_size());
 
   overflow_view->first_visible_index_ = first_overflow_index;
   overflow_view->last_visible_index_ = last_overflow_index;
 }
 
+bool ShelfView::ButtonPressedImpl(views::Button* sender,

[bruthig, 2016/06/09 03:25:59]

Consider renaming ButtonPressed as ButtonPressedImpl and keeping the declaration
and definition in the same spot.  It will make the diff easier to read and will
save your name from showing up beside a lot of lines in git blame ;)

[mohsen, 2016/06/09 21:24:51]

Right. I was trying to keep the ordering suggested by style guides (function
overrides after other functions). Now that I removed ButtonPressedImpl() (see
below), I moved ButtonPressed() definition back to its original location which
should make diff easier to read. The declaration can't be moved back as it needs
to be in the public section.

+                                  const ui::Event& event,
+                                  views::InkDrop* ink_drop) {
+  // Do not handle mouse release during drag.

[bruthig, 2016/06/09 03:25:59]

Would it be possible to replace most of these quick returns with a call to
WillIgnoreEventOnButton()?
i.e.
if (sender == overflow_button_) {
  ...
}

if (WillIgnoreEventOnButton(...))
  return false;

{
  ScopedTargetRootWindow scoped_target(
...

[mohsen, 2016/06/09 21:24:51]

It was not possible because for the double-click case we should've returned true
rather than false. However, now that I removed ButtonPressedImpl() (see below),
it is possible. Done.

+  if (dragging())
+    return false;
+
+  if (sender == overflow_button_) {
+    ToggleOverflowBubble();
+    shelf_button_pressed_metric_tracker_.ButtonPressed(
+        event, sender, ShelfItemDelegate::kNoAction);
+    return false;
+  }
+
+  int view_index = view_model_->GetIndexOfView(sender);
+  // May be -1 while in the process of animating closed.
+  if (view_index == -1)
+    return false;
+
+  // If the menu was just closed by the same event as this one, we ignore
+  // the call and don't open the menu again. See crbug.com/343005 for more
+  // detail.
+  if (is_repost_event_)
+    return false;
+
+  // Record the index for the last pressed shelf item.
+  last_pressed_index_ = view_index;
+
+  // Don't activate the item twice on double-click. Otherwise the window starts
+  // animating open due to the first click, then immediately minimizes due to
+  // the second click. The user most likely intended to open or minimize the
+  // item once, not do both.
+  if (event.flags() & ui::EF_IS_DOUBLE_CLICK)
+    return true;

[bruthig, 2016/06/09 03:26:00]

Shouldn't this be a return false?

[mohsen, 2016/06/09 21:24:51]

The return value of the function means whether the ink drop ripple is taken care
of or not. If not, we hide the ripple. HOWEVER, hiding the ripple is not needed
anymore (it was probably needed once, but as the patch developed, the need is
gone). So, I removed ButtonPressedImpl() helper altogether and moved the code
back to ButtonPressed().

+
+  {
+    ScopedTargetRootWindow scoped_target(
+        sender->GetWidget()->GetNativeView()->GetRootWindow());
+    // Slow down activation animations if shift key is pressed.
+    std::unique_ptr<ui::ScopedAnimationDurationScaleMode> slowing_animations;
+    if (event.IsShiftDown()) {
+      slowing_animations.reset(new ui::ScopedAnimationDurationScaleMode(
+          ui::ScopedAnimationDurationScaleMode::SLOW_DURATION));
+    }
+
+    // Collect usage statistics before we decide what to do with the click.
+    switch (model_->items()[view_index].type) {
+      case TYPE_APP_SHORTCUT:
+      case TYPE_WINDOWED_APP:
+      case TYPE_PLATFORM_APP:
+      case TYPE_BROWSER_SHORTCUT:
+        Shell::GetInstance()->metrics()->RecordUserMetricsAction(
+            UMA_LAUNCHER_CLICK_ON_APP);
+        break;
+
+      case TYPE_APP_LIST:
+        Shell::GetInstance()->metrics()->RecordUserMetricsAction(
+            UMA_LAUNCHER_CLICK_ON_APPLIST_BUTTON);
+        break;
+
+      case TYPE_APP_PANEL:
+      case TYPE_DIALOG:
+      case TYPE_IME_MENU:
+        break;
+
+      case TYPE_UNDEFINED:
+        NOTREACHED() << "ShelfItemType must be set.";
+        break;
+    }
+
+    ShelfItemDelegate::PerformedAction performed_action =
+        item_manager_->GetShelfItemDelegate(model_->items()[view_index].id)
+            ->ItemSelected(event);
+
+    shelf_button_pressed_metric_tracker_.ButtonPressed(event, sender,
+                                                       performed_action);
+
+    if (performed_action != ShelfItemDelegate::kNewWindowCreated &&
+        ShowListMenuForView(model_->items()[view_index], sender, event,
+                            ink_drop)) {
+      // Menu is run synchronously, the menu is closed now and we need to go to
+      // the deactivated state.
+      ink_drop->AnimateToState(views::InkDropState::DEACTIVATED);

[bruthig, 2016/06/09 03:26:00]

It might make sense to put the DEACTIVATED call in ShowListMenuForView().

[mohsen, 2016/06/09 21:24:51]

Sure. Done.

+      return true;
+    }
+    ink_drop->AnimateToState(views::InkDropState::ACTION_TRIGGERED);
+  }
+
+  return true;
+}
+
 gfx::Rect ShelfView::GetBoundsForDragInsertInScreen() {
   gfx::Size preferred_size;
   if (is_overflow_mode()) {
     DCHECK(owner_overflow_bubble_);
     gfx::Rect bubble_bounds =
         owner_overflow_bubble_->bubble_view()->GetBubbleBounds();
     preferred_size = bubble_bounds.size();
   } else {
     const int last_button_index = view_model_->view_size() - 1;
     gfx::Rect last_button_bounds =
@@ skipping 251 matching lines @@
 void ShelfView::ShelfItemMoved(int start_index, int target_index) {
   view_model_->Move(start_index, target_index);
   // When cancelling a drag due to a shelf item being added, the currently
   // dragged item is moved back to its initial position. AnimateToIdealBounds
   // will be called again when the new item is added to the |view_model_| but
   // at this time the |view_model_| is inconsistent with the |model_|.
   if (!cancelling_drag_model_changed_)
     AnimateToIdealBounds();
 }
 
-void ShelfView::ButtonPressed(views::Button* sender, const ui::Event& event) {
-  // Do not handle mouse release during drag.
-  if (dragging())
-    return;
-
-  if (sender == overflow_button_) {
-    ToggleOverflowBubble();
-    shelf_button_pressed_metric_tracker_.ButtonPressed(
-        event, sender, ShelfItemDelegate::kNoAction);
-    return;
-  }
-
-  int view_index = view_model_->GetIndexOfView(sender);
-  // May be -1 while in the process of animating closed.
-  if (view_index == -1)
-    return;
-
-  // If the menu was just closed by the same event as this one, we ignore
-  // the call and don't open the menu again. See crbug.com/343005 for more
-  // detail.
-  if (is_repost_event_)
-    return;
-
-  // Record the index for the last pressed shelf item.
-  last_pressed_index_ = view_index;
-
-  // Don't activate the item twice on double-click. Otherwise the window starts
-  // animating open due to the first click, then immediately minimizes due to
-  // the second click. The user most likely intended to open or minimize the
-  // item once, not do both.
-  if (event.flags() & ui::EF_IS_DOUBLE_CLICK)
-    return;
-
-  {
-    ScopedTargetRootWindow scoped_target(
-        sender->GetWidget()->GetNativeView()->GetRootWindow());
-    // Slow down activation animations if shift key is pressed.
-    std::unique_ptr<ui::ScopedAnimationDurationScaleMode> slowing_animations;
-    if (event.IsShiftDown()) {
-      slowing_animations.reset(new ui::ScopedAnimationDurationScaleMode(
-            ui::ScopedAnimationDurationScaleMode::SLOW_DURATION));
-    }
-
-    // Collect usage statistics before we decide what to do with the click.
-    switch (model_->items()[view_index].type) {
-      case TYPE_APP_SHORTCUT:
-      case TYPE_WINDOWED_APP:
-      case TYPE_PLATFORM_APP:
-      case TYPE_BROWSER_SHORTCUT:
-        Shell::GetInstance()->metrics()->RecordUserMetricsAction(
-            UMA_LAUNCHER_CLICK_ON_APP);
-        break;
-
-      case TYPE_APP_LIST:
-        Shell::GetInstance()->metrics()->RecordUserMetricsAction(
-            UMA_LAUNCHER_CLICK_ON_APPLIST_BUTTON);
-        break;
-
-      case TYPE_APP_PANEL:
-      case TYPE_DIALOG:
-      case TYPE_IME_MENU:
-        break;
-
-      case TYPE_UNDEFINED:
-        NOTREACHED() << "ShelfItemType must be set.";
-        break;
-    }
-
-    ShelfItemDelegate::PerformedAction performed_action =
-        item_manager_->GetShelfItemDelegate(model_->items()[view_index].id)
-            ->ItemSelected(event);
-
-    shelf_button_pressed_metric_tracker_.ButtonPressed(event, sender,
-                                                       performed_action);
-
-    if (performed_action != ShelfItemDelegate::kNewWindowCreated)
-      ShowListMenuForView(model_->items()[view_index], sender, event);
-  }
-}
-
-void ShelfView::ShowListMenuForView(const ShelfItem& item,
+bool ShelfView::ShowListMenuForView(const ShelfItem& item,
                                     views::View* source,
-                                    const ui::Event& event) {
+                                    const ui::Event& event,
+                                    views::InkDrop* ink_drop) {
   ShelfItemDelegate* item_delegate =
       item_manager_->GetShelfItemDelegate(item.id);
   std::unique_ptr<ui::MenuModel> list_menu_model(
       item_delegate->CreateApplicationMenu(event.flags()));
 
   // Make sure we have a menu and it has at least two items in addition to the
   // application title and the 3 spacing separators.
   if (!list_menu_model.get() || list_menu_model->GetItemCount() <= 5)
-    return;
+    return false;
 
+  ink_drop->AnimateToState(views::InkDropState::ACTIVATED);
   context_menu_id_ = item.id;
   ShowMenu(list_menu_model.get(), source, gfx::Point(), false,
            ui::GetMenuSourceTypeForEvent(event));
+  return true;
 }
 
 void ShelfView::ShowContextMenuForView(views::View* source,
                                        const gfx::Point& point,
                                        ui::MenuSourceType source_type) {
+  last_pressed_index_ = -1;
+
   const ShelfItem* item = ShelfItemForView(source);
   if (!item) {
     Shell::GetInstance()->ShowContextMenu(point, source_type);
     return;
   }
 
   std::unique_ptr<ui::MenuModel> context_menu_model(
       Shell::GetInstance()->delegate()->CreateContextMenu(shelf_, item));
   if (!context_menu_model)
     return;
@@ skipping 90 matching lines @@
     }
   }
 }
 
 bool ShelfView::IsRepostEvent(const ui::Event& event) {
   if (closing_event_time_.is_zero())
     return false;
 
   base::TimeDelta delta =
       base::TimeDelta(event.time_stamp() - closing_event_time_);
-  closing_event_time_ = base::TimeDelta();
   // If the current (press down) event is a repost event, the time stamp of
   // these two events should be the same.
-  return (delta.InMilliseconds() == 0);
+  return delta.InMilliseconds() == 0;
 }
 
 const ShelfItem* ShelfView::ShelfItemForView(const views::View* view) const {
   const int view_index = view_model_->GetIndexOfView(view);
   return (view_index < 0) ? nullptr : &(model_->items()[view_index]);
 }
 
 int ShelfView::CalculateShelfDistance(const gfx::Point& coordinate) const {
   const gfx::Rect bounds = GetBoundsInScreen();
   int distance = shelf_->SelectValueForShelfAlignment(
       bounds.y() - coordinate.y(), coordinate.x() - bounds.right(),
       bounds.x() - coordinate.x());
   return distance > 0 ? distance : 0;
 }
 
 }  // namespace ash