Skip to content

Commit

Permalink
[multipaste] Remember old MenuPosition
Browse files Browse the repository at this point in the history
To prevent multipaste from jumping when deleting items,
remember the old menu position.

Bug: 1165288
Change-Id: I136d43ec9524a6ddcaff1ba371a50e90ce321d0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2633963
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844314}
  • Loading branch information
Alex Newcomer authored and Chromium LUCI CQ committed Jan 16, 2021
1 parent 87483a4 commit 319d11d
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 4 deletions.
27 changes: 23 additions & 4 deletions ui/views/controls/menu/menu_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2481,14 +2481,33 @@ gfx::Rect MenuController::CalculateBubbleMenuBounds(MenuItemView* item,
item->set_actual_menu_position(MenuPosition::kBelowBounds);
}
} else if (state_.anchor == MenuAnchorPosition::kBubbleBelow) {
y = y_for_menu_below;
item->set_actual_menu_position(MenuPosition::kBelowBounds);
if (y + menu_size.height() > monitor_bounds.bottom()) {
// Respect the previous MenuPosition. The menu contents could change
// while the menu is shown, the menu position should not change.
const bool able_to_show_menu_below =
(y_for_menu_below + menu_size.height() <= monitor_bounds.bottom());
const bool able_to_show_menu_above =
y_for_menu_above >= monitor_bounds.y();
if (item->actual_menu_position() == MenuPosition::kBelowBounds &&
able_to_show_menu_below) {
y = y_for_menu_below;
} else if (item->actual_menu_position() == MenuPosition::kAboveBounds &&
able_to_show_menu_above) {
y = y_for_menu_above;
} else if (able_to_show_menu_below) {
y = y_for_menu_below;
item->set_actual_menu_position(MenuPosition::kBelowBounds);
} else if (able_to_show_menu_above) {
// No room below, but there is room above. Show above the anchor.
// Align the bottom of the menu with the bottom of the anchor.
y = y_for_menu_above;
item->set_actual_menu_position(MenuPosition::kAboveBounds);
} else {
// No room above or below. Show as low as possible. Align the bottom
// of the menu with the bottom of the screen.
y = monitor_bounds.bottom() - menu_size.height();
item->set_actual_menu_position(MenuPosition::kBestFit);
}
}

} else if (state_.anchor == MenuAnchorPosition::kBubbleLeft ||
state_.anchor == MenuAnchorPosition::kBubbleRight) {
if (state_.anchor == MenuAnchorPosition::kBubbleLeft) {
Expand Down
64 changes: 64 additions & 0 deletions ui/views/controls/menu/menu_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,70 @@ TEST_F(MenuControllerTest, VerifyMenuBubblePositionAfterSizeChanges) {
}
}

// Verifies that the context menu bubble position, MenuPosition::kBubbleBelow,
// does not shift as items are removed. The menu position will shift it items
// are added and the menu no longer fits in its previous position.
TEST_F(MenuControllerTest, VerifyMenuBubbleBelowPositionAfterSizeChanges) {
constexpr gfx::Rect kMonitorBounds(0, 0, 500, 500);
constexpr gfx::Size kMenuSize(100, 200);
const gfx::Insets border_and_shadow_insets =
BubbleBorder::GetBorderAndShadowInsets(
MenuConfig::instance().touchable_menu_shadow_elevation);

// Calculate the suitable anchor point to ensure that if the menu shows below
// the anchor point, the bottom of the menu should be one pixel off the
// bottom of the display. It means that there is insufficient space for the
// menu below the anchor.
const gfx::Point anchor_point(kMonitorBounds.width() / 2,
kMonitorBounds.bottom() + 1 -
kMenuSize.height() +
border_and_shadow_insets.top());

MenuBoundsOptions options;
options.menu_anchor = MenuAnchorPosition::kBubbleBelow;
options.monitor_bounds = kMonitorBounds;
options.anchor_bounds = gfx::Rect(anchor_point, gfx::Size());

// Case 1: There is insufficient space for the menu below `anchor_point` and
// there is no cached menu position. The menu should show above the anchor.
{
options.menu_size = kMenuSize;
ASSERT_GT(options.anchor_bounds.y() - border_and_shadow_insets.top() +
kMenuSize.height(),
kMonitorBounds.bottom());
CalculateBubbleMenuBounds(options);
EXPECT_EQ(MenuItemView::MenuPosition::kAboveBounds,
menu_item()->ActualMenuPosition());
}

// Case 2: There is insufficient space for the menu below `anchor_point`. The
// cached position is below the anchor. The menu should show above the anchor
// point.
{
options.menu_position = MenuItemView::MenuPosition::kBelowBounds;
CalculateBubbleMenuBounds(options);
EXPECT_EQ(MenuItemView::MenuPosition::kAboveBounds,
menu_item()->ActualMenuPosition());
}

// Case 3: There is enough space for the menu below `anchor_point`. The cached
// menu position is above the anchor. The menu should show above the anchor.
{
// Shrink the menu size. Verify that there is enough space below the anchor
// point now.
constexpr gfx::Size kUpdatedSize(kMenuSize.width(), kMenuSize.height() / 2);
options.menu_size = kUpdatedSize;
EXPECT_LE(options.anchor_bounds.y() - border_and_shadow_insets.top() +
kUpdatedSize.height(),
kMonitorBounds.bottom());

options.menu_position = MenuItemView::MenuPosition::kAboveBounds;
CalculateBubbleMenuBounds(options);
EXPECT_EQ(MenuItemView::MenuPosition::kAboveBounds,
menu_item()->ActualMenuPosition());
}
}

// Tests that opening the menu and pressing 'Home' selects the first menu item.
TEST_F(MenuControllerTest, FirstSelectedItem) {
SetPendingStateItem(menu_item()->GetSubmenu()->GetMenuItemAt(0));
Expand Down

0 comments on commit 319d11d

Please sign in to comment.