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

[Multipaste] Handle the click cancel event properly

In this CL, when the mouse click on the main button or the delete button
is canceled, the menu selection is refreshed by:
(1) Select the menu item hovered by mouse if any; otherwise,
(2) Select the root menu item to cancel the selection on a child item.

Bug: 1165999
Change-Id: I86bebc2b1dc08665a98da366a6a6bdb9fab532d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2625798Reviewed-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@{#843103}
parent 8c420c9d
......@@ -431,6 +431,9 @@ void ClipboardHistoryControllerImpl::ExecuteCommand(int command_id,
case Action::kSelect:
context_menu_->SelectMenuItemWithCommandId(command_id);
return;
case Action::kSelectItemHoveredByMouse:
context_menu_->SelectMenuItemHoveredByMouse();
return;
case Action::kEmpty:
NOTREACHED();
return;
......
......@@ -147,6 +147,25 @@ void ClipboardHistoryMenuModelAdapter::SelectMenuItemWithCommandId(
selected_menu_item);
}
void ClipboardHistoryMenuModelAdapter::SelectMenuItemHoveredByMouse() {
// Find the menu item hovered by mouse.
auto iter =
std::find_if(item_views_by_command_id_.cbegin(),
item_views_by_command_id_.cend(), [](const auto& iterator) {
const views::View* item_view = iterator.second;
return item_view->IsMouseHovered();
});
if (iter == item_views_by_command_id_.cend()) {
// If no item is hovered by mouse, cancel the selection on the child menu
// item by selecting the root menu item.
views::MenuController::GetActiveInstance()->SelectItemAndOpenSubmenu(
root_view_);
} else {
SelectMenuItemWithCommandId(iter->first);
}
}
void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId(
int command_id) {
// Calculate `new_selected_command_id` before removing the item specified by
......
......@@ -74,6 +74,9 @@ class ASH_EXPORT ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter {
// Selects the menu item specified by `command_id`.
void SelectMenuItemWithCommandId(int command_id);
// Selects the menu item hovered by mouse.
void SelectMenuItemHoveredByMouse();
// Removes the menu item specified by `command_id`.
void RemoveMenuItemWithCommandId(int command_id);
......
......@@ -44,7 +44,18 @@ enum class Action {
kDelete,
// Selects the activated item.
kSelect
kSelect,
// Selects the item hovered by mouse if any.
kSelectItemHoveredByMouse
};
// IDs for the views used by the clipboard history menu.
enum ClipboardHistoryMenuViewID {
// We start at 1 because 0 is not a valid view ID.
kDeleteButtonViewID = 1,
kMainButtonViewID
};
// Used in histograms, each value corresponds with an underlying format
......
......@@ -21,7 +21,9 @@ ClipboardHistoryDeleteButton::ClipboardHistoryDeleteButton(
[](ClipboardHistoryItemView* item_view, const ui::Event& event) {
item_view->HandleDeleteButtonPressEvent(event);
},
base::Unretained(listener))) {
base::Unretained(listener))),
listener_(listener) {
SetID(ClipboardHistoryUtil::kDeleteButtonViewID);
SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
SetAccessibleName(
l10n_util::GetStringUTF16(IDS_CLIPBOARD_HISTORY_DELETE_BUTTON));
......@@ -64,6 +66,13 @@ std::unique_ptr<views::InkDrop> ClipboardHistoryDeleteButton::CreateInkDrop() {
return ink_drop;
}
void ClipboardHistoryDeleteButton::OnClickCanceled(const ui::Event& event) {
DCHECK(event.IsMouseEvent());
listener_->OnMouseClickOnDescendantCanceled();
views::Button::OnClickCanceled(event);
}
void ClipboardHistoryDeleteButton::OnThemeChanged() {
// Use the light mode as default because the light mode is the default mode of
// the native theme which decides the context menu's background color.
......
......@@ -29,12 +29,16 @@ class ClipboardHistoryDeleteButton : public views::ImageButton {
const char* GetClassName() const override;
void AddLayerBeneathView(ui::Layer* layer) override;
std::unique_ptr<views::InkDrop> CreateInkDrop() override;
void OnClickCanceled(const ui::Event& event) override;
void OnThemeChanged() override;
void RemoveLayerBeneathView(ui::Layer* layer) override;
// Used to accommodate the ink drop layer. It ensures that the ink drop is
// above the view background.
views::InkDropContainerView* ink_drop_container_ = nullptr;
// The listener of button events.
ClipboardHistoryItemView* const listener_;
};
} // namespace ash
......
......@@ -233,7 +233,16 @@ bool ClipboardHistoryItemView::ShouldHighlight() const {
return pseudo_focus_ == PseudoFocus::kMainButton;
}
void ClipboardHistoryItemView::RecordButtonPressedHistogram() const {
void ClipboardHistoryItemView::OnMouseClickOnDescendantCanceled() {
// When mouse click is canceled, mouse may hover a different menu item from
// the one where the click event started. A typical way is to move the mouse
// while pressing the mouse left button. Hence, update the menu selection due
// to the mouse location change.
Activate(ClipboardHistoryUtil::Action::kSelectItemHoveredByMouse,
ui::EF_NONE);
}
void ClipboardHistoryItemView::MaybeRecordButtonPressedHistogram() const {
switch (action_) {
case Action::kDelete:
ClipboardHistoryUtil::RecordClipboardHistoryItemDeleted(
......@@ -244,6 +253,7 @@ void ClipboardHistoryItemView::RecordButtonPressedHistogram() const {
*clipboard_history_item_);
return;
case Action::kSelect:
case Action::kSelectItemHoveredByMouse:
return;
case Action::kEmpty:
NOTREACHED();
......@@ -266,7 +276,7 @@ void ClipboardHistoryItemView::Activate(Action action, int event_flags) {
DCHECK_NE(action_, action);
base::AutoReset<Action> action_to_take(&action_, action);
RecordButtonPressedHistogram();
MaybeRecordButtonPressedHistogram();
views::MenuDelegate* delegate = container_->GetDelegate();
const int command_id = container_->GetCommand();
......
......@@ -53,15 +53,11 @@ class ClipboardHistoryItemView : public views::View {
// Returns whether the highlight background should show.
bool ShouldHighlight() const;
ClipboardHistoryUtil::Action action() const { return action_; }
ClipboardHistoryDeleteButton* delete_button_for_test() {
return contents_view_->delete_button();
}
// Called when the mouse click on descendants (such as the main button or
// the delete button) gets canceled.
void OnMouseClickOnDescendantCanceled();
const ClipboardHistoryDeleteButton* delete_button_for_test() const {
return contents_view_->delete_button();
}
ClipboardHistoryUtil::Action action() const { return action_; }
protected:
// Used by subclasses to draw contents, such as text or bitmaps.
......@@ -105,8 +101,8 @@ class ClipboardHistoryItemView : public views::View {
ClipboardHistoryItemView(const ClipboardHistoryItem* clipboard_history_item,
views::MenuItemView* container);
// Records histograms after the button is pressed.
void RecordButtonPressedHistogram() const;
// Maybe record histograms after the button is pressed.
void MaybeRecordButtonPressedHistogram() const;
// Creates the contents view.
virtual std::unique_ptr<ContentsView> CreateContentsView() = 0;
......
......@@ -30,6 +30,7 @@ ClipboardHistoryMainButton::ClipboardHistoryMainButton(
container_(container) {
SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
SetID(ClipboardHistoryUtil::kMainButtonViewID);
// Let the parent handle accessibility features.
GetViewAccessibility().OverrideIsIgnored(/*value=*/true);
......@@ -68,6 +69,13 @@ std::unique_ptr<views::InkDrop> ClipboardHistoryMainButton::CreateInkDrop() {
return ink_drop;
}
void ClipboardHistoryMainButton::OnClickCanceled(const ui::Event& event) {
DCHECK(event.IsMouseEvent());
container_->OnMouseClickOnDescendantCanceled();
views::Button::OnClickCanceled(event);
}
void ClipboardHistoryMainButton::OnThemeChanged() {
views::Button::OnThemeChanged();
......
......@@ -27,6 +27,7 @@ class ClipboardHistoryMainButton : public views::Button {
// views::Button:
const char* GetClassName() const override;
std::unique_ptr<views::InkDrop> CreateInkDrop() override;
void OnClickCanceled(const ui::Event& event) override;
void OnThemeChanged() override;
void OnGestureEvent(ui::GestureEvent* event) override;
void PaintButtonContents(gfx::Canvas* canvas) override;
......
......@@ -290,7 +290,7 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
GetMenuItemViewForIndex(/*index=*/0);
EXPECT_TRUE(first_menu_item_view->IsSelected());
EXPECT_FALSE(GetHistoryItemViewForIndex(/*index=*/0)
->delete_button_for_test()
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
EXPECT_EQ(gfx::Size(256, 36), first_menu_item_view->size());
......@@ -307,7 +307,7 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
// Under mouse hovering, the second item's delete button should show.
EXPECT_TRUE(GetHistoryItemViewForIndex(/*index=*/1)
->delete_button_for_test()
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
const views::MenuItemView* third_menu_item_view =
......@@ -320,7 +320,7 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
EXPECT_FALSE(second_menu_item_view->IsSelected());
EXPECT_TRUE(third_menu_item_view->IsSelected());
EXPECT_FALSE(GetHistoryItemViewForIndex(/*index=*/2)
->delete_button_for_test()
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
}
......@@ -341,7 +341,9 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
ASSERT_TRUE(first_menu_item_view->IsSelected());
const ash::ClipboardHistoryItemView* first_history_item_view =
GetHistoryItemViewForIndex(/*index=*/0);
ASSERT_FALSE(first_history_item_view->delete_button_for_test()->GetVisible());
ASSERT_FALSE(first_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
// Press the tab key.
PressAndRelease(ui::VKEY_TAB);
......@@ -350,7 +352,9 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
// 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();
static_cast<const ash::ClipboardHistoryDeleteButton*>(
first_history_item_view->GetViewByID(
ash::ClipboardHistoryUtil::kDeleteButtonViewID));
ASSERT_TRUE(delete_button->GetVisible());
EXPECT_TRUE(const_cast<ash::ClipboardHistoryDeleteButton*>(delete_button)
->GetInkDrop()
......@@ -361,33 +365,40 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
EXPECT_FALSE(second_menu_item_view->IsSelected());
const ash::ClipboardHistoryItemView* second_history_item_view =
GetHistoryItemViewForIndex(/*index=*/1);
EXPECT_FALSE(
second_history_item_view->delete_button_for_test()->GetVisible());
EXPECT_FALSE(second_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
// Press the tab key. Verify that the second menu item is selected while its
// delete button is hidden.
PressAndRelease(ui::VKEY_TAB);
EXPECT_TRUE(second_menu_item_view->IsSelected());
EXPECT_FALSE(
second_history_item_view->delete_button_for_test()->GetVisible());
EXPECT_FALSE(second_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
// Press the tab key. Verify that the second item's delete button shows.
PressAndRelease(ui::VKEY_TAB);
EXPECT_TRUE(second_menu_item_view->IsSelected());
EXPECT_TRUE(second_history_item_view->delete_button_for_test()->GetVisible());
EXPECT_TRUE(second_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
// Press the tab key with the shift key pressed. Verify that the second item
// is selected while its delete button is hidden.
PressAndRelease(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
EXPECT_TRUE(second_menu_item_view->IsSelected());
EXPECT_FALSE(
second_history_item_view->delete_button_for_test()->GetVisible());
EXPECT_FALSE(second_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
// Press the tab key with the shift key pressed. Verify that the first item
// is selected while its delete button is visible.
PressAndRelease(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
EXPECT_TRUE(first_menu_item_view->IsSelected());
EXPECT_TRUE(first_history_item_view->delete_button_for_test()->GetVisible());
EXPECT_TRUE(first_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
EXPECT_FALSE(second_menu_item_view->IsSelected());
// Press the ENTER key. Verifies that the first item is deleted. The second
......@@ -395,8 +406,9 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
PressAndRelease(ui::VKEY_RETURN);
EXPECT_EQ(1, GetContextMenu()->GetMenuItemsCount());
EXPECT_TRUE(second_menu_item_view->IsSelected());
EXPECT_FALSE(
second_history_item_view->delete_button_for_test()->GetVisible());
EXPECT_FALSE(second_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
}
// Verifies the tab traversal on the history menu with only one item.
......@@ -412,19 +424,25 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
ASSERT_EQ(1, GetContextMenu()->GetMenuItemsCount());
const ash::ClipboardHistoryItemView* first_history_item_view =
GetHistoryItemViewForIndex(/*index=*/0);
ASSERT_FALSE(first_history_item_view->delete_button_for_test()->GetVisible());
ASSERT_FALSE(first_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
const views::MenuItemView* first_menu_item_view =
GetMenuItemViewForIndex(/*index=*/0);
ASSERT_TRUE(first_menu_item_view->IsSelected());
// Press the tab key. Verify that the delete button is visible.
PressAndRelease(ui::VKEY_TAB);
ASSERT_TRUE(first_history_item_view->delete_button_for_test()->GetVisible());
ASSERT_TRUE(first_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
// Press the tab key. Verify that the delete button is hidden. The menu item
// is still under selection.
PressAndRelease(ui::VKEY_TAB);
ASSERT_FALSE(first_history_item_view->delete_button_for_test()->GetVisible());
ASSERT_FALSE(first_history_item_view
->GetViewByID(ash::ClipboardHistoryUtil::kDeleteButtonViewID)
->GetVisible());
EXPECT_TRUE(first_menu_item_view->IsSelected());
}
......@@ -459,6 +477,39 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
menu_origin.y());
}
// Verify the handling of the click cancel event.
IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
HandleClickCancelEvent) {
LoginUser(account_id1_);
// Write some things to the clipboard.
SetClipboardText("A");
SetClipboardText("B");
// Show the menu.
ShowContextMenuViaAccelerator(/*wait_for_selection=*/true);
ASSERT_TRUE(GetClipboardHistoryController()->IsMenuShowing());
ASSERT_EQ(2, GetContextMenu()->GetMenuItemsCount());
// Press on the first menu item.
ash::ClipboardHistoryItemView* first_item_view =
GetHistoryItemViewForIndex(/*index=*/0);
GetEventGenerator()->MoveMouseTo(
first_item_view->GetBoundsInScreen().CenterPoint());
GetEventGenerator()->PressLeftButton();
// Move the mouse to the second menu item then release.
auto* second_item_view =
GetContextMenu()->GetMenuItemViewAtForTest(/*index=*/1);
ASSERT_FALSE(second_item_view->IsSelected());
GetEventGenerator()->MoveMouseTo(
second_item_view->GetBoundsInScreen().CenterPoint());
GetEventGenerator()->ReleaseLeftButton();
// Verify that the second menu item is selected now.
EXPECT_TRUE(second_item_view->IsSelected());
}
// Verifies that the selected item should be deleted by the backspace key.
IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
DeleteItemViaBackspaceKey) {
......@@ -672,8 +723,8 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryTextfieldBrowserTest,
ash::ClipboardHistoryItemView* second_item_view =
GetHistoryItemViewForIndex(/*index=*/1);
views::View* second_item_delete_button =
second_item_view->delete_button_for_test();
views::View* second_item_delete_button = second_item_view->GetViewByID(
ash::ClipboardHistoryUtil::kDeleteButtonViewID);
EXPECT_FALSE(second_item_delete_button->GetVisible());
// Long press on the second item until its delete button shows.
......
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