Skip to content

Commit

Permalink
CloseAll: Open context menu with long-press
Browse files Browse the repository at this point in the history
This CL gives the DesksBarView the ability to open a context menu on a
mini view when it is being long-pressed and desks are not rearranged.
This allows for the user to open a context menu with a long press while
still being able to start a drag with a long press as well.

Bug: 1308780
Change-Id: I4d33f0e6c5281b8d11d604a5b26a0ac5cc9f7e85
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3612344
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Ben Becker <benbecker@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002501}
  • Loading branch information
Ben Becker authored and Chromium LUCI CQ committed May 12, 2022
1 parent 3b52fd8 commit aed60e7
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 10 deletions.
8 changes: 7 additions & 1 deletion ash/wm/desks/desk_action_context_menu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ void DeskActionContextMenu::SetCombineDesksMenuItemVisibility(bool visible) {
context_menu_model_.SetVisibleAt(CommandId::kCombineDesks, visible);
}

void DeskActionContextMenu::MaybeCloseMenu() {
if (context_menu_runner_)
context_menu_runner_->Cancel();
}

void DeskActionContextMenu::ExecuteCommand(int command_id, int event_flags) {
switch (command_id) {
case CommandId::kCombineDesks:
Expand All @@ -74,7 +79,8 @@ void DeskActionContextMenu::ShowContextMenuForViewImpl(
ui::MenuSourceType source_type) {
const int run_types = views::MenuRunner::USE_ASH_SYS_UI_LAYOUT |
views::MenuRunner::CONTEXT_MENU |
views::MenuRunner::FIXED_ANCHOR;
views::MenuRunner::FIXED_ANCHOR |
views::MenuRunner::SEND_GESTURE_EVENTS_TO_OWNER;

context_menu_runner_ =
std::make_unique<views::MenuRunner>(&context_menu_model_, run_types);
Expand Down
3 changes: 3 additions & 0 deletions ash/wm/desks/desk_action_context_menu.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class DeskActionContextMenu : public views::ContextMenuController,
// can reflect whether there are windows on the desk.
void SetCombineDesksMenuItemVisibility(bool visible);

// Closes the context menu if one is running.
void MaybeCloseMenu();

// ui::SimpleMenuModel::Delegate:
void ExecuteCommand(int command_id, int event_flags) override;
void MenuClosed(ui::SimpleMenuModel* menu) override;
Expand Down
14 changes: 10 additions & 4 deletions ash/wm/desks/desk_mini_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ bool DeskMiniView::IsPointOnMiniView(const gfx::Point& screen_location) const {
return GetWidget() && HitTestPoint(point_in_view);
}

void DeskMiniView::OpenContextMenu() {
void DeskMiniView::OpenContextMenu(ui::MenuSourceType source) {
is_context_menu_open_ = true;
UpdateDeskButtonVisibility();

Expand All @@ -255,13 +255,19 @@ void DeskMiniView::OpenContextMenu() {
// visible on all desks.
context_menu_->SetCombineDesksMenuItemVisibility(ContainsAppWindows(desk_));

// TODO(crbug.com/1308780): Source will need to be different when opening with
// long press and possibly keyboard.
// Only show the combine desks context menu option if there are app windows in
// the desk.
context_menu_->SetCombineDesksMenuItemVisibility(desk_->ContainsAppWindows());
context_menu_->ShowContextMenuForView(
this,
base::i18n::IsRTL() ? desk_preview_->GetBoundsInScreen().bottom_right()
: desk_preview_->GetBoundsInScreen().bottom_left(),
ui::MENU_SOURCE_MOUSE);
source);
}

void DeskMiniView::MaybeCloseContextMenu() {
if (context_menu_)
context_menu_->MaybeCloseMenu();
}

const char* DeskMiniView::GetClassName() const {
Expand Down
15 changes: 13 additions & 2 deletions ash/wm/desks/desk_mini_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,19 @@ class ASH_EXPORT DeskMiniView : public views::View,
bool IsPointOnMiniView(const gfx::Point& screen_location) const;

// Hides the `desk_action_view_` and opens `context_menu_`. Called when
// `desk_preview_` is right-clicked or long-pressed.
void OpenContextMenu();
// `desk_preview_` is right-clicked or long-pressed. `source` is the type of
// action that caused the context menu to be opened (e.g. long press versus
// mouse click), and is provided to the context menu runner when the menu is
// open in `DeskActionContextMenu::ShowContextMenuForViewImpl` so that it can
// further evaluate menu positioning. This ends up doing nothing in particular
// in the case of the `DeskActionContextMenu` because we use a
// `views::MenuRunner::FIXED_ANCHOR` run type parameter, but the
// `MenuRunner::RunMenuAt` function still requires this parameter, so we pass
// it down to the function through this parameter.
void OpenContextMenu(ui::MenuSourceType source);

// Closes context menu on this mini view if one exists.
void MaybeCloseContextMenu();

// views::View:
const char* GetClassName() const override;
Expand Down
4 changes: 1 addition & 3 deletions ash/wm/desks/desk_preview_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ bool DeskPreviewView::OnMousePressed(const ui::MouseEvent& event) {
// should open the context menu.
if (features::IsDesksCloseAllEnabled() && event.IsRightMouseButton()) {
DeskNameView::CommitChanges(GetWidget());
mini_view_->OpenContextMenu();
mini_view_->OpenContextMenu(ui::MENU_SOURCE_MOUSE);
} else {
mini_view_->owner_bar()->HandlePressEvent(mini_view_, event);
}
Expand All @@ -441,8 +441,6 @@ void DeskPreviewView::OnGestureEvent(ui::GestureEvent* event) {
switch (event->type()) {
// Only long press can trigger drag & drop.
case ui::ET_GESTURE_LONG_PRESS:
// TODO(crbug.com/1308780): Need to figure out how we can still maintain
// drag functionality while allowing long press to open the context menu.
owner_bar->HandleLongPressEvent(mini_view_, *event);
event->SetHandled();
break;
Expand Down
6 changes: 6 additions & 0 deletions ash/wm/desks/desks_bar_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,9 @@ void DesksBarView::HandleLongPressEvent(DeskMiniView* mini_view,
gfx::PointF location = event.target()->GetScreenLocationF(event);
InitDragDesk(mini_view, location);
StartDragDesk(mini_view, location, event.IsMouseEvent());

if (features::IsDesksCloseAllEnabled())
mini_view->OpenContextMenu(ui::MENU_SOURCE_LONG_PRESS);
}

void DesksBarView::HandleDragEvent(DeskMiniView* mini_view,
Expand All @@ -556,6 +559,9 @@ void DesksBarView::HandleDragEvent(DeskMiniView* mini_view,
if (!drag_proxy_ || mini_view->is_animating_to_remove())
return;

if (features::IsDesksCloseAllEnabled())
mini_view->MaybeCloseContextMenu();

gfx::PointF location = event.target()->GetScreenLocationF(event);

// If the drag proxy is initialized, start the drag. If the drag started,
Expand Down
7 changes: 7 additions & 0 deletions ash/wm/desks/desks_test_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "ash/wm/desks/persistent_desks_bar_view.h"
#include "ash/wm/overview/overview_controller.h"
#include "ash/wm/overview/overview_grid.h"
#include "ui/views/controls/menu/menu_runner.h"

namespace ash {

Expand Down Expand Up @@ -106,6 +107,7 @@ const ui::SimpleMenuModel& DesksTestApi::GetContextMenuModelForDesk(int index) {
return GetContextMenuForDesk(index)->context_menu_model_;
}

// static
views::View* DesksTestApi::GetHighlightOverlayForDeskPreview(int index) {
return GetDesksBarView()
->mini_views()[index]
Expand All @@ -128,6 +130,11 @@ bool DesksTestApi::DesksControllerCanUndoDeskRemoval() {
return DesksController::Get()->temporary_removed_desk_ != nullptr;
}

// static
bool DesksTestApi::IsContextMenuRunningForDesk(int index) {
return GetContextMenuForDesk(index)->context_menu_runner_->IsRunning();
}

// static
bool DesksTestApi::IsDesksBarLeftGradientVisible() {
return !GetDesksBarView()
Expand Down
1 change: 1 addition & 0 deletions ash/wm/desks/desks_test_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class DesksTestApi {
static bool HasVerticalDotsButton();
static bool DesksControllerHasDesk(Desk* desk);
static bool DesksControllerCanUndoDeskRemoval();
static bool IsContextMenuRunningForDesk(int index);

static bool IsDesksBarLeftGradientVisible();
static bool IsDesksBarRightGradientVisible();
Expand Down
18 changes: 18 additions & 0 deletions ash/wm/desks/desks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7776,6 +7776,24 @@ TEST_F(DesksCloseAllTest, TestMetricsRecordingWhenCloseAllWindows) {
}
}

// Checks that a `DeskActionContextMenu` opens when the user long-presses a
// desk's mini view.
TEST_F(DesksCloseAllTest, ContextMenuOpensOnLongPress) {
NewDesk();
EnterOverview();
ASSERT_TRUE(Shell::Get()->overview_controller()->InOverviewSession());

// Long press on the first desk preview view.
const DeskPreviewView* desk_preview_view =
GetPrimaryRootDesksBarView()->mini_views()[0]->desk_preview();
const gfx::Point desk_preview_view_center =
desk_preview_view->GetBoundsInScreen().CenterPoint();
auto* event_generator = GetEventGenerator();
LongGestureTap(desk_preview_view_center, event_generator);

EXPECT_TRUE(DesksTestApi::IsContextMenuRunningForDesk(0));
}

// TODO(afakhry): Add more tests:
// - Always on top windows are not tracked by any desk.
// - Reusing containers when desks are removed and created.
Expand Down

0 comments on commit aed60e7

Please sign in to comment.