MD - add ripples to DL shelf items

chrome/browser/ui/views/download/download_item_view_md.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 "chrome/browser/ui/views/download/download_item_view_md.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <vector>
@@ skipping 39 matching lines @@
 #include "ui/events/event.h"
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/text_elider.h"
 #include "ui/gfx/text_utils.h"
 #include "ui/gfx/vector_icons_public.h"
+#include "ui/views/animation/flood_fill_ink_drop_animation.h"
 #include "ui/views/animation/ink_drop_delegate.h"
+#include "ui/views/animation/ink_drop_hover.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/mouse_constants.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 
 using content::DownloadItem;
@@ skipping 80 matching lines @@
 
 DownloadItemViewMd::DownloadItemViewMd(DownloadItem* download_item,
                                        DownloadShelfView* parent)
     : shelf_(parent),
       status_text_(l10n_util::GetStringUTF16(IDS_DOWNLOAD_STATUS_STARTING)),
       dropdown_state_(NORMAL),
       mode_(NORMAL_MODE),
       dragging_(false),
       starting_drag_(false),
       model_(download_item),
+      ink_drop_delegate_(this, this),
       save_button_(nullptr),
       discard_button_(nullptr),
       dropdown_button_(new BarControlButton(this)),
       dangerous_download_label_(nullptr),
       dangerous_download_label_sized_(false),
       disabled_while_opening_(false),
       creation_time_(base::Time::Now()),
       time_download_warning_shown_(base::Time()),
       weak_ptr_factory_(this) {
   DCHECK(download());
   DCHECK(ui::MaterialDesignController::IsModeMaterial());
   download()->AddObserver(this);
   set_context_menu_controller(this);
 
   dropdown_button_->SetBorder(
       views::Border::CreateEmptyBorder(gfx::Insets(kDropdownBorderWidth)));
   dropdown_button_->set_ink_drop_size(gfx::Size(32, 32));
   AddChildView(dropdown_button_);
 
   LoadIcon();
 
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   font_list_ =
       rb.GetFontList(ui::ResourceBundle::BaseFont).DeriveWithSizeDelta(1);
   status_font_list_ =
       rb.GetFontList(ui::ResourceBundle::BaseFont).DeriveWithSizeDelta(-2);
 
-  body_hover_animation_.reset(new gfx::SlideAnimation(this));

[bruthig, 2016/03/31 19:39:03]

Is this not needed for non-md?

[Evan Stade, 2016/03/31 21:07:38]

no, this file is md-only.

[bruthig, 2016/03/31 22:46:19]

Acknowledged.

-  drop_hover_animation_.reset(new gfx::SlideAnimation(this));
-
   SetAccessibilityFocusable(true);
 
   OnDownloadUpdated(download());
 
   SetDropdownState(NORMAL);
   UpdateColorsFromTheme();
 }
 
 DownloadItemViewMd::~DownloadItemViewMd() {
   StopDownloadProgress();
@@ skipping 168 matching lines @@
   } else {
     width = kStartPadding + DownloadShelf::kProgressIndicatorSize +
             kProgressTextPadding + kTextWidth +
             dropdown_button_->GetPreferredSize().width() + kEndPadding;
   }
 
   return gfx::Size(width, std::max(kDefaultHeight,
                                    2 * kMinimumVerticalPadding + child_height));
 }
 
