Commit f84561d6 authored by Andrew Xu's avatar Andrew Xu Committed by Chromium LUCI CQ

[Multipaste] Highlight delete button when under pseudo focus

This CL shows the delete button's inkdrop highlight when the button
is under pseudo focus and protects the feature by a browser test case.

Bug: 1151493
Change-Id: I3b42cb001e63002b658b84c662338e773efe5c09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2583208Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarDavid Black <dmblack@google.com>
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835829}
parent 84f1ddcc
...@@ -60,6 +60,7 @@ void ClipboardHistoryDeleteButton::AddLayerBeneathView(ui::Layer* layer) { ...@@ -60,6 +60,7 @@ void ClipboardHistoryDeleteButton::AddLayerBeneathView(ui::Layer* layer) {
std::unique_ptr<views::InkDrop> ClipboardHistoryDeleteButton::CreateInkDrop() { std::unique_ptr<views::InkDrop> ClipboardHistoryDeleteButton::CreateInkDrop() {
std::unique_ptr<views::InkDrop> ink_drop = views::Button::CreateInkDrop(); std::unique_ptr<views::InkDrop> ink_drop = views::Button::CreateInkDrop();
ink_drop->SetShowHighlightOnHover(false); ink_drop->SetShowHighlightOnHover(false);
ink_drop->SetShowHighlightOnFocus(true);
return ink_drop; return ink_drop;
} }
...@@ -77,6 +78,7 @@ void ClipboardHistoryDeleteButton::OnThemeChanged() { ...@@ -77,6 +78,7 @@ void ClipboardHistoryDeleteButton::OnThemeChanged() {
AshColorProvider::Get()->GetRippleAttributes(); AshColorProvider::Get()->GetRippleAttributes();
SetInkDropBaseColor(ripple_attributes.base_color); SetInkDropBaseColor(ripple_attributes.base_color);
SetInkDropVisibleOpacity(ripple_attributes.inkdrop_opacity); SetInkDropVisibleOpacity(ripple_attributes.inkdrop_opacity);
SetInkDropHighlightOpacity(ripple_attributes.highlight_opacity);
} }
void ClipboardHistoryDeleteButton::RemoveLayerBeneathView(ui::Layer* layer) { void ClipboardHistoryDeleteButton::RemoveLayerBeneathView(ui::Layer* layer) {
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "ui/accessibility/ax_node_data.h" #include "ui/accessibility/ax_node_data.h"
#include "ui/base/clipboard/clipboard_data.h" #include "ui/base/clipboard/clipboard_data.h"
#include "ui/views/accessibility/view_accessibility.h" #include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/animation/ink_drop.h"
#include "ui/views/controls/menu/menu_config.h" #include "ui/views/controls/menu/menu_config.h"
#include "ui/views/controls/menu/menu_item_view.h" #include "ui/views/controls/menu/menu_item_view.h"
#include "ui/views/layout/fill_layout.h" #include "ui/views/layout/fill_layout.h"
...@@ -39,6 +40,18 @@ void ClipboardHistoryItemView::ContentsView::InstallDeleteButton() { ...@@ -39,6 +40,18 @@ void ClipboardHistoryItemView::ContentsView::InstallDeleteButton() {
delete_button_ = CreateDeleteButton(); delete_button_ = CreateDeleteButton();
} }
void ClipboardHistoryItemView::ContentsView::OnHostPseudoFocusUpdated() {
delete_button_->SetVisible(container_->ShouldShowDeleteButton());
const bool focused =
(container_->pseudo_focus_ == PseudoFocus::kDeleteButton);
delete_button_->GetInkDrop()->SetFocused(focused);
if (focused) {
delete_button_->NotifyAccessibilityEvent(ax::mojom::Event::kHover,
/*send_native_event*/ true);
}
}
const char* ClipboardHistoryItemView::ContentsView::GetClassName() const { const char* ClipboardHistoryItemView::ContentsView::GetClassName() const {
return "ContenstView"; return "ContenstView";
} }
...@@ -216,6 +229,10 @@ void ClipboardHistoryItemView::OnSelectionChanged() { ...@@ -216,6 +229,10 @@ void ClipboardHistoryItemView::OnSelectionChanged() {
InitiatePseudoFocus(/*reverse=*/false); InitiatePseudoFocus(/*reverse=*/false);
} }
bool ClipboardHistoryItemView::ShouldHighlight() const {
return pseudo_focus_ == PseudoFocus::kMainButton;
}
void ClipboardHistoryItemView::RecordButtonPressedHistogram() const { void ClipboardHistoryItemView::RecordButtonPressedHistogram() const {
switch (action_) { switch (action_) {
case Action::kDelete: case Action::kDelete:
...@@ -278,10 +295,6 @@ Action ClipboardHistoryItemView::CalculateActionForMainButtonClick() const { ...@@ -278,10 +295,6 @@ Action ClipboardHistoryItemView::CalculateActionForMainButtonClick() const {
} }
} }
bool ClipboardHistoryItemView::ShouldHighlight() const {
return pseudo_focus_ == PseudoFocus::kMainButton;
}
bool ClipboardHistoryItemView::ShouldShowDeleteButton() const { bool ClipboardHistoryItemView::ShouldShowDeleteButton() const {
return (pseudo_focus_ == PseudoFocus::kMainButton && IsMouseHovered()) || return (pseudo_focus_ == PseudoFocus::kMainButton && IsMouseHovered()) ||
pseudo_focus_ == PseudoFocus::kDeleteButton || pseudo_focus_ == PseudoFocus::kDeleteButton ||
...@@ -299,27 +312,18 @@ void ClipboardHistoryItemView::InitiatePseudoFocus(bool reverse) { ...@@ -299,27 +312,18 @@ void ClipboardHistoryItemView::InitiatePseudoFocus(bool reverse) {
} }
void ClipboardHistoryItemView::SetPseudoFocus(PseudoFocus new_pseudo_focus) { void ClipboardHistoryItemView::SetPseudoFocus(PseudoFocus new_pseudo_focus) {
DCHECK_NE(PseudoFocus::kMaxValue, new_pseudo_focus);
if (pseudo_focus_ == new_pseudo_focus) if (pseudo_focus_ == new_pseudo_focus)
return; return;
pseudo_focus_ = new_pseudo_focus; pseudo_focus_ = new_pseudo_focus;
contents_view_->delete_button()->SetVisible(ShouldShowDeleteButton()); if (pseudo_focus_ == PseudoFocus::kMainButton) {
main_button_->SetShouldHighlight(ShouldHighlight());
switch (pseudo_focus_) {
case PseudoFocus::kEmpty:
break;
case PseudoFocus::kMainButton:
NotifyAccessibilityEvent(ax::mojom::Event::kSelection, NotifyAccessibilityEvent(ax::mojom::Event::kSelection,
/*send_native_event=*/true); /*send_native_event=*/true);
break;
case PseudoFocus::kDeleteButton:
contents_view_->delete_button()->NotifyAccessibilityEvent(
ax::mojom::Event::kHover, /*send_native_event*/ true);
break;
case PseudoFocus::kMaxValue:
NOTREACHED();
break;
} }
contents_view_->OnHostPseudoFocusUpdated();
main_button_->OnHostPseudoFocusUpdated();
} }
} // namespace ash } // namespace ash
...@@ -50,6 +50,9 @@ class ClipboardHistoryItemView : public views::View { ...@@ -50,6 +50,9 @@ class ClipboardHistoryItemView : public views::View {
// Called when the selection state has changed. // Called when the selection state has changed.
void OnSelectionChanged(); void OnSelectionChanged();
// Returns whether the highlight background should show.
bool ShouldHighlight() const;
ClipboardHistoryUtil::Action action() const { return action_; } ClipboardHistoryUtil::Action action() const { return action_; }
ClipboardHistoryDeleteButton* delete_button_for_test() { ClipboardHistoryDeleteButton* delete_button_for_test() {
...@@ -72,6 +75,8 @@ class ClipboardHistoryItemView : public views::View { ...@@ -72,6 +75,8 @@ class ClipboardHistoryItemView : public views::View {
// Install DeleteButton on the contents view. // Install DeleteButton on the contents view.
void InstallDeleteButton(); void InstallDeleteButton();
void OnHostPseudoFocusUpdated();
ClipboardHistoryDeleteButton* delete_button() { return delete_button_; } ClipboardHistoryDeleteButton* delete_button() { return delete_button_; }
const ClipboardHistoryDeleteButton* delete_button() const { const ClipboardHistoryDeleteButton* delete_button() const {
return delete_button_; return delete_button_;
...@@ -143,9 +148,6 @@ class ClipboardHistoryItemView : public views::View { ...@@ -143,9 +148,6 @@ class ClipboardHistoryItemView : public views::View {
// Calculates the action type when `main_button_` is clicked. // Calculates the action type when `main_button_` is clicked.
ClipboardHistoryUtil::Action CalculateActionForMainButtonClick() const; ClipboardHistoryUtil::Action CalculateActionForMainButtonClick() const;
// Returns whether the highlight background should show.
bool ShouldHighlight() const;
bool ShouldShowDeleteButton() const; bool ShouldShowDeleteButton() const;
// Called when receiving pseudo focus for the first time. // Called when receiving pseudo focus for the first time.
......
...@@ -37,6 +37,10 @@ ClipboardHistoryMainButton::ClipboardHistoryMainButton( ...@@ -37,6 +37,10 @@ ClipboardHistoryMainButton::ClipboardHistoryMainButton(
ClipboardHistoryMainButton::~ClipboardHistoryMainButton() = default; ClipboardHistoryMainButton::~ClipboardHistoryMainButton() = default;
void ClipboardHistoryMainButton::OnHostPseudoFocusUpdated() {
SetShouldHighlight(container_->ShouldHighlight());
}
void ClipboardHistoryMainButton::SetShouldHighlight(bool should_highlight) { void ClipboardHistoryMainButton::SetShouldHighlight(bool should_highlight) {
if (should_highlight_ == should_highlight) if (should_highlight_ == should_highlight)
return; return;
......
...@@ -19,9 +19,11 @@ class ClipboardHistoryMainButton : public views::Button { ...@@ -19,9 +19,11 @@ class ClipboardHistoryMainButton : public views::Button {
delete; delete;
~ClipboardHistoryMainButton() override; ~ClipboardHistoryMainButton() override;
void SetShouldHighlight(bool should_highlight); void OnHostPseudoFocusUpdated();
private: private:
void SetShouldHighlight(bool should_highlight);
// views::Button: // views::Button:
const char* GetClassName() const override; const char* GetClassName() const override;
std::unique_ptr<views::InkDrop> CreateInkDrop() override; std::unique_ptr<views::InkDrop> CreateInkDrop() override;
......
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#include "ui/base/data_transfer_policy/data_transfer_endpoint.h" #include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
#include "ui/base/data_transfer_policy/data_transfer_policy_controller.h" #include "ui/base/data_transfer_policy/data_transfer_policy_controller.h"
#include "ui/events/test/event_generator.h" #include "ui/events/test/event_generator.h"
#include "ui/views/animation/ink_drop.h"
#include "ui/views/controls/menu/menu_config.h" #include "ui/views/controls/menu/menu_config.h"
#include "ui/views/controls/menu/menu_item_view.h" #include "ui/views/controls/menu/menu_item_view.h"
#include "ui/views/controls/textfield/textfield.h" #include "ui/views/controls/textfield/textfield.h"
...@@ -330,10 +331,18 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest, ...@@ -330,10 +331,18 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
GetHistoryItemViewForIndex(/*index=*/0); GetHistoryItemViewForIndex(/*index=*/0);
ASSERT_FALSE(first_history_item_view->delete_button_for_test()->GetVisible()); ASSERT_FALSE(first_history_item_view->delete_button_for_test()->GetVisible());
// Press the tab key. Verify that the first menu item's delete button shows. // Press the tab key.
PressAndRelease(ui::VKEY_TAB); PressAndRelease(ui::VKEY_TAB);
EXPECT_TRUE(first_menu_item_view->IsSelected()); EXPECT_TRUE(first_menu_item_view->IsSelected());
ASSERT_TRUE(first_history_item_view->delete_button_for_test()->GetVisible());
// Verify that the first menu item's delete button shows. In addition, the
// delete button's inkdrop highlight should fade in or be visible.
const ash::ClipboardHistoryDeleteButton* delete_button =
first_history_item_view->delete_button_for_test();
ASSERT_TRUE(delete_button->GetVisible());
EXPECT_TRUE(const_cast<ash::ClipboardHistoryDeleteButton*>(delete_button)
->GetInkDrop()
->IsHighlightFadingInOrVisible());
const views::MenuItemView* second_menu_item_view = const views::MenuItemView* second_menu_item_view =
GetMenuItemViewForIndex(/*index=*/1); GetMenuItemViewForIndex(/*index=*/1);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment