Skip to content

Commit

Permalink
Changed default behavior for inkdrops on buttons.
Browse files Browse the repository at this point in the history
Changed the default value of has_ink_drop_action_on_click_ from false
to true. This has the effect of triggering the
InkDropState::ACTION_TRIGGERED transition animation on click. With
this enabled the inkdrop will fade out by default after a mouse click
event. This better represents the expected behavior for the button
class.

Given there might be buttons that currently rely on the false default
value, an audit that looked at all existing uses of the inkdrop in
buttons was made to ensure that existing code continues to function
as expected. Changes have been made to maintain old functionality
where necessary.

Bug: 1025666
Change-Id: I0074030a56ca3de4c5249598e04897dd22ddae21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1963195
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Tim Song <tengs@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#725332}
  • Loading branch information
Thomas Lukaszewicz authored and Commit Bot committed Dec 17, 2019
1 parent e172710 commit 7919903
Show file tree
Hide file tree
Showing 29 changed files with 2 additions and 34 deletions.
1 change: 0 additions & 1 deletion ash/app_list/views/assistant/privacy_info_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ void PrivacyInfoView::InitCloseButton() {
close_button_->set_ink_drop_visible_opacity(kInkDropVisibleOpacity);
close_button_->set_ink_drop_highlight_opacity(kInkDropHighlightOpacity);
close_button_->set_ink_drop_base_color(kInkDropBaseColor);
close_button_->set_has_ink_drop_action_on_click(true);
views::InstallCircleHighlightPathGenerator(close_button_);
row_container_->AddChildView(close_button_);
}
Expand Down
5 changes: 0 additions & 5 deletions ash/app_list/views/page_switcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,6 @@ class PageSwitcherButton : public views::Button {
: kInkDropRadiusForFolderGrid));
}

void NotifyClick(const ui::Event& event) override {
Button::NotifyClick(event);
GetInkDrop()->AnimateToState(views::InkDropState::ACTION_TRIGGERED);
}

private:
// Stores the information of how to paint the button.
struct PaintButtonInfo {
Expand Down
1 change: 0 additions & 1 deletion ash/assistant/ui/base/assistant_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ AssistantButton::AssistantButton(AssistantButtonListener* listener,

// Ink drop.
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_ink_drop_base_color(kInkDropBaseColor);
set_ink_drop_visible_opacity(kInkDropVisibleOpacity);
}
Expand Down
1 change: 0 additions & 1 deletion ash/login/ui/lock_screen_media_controls_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ class MediaActionButton : public views::ImageButton {
action == MediaSessionAction::kPlay),
icon_size_(icon_size) {
SetInkDropMode(views::Button::InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
SetImageHorizontalAlignment(views::ImageButton::ALIGN_CENTER);
SetImageVerticalAlignment(views::ImageButton::ALIGN_MIDDLE);
SetBorder(
Expand Down
1 change: 0 additions & 1 deletion ash/login/ui/login_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ LoginButton::LoginButton(views::ButtonListener* listener)
SetInstallFocusRingOnFocus(true);
focus_ring()->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
}

LoginButton::~LoginButton() = default;
Expand Down
2 changes: 0 additions & 2 deletions ash/shelf/login_shelf_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ class LoginShelfButton : public views::LabelButton {
focus_ring()->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
SetFocusPainter(nullptr);
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_ink_drop_base_color(ShelfConfig::Get()->shelf_ink_drop_base_color());
set_ink_drop_visible_opacity(
ShelfConfig::Get()->shelf_ink_drop_visible_opacity());
Expand Down Expand Up @@ -264,7 +263,6 @@ class KioskAppsButton : public views::MenuButton,
focus_ring()->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
SetFocusPainter(nullptr);
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_ink_drop_base_color(ShelfConfig::Get()->shelf_ink_drop_base_color());
set_ink_drop_visible_opacity(
ShelfConfig::Get()->shelf_ink_drop_visible_opacity());
Expand Down
1 change: 0 additions & 1 deletion ash/shelf/scroll_arrow_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ ScrollArrowView::ScrollArrowView(ArrowType arrow_type,
: ShelfButton(shelf, shelf_button_delegate),
arrow_type_(arrow_type),
is_horizontal_alignment_(is_horizontal_alignment) {
set_has_ink_drop_action_on_click(true);
SetInkDropMode(InkDropMode::ON_NO_GESTURE_HANDLER);
}

Expand Down
1 change: 1 addition & 0 deletions ash/shelf/shelf_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ ShelfButton::ShelfButton(Shelf* shelf,
ShelfConfig::Get()->shelf_ink_drop_visible_opacity());
SetFocusBehavior(FocusBehavior::ALWAYS);
SetInkDropMode(InkDropMode::ON_NO_GESTURE_HANDLER);
set_has_ink_drop_action_on_click(false);
SetFocusPainter(views::Painter::CreateSolidFocusPainter(
ShelfConfig::Get()->shelf_focus_border_color(), kFocusBorderThickness,
gfx::InsetsF()));
Expand Down
1 change: 0 additions & 1 deletion ash/shelf/shelf_control_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ ShelfControlButton::ShelfControlButton(
Shelf* shelf,
ShelfButtonDelegate* shelf_button_delegate)
: ShelfButton(shelf, shelf_button_delegate) {
set_has_ink_drop_action_on_click(true);
SetInstallFocusRingOnFocus(true);
views::HighlightPathGenerator::Install(
this, std::make_unique<ShelfControlButtonHighlightPathGenerator>());
Expand Down
1 change: 0 additions & 1 deletion ash/system/toast/toast_overlay.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ class ToastOverlayButton : public views::LabelButton {
const SkColor toast_backgrond_color)
: views::LabelButton(listener, text, CONTEXT_TOAST_OVERLAY) {
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_ink_drop_base_color(AshColorProvider::Get()
->GetRippleAttributes(toast_backgrond_color)
.base_color);
Expand Down
1 change: 0 additions & 1 deletion ash/system/tray/tray_popup_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ void TrayPopupUtils::ConfigureTrayPopupButton(views::Button* button) {
button->SetInstallFocusRingOnFocus(true);
button->SetFocusForPlatform();
button->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
button->set_has_ink_drop_action_on_click(true);
}

void TrayPopupUtils::ConfigureAsStickyHeader(views::View* view) {
Expand Down
1 change: 0 additions & 1 deletion ash/wm/desks/close_desk_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ CloseDeskButton::CloseDeskButton(views::ButtonListener* listener)
inkdrop_base_color_ = ripple_attributes.base_color;

SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_ink_drop_visible_opacity(ripple_attributes.inkdrop_opacity);
SetFocusPainter(nullptr);

Expand Down
1 change: 0 additions & 1 deletion ash/wm/desks/new_desk_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ NewDeskButton::NewDeskButton(views::ButtonListener* listener)
SetTextColor(views::Button::STATE_DISABLED, kDisabledTextAndIconColor);
SetImageLabelSpacing(kImageLabelSpacing);
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_ink_drop_visible_opacity(kInkDropVisibleOpacity);
SetFocusPainter(nullptr);

Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ class BookmarkButtonBase : public views::LabelButton {
DISTANCE_RELATED_LABEL_HORIZONTAL_LIST));
views::InstallPillHighlightPathGenerator(this);
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_ink_drop_visible_opacity(kToolbarInkDropVisibleOpacity);
show_animation_ = std::make_unique<gfx::SlideAnimation>(this);
if (!animations_enabled) {
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/views/tabs/tab.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ Tab::Tab(TabController* controller)
close_button_ = new TabCloseButton(
this, base::BindRepeating(&TabController::OnMouseEventInTab,
base::Unretained(controller_)));
close_button_->set_has_ink_drop_action_on_click(true);
AddChildView(close_button_);

tab_close_button_observer_ = std::make_unique<TabCloseButtonObserver>(
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/views/toolbar/toolbar_action_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ ToolbarActionView::ToolbarActionView(
view_controller_(view_controller),
delegate_(delegate) {
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_hide_ink_drop_when_showing_context_menu(false);
set_show_ink_drop_when_hot_tracked(true);
SetID(VIEW_ID_BROWSER_ACTION);
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/views/toolbar/toolbar_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ ToolbarButton::ToolbarButton(views::ButtonListener* listener,
tab_strip_model_(tab_strip_model),
trigger_menu_on_long_press_(trigger_menu_on_long_press),
highlight_color_animation_(this) {
set_has_ink_drop_action_on_click(true);
set_context_menu_controller(this);

if (base::FeatureList::IsEnabled(views::kInstallableInkDropFeature)) {
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ void InstallToolbarButtonHighlightPathGenerator(views::View* host) {
}

void ConfigureInkDropForToolbar(views::Button* host) {
host->set_has_ink_drop_action_on_click(true);
InstallToolbarButtonHighlightPathGenerator(host);
host->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
host->set_ink_drop_visible_opacity(kToolbarInkDropVisibleOpacity);
Expand Down
2 changes: 0 additions & 2 deletions ui/chromeos/search_box/search_box_view_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ class SearchBoxImageButton : public views::ImageButton {
SetPaintToLayer();
layer()->SetFillsBoundsOpaquely(false);
SetInkDropMode(InkDropMode::ON);
// InkDropState will reset after clicking.
set_has_ink_drop_action_on_click(true);

SetPreferredSize({kButtonSizeDip, kButtonSizeDip});
SetImageHorizontalAlignment(ALIGN_CENTER);
Expand Down
1 change: 0 additions & 1 deletion ui/message_center/views/notification_view_md.cc
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ NotificationButtonMD::NotificationButtonMD(
placeholder_(placeholder) {
SetHorizontalAlignment(gfx::ALIGN_CENTER);
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
set_ink_drop_base_color(SK_ColorBLACK);
set_ink_drop_visible_opacity(kActionButtonInkDropRippleVisibleOpacity);
SetEnabledTextColors(kActionButtonTextColor);
Expand Down
1 change: 0 additions & 1 deletion ui/message_center/views/padded_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ PaddedButton::PaddedButton(views::ButtonListener* listener)

SetInkDropMode(InkDropMode::ON);
set_ink_drop_base_color(SkColorSetA(SK_ColorBLACK, 0.6 * 0xff));
set_has_ink_drop_action_on_click(true);
}

std::unique_ptr<views::InkDrop> PaddedButton::CreateInkDrop() {
Expand Down
2 changes: 1 addition & 1 deletion ui/views/controls/button/button.h
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ class VIEWS_EXPORT Button : public InkDropHostView,

// True when a button click should trigger an animation action on
// ink_drop_delegate().
bool has_ink_drop_action_on_click_ = false;
bool has_ink_drop_action_on_click_ = true;

// When true, the ink drop ripple and hover will be hidden prior to showing
// the context menu.
Expand Down
1 change: 0 additions & 1 deletion ui/views/controls/button/checkbox.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ Checkbox::Checkbox(const base::string16& label, ButtonListener* listener)

set_request_focus_on_press(false);
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);

// Limit the checkbox height to match the legacy appearance.
const gfx::Size preferred_size(LabelButton::CalculatePreferredSize());
Expand Down
1 change: 0 additions & 1 deletion ui/views/controls/button/image_button_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ std::unique_ptr<ToggleImageButton> CreateVectorToggleImageButton(

void ConfigureVectorImageButton(ImageButton* button) {
button->SetInkDropMode(Button::InkDropMode::ON);
button->set_has_ink_drop_action_on_click(true);
button->SetImageHorizontalAlignment(ImageButton::ALIGN_CENTER);
button->SetImageVerticalAlignment(ImageButton::ALIGN_MIDDLE);
button->SetBorder(CreateEmptyBorder(
Expand Down
1 change: 0 additions & 1 deletion ui/views/controls/button/md_text_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ MdTextButton::MdTextButton(ButtonListener* listener, int button_context)
: LabelButton(listener, base::string16(), button_context),
is_prominent_(false) {
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
SetCornerRadius(LayoutProvider::Get()->GetCornerRadiusMetric(EMPHASIS_LOW));
SetHorizontalAlignment(gfx::ALIGN_CENTER);
SetFocusForPlatform();
Expand Down
1 change: 0 additions & 1 deletion ui/views/controls/button/toggle_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ ToggleButton::ToggleButton(ButtonListener* listener) : Button(listener) {
// regression in crbug.com/1031983, but a matching FocusRing would probably be
// desirable.
SetInstallFocusRingOnFocus(false);
set_has_ink_drop_action_on_click(true);
}

ToggleButton::~ToggleButton() {
Expand Down
1 change: 0 additions & 1 deletion ui/views/controls/combobox/combobox.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class TransparentButton : public Button {
ButtonController::NotifyAction::kOnPress);

SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
}
~TransparentButton() override = default;

Expand Down
1 change: 0 additions & 1 deletion ui/views/controls/editable_combobox/editable_combobox.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ class Arrow : public Button {
ButtonController::NotifyAction::kOnPress);

SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
}
~Arrow() override = default;

Expand Down
1 change: 0 additions & 1 deletion ui/views/window/frame_caption_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ FrameCaptionButton::FrameCaptionButton(views::ButtonListener* listener,
set_animate_on_state_change(true);
swap_images_animation_->Reset(1);

set_has_ink_drop_action_on_click(true);
SetInkDropMode(InkDropMode::ON);
set_ink_drop_visible_opacity(kInkDropVisibleOpacity);
UpdateInkDropBaseColor();
Expand Down

0 comments on commit 7919903

Please sign in to comment.