-// Handle a mouse click and open the context menu if the mouse is
-// over the drop-down region.
 bool DownloadItemViewMd::OnMousePressed(const ui::MouseEvent& event) {
   HandlePressEvent(event, event.IsOnlyLeftMouseButton());
+  if (!IsShowingWarningDialog()) {
+    last_ink_drop_location_ = event.location();
+    ink_drop_delegate_.OnAction(views::InkDropState::ACTION_PENDING);
+  }
   return true;
 }
 
 // Handle drag (file copy) operations.
 bool DownloadItemViewMd::OnMouseDragged(const ui::MouseEvent& event) {
   // Mouse should not activate us in dangerous mode.
   if (IsShowingWarningDialog())
     return true;
 
   if (!starting_drag_) {
     starting_drag_ = true;
     drag_start_point_ = event.location();
   }
   if (dragging_) {
     if (download()->GetState() == DownloadItem::COMPLETE) {
       IconManager* im = g_browser_process->icon_manager();
       gfx::Image* icon = im->LookupIconFromFilepath(
           download()->GetTargetFilePath(), IconLoader::SMALL);
       views::Widget* widget = GetWidget();
       DragDownloadItem(download(), icon,
                        widget ? widget->GetNativeView() : NULL);
+      ink_drop_delegate_.OnAction(views::InkDropState::ACTIVATED);

[bruthig, 2016/03/31 19:39:02]

How come you are leaving a ripple active during a drag operation?  For the
BookmarksButtons we decided to hide the hover/ripple during a drag.  We should
probably be consistent.

[Evan Stade, 2016/03/31 21:07:38]

ok, done

     }
   } else if (ExceededDragThreshold(event.location() - drag_start_point_)) {
     dragging_ = true;
   }
   return true;
 }
 
 void DownloadItemViewMd::OnMouseReleased(const ui::MouseEvent& event) {
   HandleClickEvent(event, event.IsOnlyLeftMouseButton());
+
+  if (!IsShowingWarningDialog()) {
+    ink_drop_delegate_.OnAction(views::InkDropState::HIDDEN);

[bruthig, 2016/03/31 19:39:02]

Does it make sense to move this to the HandleClickEvent() method?

[Evan Stade, 2016/03/31 21:07:38]

Done.

+    ink_drop_delegate_.SetHovered(IsMouseHovered());

[bruthig, 2016/03/31 19:39:02]

Why do you need to set hovered here?  When the HIDDEN animation ends the
InkDropAnimationControllerImpl should fade the hover back in after a delay.  Is
it the delay that you are trying to skip?

[Evan Stade, 2016/03/31 21:07:38]

doesn't seem to really do anything. Removed.

+  }
 }
 
 void DownloadItemViewMd::OnMouseCaptureLost() {
   // Mouse should not activate us in dangerous mode.
-  if (mode_ == DANGEROUS_MODE)
+  if (mode_ != NORMAL_MODE)
     return;
 
   if (dragging_) {
     // Starting a drag results in a MouseCaptureLost.
     dragging_ = false;
     starting_drag_ = false;
   }
 }
 
+void DownloadItemViewMd::OnMouseEntered(const ui::MouseEvent& event) {
+  if (!IsShowingWarningDialog())
+    ink_drop_delegate_.SetHovered(true);

[bruthig, 2016/03/31 19:39:02]

How come the ButtonInkDropAnimation::OnMouseEntered() handler doesn't work here?
 I'm not sure if this is helpful but FYI if an InkDropAnimation is visible then
the InkDropAnimationControllerImpl won't actually make the hover visible even
when SetHovered(true) is called.  Think of it as logical hover versus visibly
hovered.  The InkDropDelegate::SetHovered() controls the logical state and the
InkDropAnimationControllerImpl is responsible for deciding when to actually show
it so that it meshes nicely with the ripple.

Similar applies to OnMouseExited() below.

[Evan Stade, 2016/03/31 21:07:38]

I dunno why I added this. Removed. Added a check for item state in
CreateInkDropHover.

+}
+
+void DownloadItemViewMd::OnMouseExited(const ui::MouseEvent& event) {
+  ink_drop_delegate_.SetHovered(false);
+}
+
 bool DownloadItemViewMd::OnKeyPressed(const ui::KeyEvent& event) {
   // Key press should not activate us in dangerous mode.
   if (IsShowingWarningDialog())
     return true;
 
   if (event.key_code() == ui::VKEY_SPACE ||
       event.key_code() == ui::VKEY_RETURN) {
     // OpenDownload may delete this, so don't add any code after this line.
     OpenDownload();
     return true;
   }
   return false;
 }
 
 bool DownloadItemViewMd::GetTooltipText(const gfx::Point& p,
                                         base::string16* tooltip) const {
   if (IsShowingWarningDialog()) {
     tooltip->clear();
     return false;
   }
 
   tooltip->assign(tooltip_text_);
 
   return true;
 }
 
+void DownloadItemViewMd::OnDragDone() {
+  ink_drop_delegate_.OnAction(views::InkDropState::DEACTIVATED);

[bruthig, 2016/03/31 19:39:03]

FYI On Chrome-Linux I am able to get the ripple to persist when dragging an item
and dropping it in the content area.  This may not be getting called.

[Evan Stade, 2016/03/31 21:07:38]

removed

+}
+
 void DownloadItemViewMd::GetAccessibleState(ui::AXViewState* state) {
   state->name = accessible_name_;
   state->role = ui::AX_ROLE_BUTTON;
   if (model_.IsDangerous())
     state->AddStateFlag(ui::AX_STATE_DISABLED);
   else
     state->AddStateFlag(ui::AX_STATE_HASPOPUP);
 }
 
 void DownloadItemViewMd::OnThemeChanged() {
   UpdateColorsFromTheme();
 }
 
