Skip to content

Commit

Permalink
Remove InkDropHostView
Browse files Browse the repository at this point in the history
This makes existing subclasses of InkDropHostView inherit from View
instead. They also install their own InkDrops as direct class members.

The largest affected part is the Button subhierarchy, in which
Button::ink_drop() now mimics the existing Button::focus_ring().

TEST=Open Chrome OS and long-press the system tray button at the right \
     bottom, to see the ripple behavior does not change before and \
     after the change.
TEST=Create a notification by using \
     https://tests.peter.sh/notification-generator/, long-press the \
     notification and see the ripple animation for the blocking \
     settings does not change before and after the change.

Bug: 931964
Change-Id: If1f38afaa00a9a8dcac95590e6ae3037b4216b64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2885426
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#882406}
  • Loading branch information
pbos authored and Chromium LUCI CQ committed May 13, 2021
1 parent 6ff5edd commit 3973bef
Show file tree
Hide file tree
Showing 31 changed files with 138 additions and 180 deletions.
2 changes: 1 addition & 1 deletion ash/app_list/views/expand_arrow_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ ExpandArrowView::ExpandArrowView(ContentsView* contents_view,
views::InkDrop::UseInkDropWithoutAutoHighlight(ink_drop(),
/*highlight_on_hover=*/false);
ink_drop()->SetCreateRippleCallback(base::BindRepeating(
[](InkDropHostView* host) -> std::unique_ptr<views::InkDropRipple> {
[](Button* host) -> std::unique_ptr<views::InkDropRipple> {
const AppListColorProvider* color_provider =
AppListColorProvider::Get();
return std::make_unique<views::FloodFillInkDropRipple>(
Expand Down
2 changes: 1 addition & 1 deletion ash/app_list/views/search_result_suggestion_chip_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ SearchResultSuggestionChipView::SearchResultSuggestionChipView(
views::InkDrop::UseInkDropWithoutAutoHighlight(ink_drop(),
/*highlight_on_hover=*/false);
ink_drop()->SetCreateRippleCallback(base::BindRepeating(
[](InkDropHostView* host) -> std::unique_ptr<views::InkDropRipple> {
[](Button* host) -> std::unique_ptr<views::InkDropRipple> {
const gfx::Point center = host->GetLocalBounds().CenterPoint();
const int ripple_radius = host->width() / 2;
const gfx::Rect bounds(center.x() - ripple_radius,
Expand Down
4 changes: 2 additions & 2 deletions ash/assistant/ui/base/assistant_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ AssistantButton::AssistantButton(AssistantButtonListener* listener,
views::InstallCircleHighlightPathGenerator(this, gfx::Insets(kInkDropInset));
views::InkDrop::UseInkDropForFloodFillRipple(ink_drop());
ink_drop()->SetCreateHighlightCallback(base::BindRepeating(
[](InkDropHostView* host) {
[](Button* host) {
auto highlight = std::make_unique<views::InkDropHighlight>(
gfx::SizeF(host->size()), host->ink_drop()->GetBaseColor());
highlight->set_visible_opacity(kInkDropHighlightOpacity);
return highlight;
},
this));
ink_drop()->SetCreateRippleCallback(base::BindRepeating(
[](InkDropHostView* host) -> std::unique_ptr<views::InkDropRipple> {
[](Button* host) -> std::unique_ptr<views::InkDropRipple> {
return std::make_unique<views::FloodFillInkDropRipple>(
host->size(), gfx::Insets(kInkDropInset),
host->ink_drop()->GetInkDropCenterBasedOnLastEvent(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,22 +94,15 @@ void AssistantOnboardingSuggestionView::ChildPreferredSizeChanged(
}

void AssistantOnboardingSuggestionView::AddLayerBeneathView(ui::Layer* layer) {
// This method is called by InkDropHostView, a base class of Button, to add
// ink drop layers beneath |this| view. Unfortunately, this will cause ink
// drop layers to also paint below our background and, because our background
// is opaque, they will not be visible to the user. To work around this, we
// instead add ink drop layers beneath |ink_drop_container_| which *will*
// paint above our background.
// This routes background layers to `ink_drop_container_` instead of `this` to
// avoid painting effects underneath our background.
ink_drop_container_->AddLayerBeneathView(layer);
}

void AssistantOnboardingSuggestionView::RemoveLayerBeneathView(
ui::Layer* layer) {
// This method is called by InkDropHostView, a base class of Button, to remove
// ink drop layers beneath |this| view. Because we instead added ink drop
// layers beneath |ink_drop_container_| to work around paint ordering issues,
// we inversely need to remove ink drop layers from |ink_drop_container_|
// here. See also comments in AddLayerBeneathView().
// This routes background layers to `ink_drop_container_` instead of `this` to
// avoid painting effects underneath our background.
ink_drop_container_->RemoveLayerBeneathView(layer);
}

Expand Down Expand Up @@ -146,12 +139,8 @@ void AssistantOnboardingSuggestionView::InitLayout(
views::InstallRoundRectHighlightPathGenerator(this, gfx::Insets(),
kCornerRadiusDip);

// By default, InkDropHostView, a base class of Button, will add ink drop
// layers beneath |this| view. Unfortunately, this will cause ink drop layers
// to paint below our background and, because our background is opaque, they
// will not be visible to the user. To work around this, we will instead be
// adding/removing ink drop layers as necessary to/from |ink_drop_container_|
// which *will* paint above our background.
// This is used as a parent for ink-drop effects to prevent painting them
// below the background for `this`.
ink_drop_container_ =
AddChildView(std::make_unique<views::InkDropContainerView>());

Expand Down Expand Up @@ -226,4 +215,4 @@ void AssistantOnboardingSuggestionView::OnButtonPressed() {
BEGIN_METADATA(AssistantOnboardingSuggestionView, views::Button)
END_METADATA

} // namespace ash
} // namespace ash
26 changes: 12 additions & 14 deletions ash/lock_screen_action/lock_screen_action_background_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,50 +23,48 @@

namespace ash {

class LockScreenActionBackgroundView::NoteBackground
: public views::InkDropHostView {
class LockScreenActionBackgroundView::NoteBackground : public views::View {
public:
explicit NoteBackground(views::InkDropObserver* observer)
: observer_(observer) {
DCHECK(observer);
ink_drop()->SetMode(views::InkDropHost::InkDropMode::ON_NO_GESTURE_HANDLER);

ink_drop()->SetCreateInkDropCallback(base::BindRepeating(
ink_drop_.SetMode(views::InkDropHost::InkDropMode::ON_NO_GESTURE_HANDLER);
ink_drop_.SetCreateInkDropCallback(base::BindRepeating(
[](NoteBackground* host) {
std::unique_ptr<views::InkDrop> ink_drop =
views::InkDrop::CreateInkDropWithoutAutoHighlight(
host->ink_drop(), /*highlight_on_hover=*/false);
&host->ink_drop_, /*highlight_on_hover=*/false);
ink_drop->AddObserver(host->observer_);
return ink_drop;
},
this));
ink_drop()->SetCreateRippleCallback(base::BindRepeating(
[](InkDropHostView* host) -> std::unique_ptr<views::InkDropRipple> {
ink_drop_.SetCreateRippleCallback(base::BindRepeating(
[](NoteBackground* host) -> std::unique_ptr<views::InkDropRipple> {
const gfx::Point center = base::i18n::IsRTL()
? host->GetLocalBounds().origin()
: host->GetLocalBounds().top_right();
auto ink_drop_ripple =
std::make_unique<views::FloodFillInkDropRipple>(
host->size(), gfx::Insets(), center,
host->ink_drop()->GetBaseColor(), 1);
host->ink_drop_.GetBaseColor(), 1);
ink_drop_ripple->set_use_hide_transform_duration_for_hide_fade_out(
true);
ink_drop_ripple->set_duration_factor(1.5);
return ink_drop_ripple;
},
this));

// TODO(pbos): See if this is a no-op when replaced with
// ink_drop()->SetBaseColor(), i.e. that nothing sets it later.
ink_drop()->SetBaseColorCallback(
base::BindRepeating([]() { return SK_ColorBLACK; }));
ink_drop_.SetBaseColor(SK_ColorBLACK);
}

~NoteBackground() override = default;

views::InkDropHost* ink_drop() { return &ink_drop_; }

private:
views::InkDropObserver* observer_;

views::InkDropHost ink_drop_{this};

DISALLOW_COPY_AND_ASSIGN(NoteBackground);
};

Expand Down
2 changes: 1 addition & 1 deletion ash/login/ui/lock_screen_media_controls_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class MediaActionButton : public views::ImageButton {
ink_drop()->SetMode(views::InkDropHost::InkDropMode::ON);
SetHasInkDropActionOnClick(true);
ink_drop()->SetCreateHighlightCallback(base::BindRepeating(
[](InkDropHostView* host) {
[](Button* host) {
return std::make_unique<views::InkDropHighlight>(
gfx::SizeF(host->size()), host->ink_drop()->GetBaseColor());
},
Expand Down
2 changes: 1 addition & 1 deletion ash/login/ui/login_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ LoginButton::LoginButton(PressedCallback callback)
ink_drop()->SetMode(views::InkDropHost::InkDropMode::ON);
SetHasInkDropActionOnClick(true);
ink_drop()->SetCreateHighlightCallback(base::BindRepeating(
[](InkDropHostView* host) {
[](Button* host) {
return std::make_unique<views::InkDropHighlight>(
gfx::SizeF(host->size()), kInkDropHighlightColor);
},
Expand Down
17 changes: 9 additions & 8 deletions ash/login/ui/login_pin_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ int GetViewIdForPinNumber(int number) {
}

// A base class for pin button in the pin keyboard.
class BasePinButton : public views::InkDropHostView {
class BasePinButton : public views::View {
public:
BasePinButton(const LoginPalette& palette,
const gfx::Size& size,
Expand Down Expand Up @@ -143,16 +143,15 @@ class BasePinButton : public views::InkDropHostView {
palette_ = palette;
}

// views::InkDropHostView:
void OnPaint(gfx::Canvas* canvas) override {
InkDropHostView::OnPaint(canvas);
}
views::InkDropHost* ink_drop() { return &ink_drop_; }

// views::View:
void OnFocus() override {
InkDropHostView::OnFocus();
View::OnFocus();
SchedulePaint();
}
void OnBlur() override {
InkDropHostView::OnBlur();
View::OnBlur();
SchedulePaint();
}
void OnEvent(ui::Event* event) override {
Expand All @@ -167,7 +166,7 @@ class BasePinButton : public views::InkDropHostView {
return;
}

views::InkDropHostView::OnEvent(event);
views::View::OnEvent(event);
}
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
node_data->SetName(accessible_name_);
Expand Down Expand Up @@ -195,6 +194,8 @@ class BasePinButton : public views::InkDropHostView {
LoginPalette palette_;

private:
views::InkDropHost ink_drop_{this};

const std::u16string accessible_name_;

DISALLOW_COPY_AND_ASSIGN(BasePinButton);
Expand Down
2 changes: 1 addition & 1 deletion ash/search_box/search_box_view_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class SearchBoxImageButton : public views::ImageButton {
// InkDropState will reset after clicking.
SetHasInkDropActionOnClick(true);
ink_drop()->SetCreateHighlightCallback(base::BindRepeating(
[](InkDropHostView* host) {
[](Button* host) {
constexpr SkColor ripple_color =
SkColorSetA(gfx::kGoogleGrey900, 0x12);
auto highlight = std::make_unique<views::InkDropHighlight>(
Expand Down
3 changes: 1 addition & 2 deletions ash/shelf/scrollable_shelf_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,7 @@ TEST_P(ScrollableShelfViewRTLTest, VerifyActivateIconRippleOnVerySmallDisplay) {
UpdateDisplay("60x601");

// Activate a shelf icon's ink drop. Verify that no crash happens.
views::InkDropHostView* icon = test_api_->GetButton(0);
auto* ink_drop = icon->ink_drop()->GetInkDrop();
auto* ink_drop = test_api_->GetButton(0)->ink_drop()->GetInkDrop();
ink_drop->SnapToActivated();
EXPECT_EQ(views::InkDropState::ACTIVATED, ink_drop->GetTargetInkDropState());
}
Expand Down
4 changes: 2 additions & 2 deletions ash/system/message_center/stacked_notification_bar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class StackingBarLabelButton : public views::LabelButton {
// ConfigureTrayPopupButton configures the InkDrop and these callbacks
// override that behavior.
ink_drop()->SetCreateHighlightCallback(base::BindRepeating(
[](InkDropHostView* host) {
[](Button* host) {
auto highlight = std::make_unique<views::InkDropHighlight>(
gfx::SizeF(host->size()), message_center_style::kInkRippleColor);
highlight->set_visible_opacity(
Expand All @@ -64,7 +64,7 @@ class StackingBarLabelButton : public views::LabelButton {
},
this));
ink_drop()->SetCreateRippleCallback(base::BindRepeating(
[](InkDropHostView* host) -> std::unique_ptr<views::InkDropRipple> {
[](Button* host) -> std::unique_ptr<views::InkDropRipple> {
return std::make_unique<views::FloodFillInkDropRipple>(
host->size(),
host->ink_drop()->GetInkDropCenterBasedOnLastEvent(),
Expand Down
2 changes: 1 addition & 1 deletion ash/system/toast/toast_overlay.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class ToastOverlayButton : public views::LabelButton {
ink_drop()->SetMode(views::InkDropHost::InkDropMode::ON);
SetHasInkDropActionOnClick(true);
ink_drop()->SetCreateHighlightCallback(base::BindRepeating(
[](InkDropHostView* host) {
[](Button* host) {
return std::make_unique<views::InkDropHighlight>(
gfx::SizeF(host->GetLocalBounds().size()),
host->ink_drop()->GetBaseColor());
Expand Down
3 changes: 1 addition & 2 deletions ash/system/tray/actionable_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ ActionableView::ActionableView(TrayPopupInkDropStyle ink_drop_style)
SetFocusPainter(TrayPopupUtils::CreateFocusPainter());
TrayPopupUtils::InstallHighlightPathGenerator(this, ink_drop_style_);
ink_drop()->SetCreateInkDropCallback(base::BindRepeating(
[](InkDropHostView* host) { return TrayPopupUtils::CreateInkDrop(host); },
this));
[](Button* host) { return TrayPopupUtils::CreateInkDrop(host); }, this));
ink_drop()->SetCreateHighlightCallback(base::BindRepeating(
&TrayPopupUtils::CreateInkDropHighlight, base::Unretained(this)));
ink_drop()->SetCreateRippleCallback(base::BindRepeating(
Expand Down
4 changes: 2 additions & 2 deletions ash/system/tray/tray_popup_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ views::Separator* TrayPopupUtils::CreateVerticalSeparator() {
}

std::unique_ptr<views::InkDrop> TrayPopupUtils::CreateInkDrop(
views::InkDropHostView* host,
views::Button* host,
bool highlight_on_hover,
bool highlight_on_focus) {
return views::InkDrop::CreateInkDropForFloodFillRipple(
Expand All @@ -296,7 +296,7 @@ std::unique_ptr<views::InkDrop> TrayPopupUtils::CreateInkDrop(

std::unique_ptr<views::InkDropRipple> TrayPopupUtils::CreateInkDropRipple(
TrayPopupInkDropStyle ink_drop_style,
const views::InkDropHostView* host) {
const views::Button* host) {
const AshColorProvider::RippleAttributes ripple_attributes =
AshColorProvider::Get()->GetRippleAttributes();
return std::make_unique<views::FloodFillInkDropRipple>(
Expand Down
5 changes: 2 additions & 3 deletions ash/system/tray/tray_popup_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class ImageView;
class InkDrop;
class InkDropRipple;
class InkDropHighlight;
class InkDropHostView;
class Label;
class LabelButton;
class Painter;
Expand Down Expand Up @@ -162,7 +161,7 @@ class TrayPopupUtils {
// All targetable views in the system menu should delegate
// InkDropHost::CreateInkDrop() calls here.
static std::unique_ptr<views::InkDrop> CreateInkDrop(
views::InkDropHostView* host,
views::Button* host,
bool highlight_on_hover = false,
bool highlight_on_focus = false);

Expand All @@ -173,7 +172,7 @@ class TrayPopupUtils {
// InkDropHost::CreateInkDropRipple() calls here.
static std::unique_ptr<views::InkDropRipple> CreateInkDropRipple(
TrayPopupInkDropStyle ink_drop_style,
const views::InkDropHostView* host);
const views::Button* host);

// Creates in InkDropHighlight instance for |host|.
//
Expand Down
2 changes: 1 addition & 1 deletion ash/system/unified/page_indicator_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class PageIndicatorView::PageIndicatorButton : public views::Button {
ink_drop()->SetMode(views::InkDropHost::InkDropMode::ON);
views::InstallFixedSizeCircleHighlightPathGenerator(this, kInkDropRadius);
ink_drop()->SetCreateInkDropCallback(base::BindRepeating(
[](InkDropHostView* host) {
[](Button* host) {
return TrayPopupUtils::CreateInkDrop(host,
/*highlight_on_hover=*/true);
},
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/toolbar/toolbar_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ ToolbarButton::ToolbarButton(PressedCallback callback,
installable_ink_drop_ = std::make_unique<views::InstallableInkDrop>(this);
installable_ink_drop_->SetConfig(GetToolbarInstallableInkDropConfig(this));
ink_drop()->SetCreateInkDropCallback(base::BindRepeating(
[](InkDropHostView* host) -> std::unique_ptr<views::InkDrop> {
[](Button* host) -> std::unique_ptr<views::InkDrop> {
// Ensure this doesn't get called when InstallableInkDrops are
// enabled.
DCHECK(
Expand Down
2 changes: 1 addition & 1 deletion ui/message_center/views/message_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ void MessageView::AddedToWidget() {
}

void MessageView::OnThemeChanged() {
InkDropHostView::OnThemeChanged();
View::OnThemeChanged();
UpdateBackgroundPainter();
}

Expand Down
9 changes: 2 additions & 7 deletions ui/message_center/views/message_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "ui/message_center/message_center_export.h"
#include "ui/message_center/public/cpp/notification.h"
#include "ui/message_center/public/cpp/notification_delegate.h"
#include "ui/views/animation/ink_drop_host_view.h"
#include "ui/views/animation/slide_out_controller.h"
#include "ui/views/animation/slide_out_controller_delegate.h"
#include "ui/views/controls/focus_ring.h"
Expand All @@ -42,12 +41,8 @@ class NotificationControlButtonsView;

// An base class for a notification entry. Contains background and other
// elements shared by derived notification views.
// TODO(pkasting): This class only subclasses InkDropHostView because the
// NotificationViewMD subclass needs ink drop functionality. Rework ink drops
// to not need to be the base class of views which use them, and move the
// functionality to the subclass that uses these.
class MESSAGE_CENTER_EXPORT MessageView
: public views::InkDropHostView,
: public views::View,
public views::SlideOutControllerDelegate,
public views::FocusChangeListener {
public:
Expand Down Expand Up @@ -117,7 +112,7 @@ class MESSAGE_CENTER_EXPORT MessageView
virtual void OnSettingsButtonPressed(const ui::Event& event);
virtual void OnSnoozeButtonPressed(const ui::Event& event);

// views::InkDropHostView:
// views::View:
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
bool OnMousePressed(const ui::MouseEvent& event) override;
bool OnMouseDragged(const ui::MouseEvent& event) override;
Expand Down
Loading

0 comments on commit 3973bef

Please sign in to comment.