+scoped_ptr<views::InkDropAnimation> DownloadItemViewMd::CreateInkDropAnimation()
+    const {
+  return make_scoped_ptr(new views::FloodFillInkDropAnimation(
+      size(), last_ink_drop_location_,
+      color_utils::DeriveDefaultIconColor(GetTextColor())));
+}
+
+scoped_ptr<views::InkDropHover> DownloadItemViewMd::CreateInkDropHover() const {
+  // TODO(estade): when the item is animating open and the user hovers, the

[bruthig, 2016/03/31 19:39:02]

Is the size still stuck if you mouse off and mouse on again?

[Evan Stade, 2016/03/31 21:07:38]

nope

+  // effect gets stuck at that size and does not grow with the item. Using
+  // GetPreferredSize() instead of size() here does not fix it.
+  return make_scoped_ptr(new views::InkDropHover(
+      size(), kInkDropSmallCornerRadius,
+      GetLocalBounds().CenterPoint(),
+      color_utils::DeriveDefaultIconColor(GetTextColor())));
+}
+
 void DownloadItemViewMd::OnGestureEvent(ui::GestureEvent* event) {
   if (event->type() == ui::ET_GESTURE_TAP_DOWN) {
     HandlePressEvent(*event, true);
     event->SetHandled();
+    if (mode_ == NORMAL_MODE) {
+      last_ink_drop_location_ = event->location();
+      ink_drop_delegate_.OnAction(views::InkDropState::QUICK_ACTION);

[bruthig, 2016/03/31 19:39:02]

I would expect the QUICK_ACTION trigger to be tied closely to the OpenDownload()
action.  Would it make sense to move this into the OpenDownload() call?  The
added benefit there is that a keyboard action will also trigger the ripple,
which is what we want.

[Evan Stade, 2016/03/31 21:07:38]

I only did this because of this comment above the enum value:

  // The quick action for the view has been triggered. e.g. a tap gesture to
  // click a button.

I can add it to OnKeyEvent as well, although I'm not sure how to focus the
download items in the first place.

If I move this to OpenDownload, then clicking with a mouse creates a double
ripple effect.

[bruthig, 2016/03/31 22:46:19]

If both event handling methods delegate to a common function like OpenDownload()
then it should be moved to that common spot.  Feel free to update that enum
comment to something less confusing.
You can use F6 to cycle between the different child Views of the BrowserView ;)
That's odd, I would add some logging or set a breakpoint in
FloodFillInkDropAnimation::AnimateToState() to see what state changes are being
triggered and from where.

It would be nice to have this logging in place with a switch to turn it on/off. 
Is there already some logging framework to do this?  Like 'trace' logs or
something?

As an aside, I was also seeing a double ripple on the drop down button as well.

[Evan Stade, 2016/04/01 19:03:13]

Why is that odd? We're calling OnAction twice in the click case.
see https://chromium.googlesource.com/chromium/src/+/master/base/logging.h

[Evan Stade, 2016/04/01 19:04:08]

you can ignore this comment -- I understand better now.

+    }
     return;
   }
 
   if (event->type() == ui::ET_GESTURE_TAP) {
     HandleClickEvent(*event, true);
     event->SetHandled();
     return;
   }
 
   views::View::OnGestureEvent(event);
@@ skipping 586 matching lines @@
   }
 }
 
 void DownloadItemViewMd::ProgressTimerFired() {
   // Only repaint for the indeterminate size case. Otherwise, we'll repaint only
   // when there's an update notified via OnDownloadUpdated().
   if (model_.PercentComplete() < 0)
     SchedulePaint();
 }
 
-SkColor DownloadItemViewMd::GetTextColor() {
+SkColor DownloadItemViewMd::GetTextColor() const {
   return GetTextColorForThemeProvider(GetThemeProvider());
 }
 
-SkColor DownloadItemViewMd::GetDimmedTextColor() {
+SkColor DownloadItemViewMd::GetDimmedTextColor() const {
   return SkColorSetA(GetTextColor(), 0xC7);
 }