partial_split: Fix tablet menu layout error cases
This disables the 1/3 option and enables the 2/3 option on the tablet menu partial button based on the window minimum size. UX deck: https://www.figma.com/file/mqbkvuR7RNSPqUKyHWeqyg/Window-layout-menu-error-cases?node-id=0%3A1&t=46Q46fDKkFg5ou0c-0 TabletModeMultitaskMenuEventHandlerTest.WindowMinimumSizes Test: ash_unittests Bug: b/259414511 Change-Id: Ifac3beaee7ac2a99da4118d4c36cf2ec000ef5ec Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4160253 Reviewed-by: Sammie Quon <sammiequon@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Commit-Queue: Sophie Wen <sophiewen@chromium.org> Cr-Commit-Position: refs/heads/main@{#1094604}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
23f5d25fcb
commit
1b016deb6b
ash
frame
caption_buttons
wm
chromeos/ui/frame/multitask_menu
@ -804,7 +804,7 @@ TEST_F(MultitaskMenuTest, TestMultitaskMenuPartialSplit) {
|
|||||||
ShowMultitaskMenu();
|
ShowMultitaskMenu();
|
||||||
generator->MoveMouseTo(multitask_menu()
|
generator->MoveMouseTo(multitask_menu()
|
||||||
->multitask_menu_view_for_testing()
|
->multitask_menu_view_for_testing()
|
||||||
->partial_button_for_testing()
|
->partial_button()
|
||||||
->GetBoundsInScreen()
|
->GetBoundsInScreen()
|
||||||
.left_center());
|
.left_center());
|
||||||
generator->ClickLeftButton();
|
generator->ClickLeftButton();
|
||||||
@ -822,7 +822,7 @@ TEST_F(MultitaskMenuTest, TestMultitaskMenuPartialSplit) {
|
|||||||
ShowMultitaskMenu();
|
ShowMultitaskMenu();
|
||||||
gfx::Rect partial_bounds(multitask_menu()
|
gfx::Rect partial_bounds(multitask_menu()
|
||||||
->multitask_menu_view_for_testing()
|
->multitask_menu_view_for_testing()
|
||||||
->partial_button_for_testing()
|
->partial_button()
|
||||||
->GetBoundsInScreen());
|
->GetBoundsInScreen());
|
||||||
gfx::Point secondary_center(
|
gfx::Point secondary_center(
|
||||||
gfx::Point(partial_bounds.x() + partial_bounds.width() * 0.67f,
|
gfx::Point(partial_bounds.x() + partial_bounds.width() * 0.67f,
|
||||||
|
@ -12,8 +12,11 @@
|
|||||||
#include "ash/wm/splitview/split_view_controller.h"
|
#include "ash/wm/splitview/split_view_controller.h"
|
||||||
#include "ash/wm/tablet_mode/tablet_mode_multitask_menu_event_handler.h"
|
#include "ash/wm/tablet_mode/tablet_mode_multitask_menu_event_handler.h"
|
||||||
#include "ash/wm/window_state.h"
|
#include "ash/wm/window_state.h"
|
||||||
|
#include "chromeos/ui/base/display_util.h"
|
||||||
#include "chromeos/ui/frame/multitask_menu/multitask_menu_view.h"
|
#include "chromeos/ui/frame/multitask_menu/multitask_menu_view.h"
|
||||||
|
#include "chromeos/ui/frame/multitask_menu/split_button_view.h"
|
||||||
#include "chromeos/ui/wm/window_util.h"
|
#include "chromeos/ui/wm/window_util.h"
|
||||||
|
#include "ui/aura/window_delegate.h"
|
||||||
#include "ui/base/metadata/metadata_impl_macros.h"
|
#include "ui/base/metadata/metadata_impl_macros.h"
|
||||||
#include "ui/compositor/layer.h"
|
#include "ui/compositor/layer.h"
|
||||||
#include "ui/compositor/layer_type.h"
|
#include "ui/compositor/layer_type.h"
|
||||||
@ -71,17 +74,44 @@ class TabletModeMultitaskMenuView : public views::View {
|
|||||||
uint8_t buttons = chromeos::MultitaskMenuView::kFullscreen;
|
uint8_t buttons = chromeos::MultitaskMenuView::kFullscreen;
|
||||||
if (SplitViewController::Get(window)->CanSnapWindow(window)) {
|
if (SplitViewController::Get(window)->CanSnapWindow(window)) {
|
||||||
buttons |= chromeos::MultitaskMenuView::kHalfSplit;
|
buttons |= chromeos::MultitaskMenuView::kHalfSplit;
|
||||||
|
}
|
||||||
|
|
||||||
|
// TODO(sophiewen): Refactor from SplitViewController.
|
||||||
|
DCHECK(window->delegate());
|
||||||
|
const bool horizontal = chromeos::IsDisplayLayoutHorizontal(
|
||||||
|
display::Screen::GetScreen()->GetDisplayNearestWindow(window));
|
||||||
|
const gfx::Size min_size = window->delegate()->GetMinimumSize();
|
||||||
|
const int min_length = horizontal ? min_size.width() : min_size.height();
|
||||||
|
const gfx::Rect work_area = display::Screen::GetScreen()
|
||||||
|
->GetDisplayNearestWindow(window)
|
||||||
|
.work_area();
|
||||||
|
const int work_area_length =
|
||||||
|
horizontal ? work_area.width() : work_area.height();
|
||||||
|
if (min_length <= work_area_length * chromeos::kTwoThirdSnapRatio) {
|
||||||
|
// If `min_length` <= 2/3, it can fit into 2/3 split, so ensure that
|
||||||
|
// we show the full partial button.
|
||||||
buttons |= chromeos::MultitaskMenuView::kPartialSplit;
|
buttons |= chromeos::MultitaskMenuView::kPartialSplit;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (chromeos::wm::CanFloatWindow(window))
|
if (chromeos::wm::CanFloatWindow(window))
|
||||||
buttons |= chromeos::MultitaskMenuView::kFloat;
|
buttons |= chromeos::MultitaskMenuView::kFloat;
|
||||||
|
|
||||||
menu_view_for_testing_ =
|
menu_view_base_ =
|
||||||
AddChildView(std::make_unique<chromeos::MultitaskMenuView>(
|
AddChildView(std::make_unique<chromeos::MultitaskMenuView>(
|
||||||
window, std::move(callback), buttons));
|
window, std::move(callback), buttons));
|
||||||
|
|
||||||
auto* layout = menu_view_for_testing_->SetLayoutManager(
|
if (menu_view_base_->partial_button() &&
|
||||||
std::make_unique<views::BoxLayout>(
|
min_length > work_area_length * chromeos::kOneThirdSnapRatio) {
|
||||||
|
// If `min_length` > 1/3, it can't fit into 1/3 split, so disable
|
||||||
|
// the 1/3 option in the partial button. Note that `GetRightBottomButton`
|
||||||
|
// must be disabled after `partial_button` is initialized to update the
|
||||||
|
// color in `SplitButton::OnPaintBackground()`.
|
||||||
|
menu_view_base_->partial_button()->GetRightBottomButton()->SetEnabled(
|
||||||
|
false);
|
||||||
|
}
|
||||||
|
|
||||||
|
auto* layout =
|
||||||
|
menu_view_base_->SetLayoutManager(std::make_unique<views::BoxLayout>(
|
||||||
views::BoxLayout::Orientation::kHorizontal, kInsideBorderInsets,
|
views::BoxLayout::Orientation::kHorizontal, kInsideBorderInsets,
|
||||||
kBetweenButtonSpacing));
|
kBetweenButtonSpacing));
|
||||||
layout->set_main_axis_alignment(
|
layout->set_main_axis_alignment(
|
||||||
@ -103,14 +133,12 @@ class TabletModeMultitaskMenuView : public views::View {
|
|||||||
delete;
|
delete;
|
||||||
~TabletModeMultitaskMenuView() override = default;
|
~TabletModeMultitaskMenuView() override = default;
|
||||||
|
|
||||||
|
chromeos::MultitaskMenuView* menu_view_base() { return menu_view_base_; }
|
||||||
|
|
||||||
SystemShadow* shadow() { return shadow_.get(); }
|
SystemShadow* shadow() { return shadow_.get(); }
|
||||||
|
|
||||||
chromeos::MultitaskMenuView* menu_view_for_testing() {
|
|
||||||
return menu_view_for_testing_;
|
|
||||||
}
|
|
||||||
|
|
||||||
private:
|
private:
|
||||||
raw_ptr<chromeos::MultitaskMenuView> menu_view_for_testing_ = nullptr;
|
raw_ptr<chromeos::MultitaskMenuView> menu_view_base_ = nullptr;
|
||||||
|
|
||||||
std::unique_ptr<SystemShadow> shadow_;
|
std::unique_ptr<SystemShadow> shadow_;
|
||||||
};
|
};
|
||||||
@ -303,7 +331,7 @@ void TabletModeMultitaskMenu::OnDisplayMetricsChanged(
|
|||||||
chromeos::MultitaskMenuView*
|
chromeos::MultitaskMenuView*
|
||||||
TabletModeMultitaskMenu::GetMultitaskMenuViewForTesting() {
|
TabletModeMultitaskMenu::GetMultitaskMenuViewForTesting() {
|
||||||
return static_cast<TabletModeMultitaskMenuView*>(menu_view_)
|
return static_cast<TabletModeMultitaskMenuView*>(menu_view_)
|
||||||
->menu_view_for_testing(); // IN-TEST
|
->menu_view_base(); // IN-TEST
|
||||||
}
|
}
|
||||||
|
|
||||||
} // namespace ash
|
} // namespace ash
|
||||||
|
@ -134,7 +134,7 @@ class TabletModeMultitaskMenuEventHandlerTest : public AshTestBase {
|
|||||||
void PressPartialPrimary(const aura::Window& window) {
|
void PressPartialPrimary(const aura::Window& window) {
|
||||||
ShowMultitaskMenu(window);
|
ShowMultitaskMenu(window);
|
||||||
GetEventGenerator()->GestureTapAt(GetMultitaskMenuView(GetMultitaskMenu())
|
GetEventGenerator()->GestureTapAt(GetMultitaskMenuView(GetMultitaskMenu())
|
||||||
->partial_button_for_testing()
|
->partial_button()
|
||||||
->GetBoundsInScreen()
|
->GetBoundsInScreen()
|
||||||
.left_center());
|
.left_center());
|
||||||
}
|
}
|
||||||
@ -142,7 +142,7 @@ class TabletModeMultitaskMenuEventHandlerTest : public AshTestBase {
|
|||||||
void PressPartialSecondary(const aura::Window& window) {
|
void PressPartialSecondary(const aura::Window& window) {
|
||||||
ShowMultitaskMenu(window);
|
ShowMultitaskMenu(window);
|
||||||
gfx::Rect partial_bounds(GetMultitaskMenuView(GetMultitaskMenu())
|
gfx::Rect partial_bounds(GetMultitaskMenuView(GetMultitaskMenu())
|
||||||
->partial_button_for_testing()
|
->partial_button()
|
||||||
->GetBoundsInScreen());
|
->GetBoundsInScreen());
|
||||||
gfx::Point secondary_center(
|
gfx::Point secondary_center(
|
||||||
gfx::Point(partial_bounds.x() + partial_bounds.width() * 0.67f,
|
gfx::Point(partial_bounds.x() + partial_bounds.width() * 0.67f,
|
||||||
@ -248,7 +248,7 @@ TEST_F(TabletModeMultitaskMenuEventHandlerTest, BasicShowMenu) {
|
|||||||
GetMultitaskMenuView(multitask_menu);
|
GetMultitaskMenuView(multitask_menu);
|
||||||
ASSERT_TRUE(multitask_menu_view);
|
ASSERT_TRUE(multitask_menu_view);
|
||||||
EXPECT_TRUE(multitask_menu_view->half_button_for_testing());
|
EXPECT_TRUE(multitask_menu_view->half_button_for_testing());
|
||||||
EXPECT_TRUE(multitask_menu_view->partial_button_for_testing());
|
EXPECT_TRUE(multitask_menu_view->partial_button());
|
||||||
EXPECT_TRUE(multitask_menu_view->full_button_for_testing());
|
EXPECT_TRUE(multitask_menu_view->full_button_for_testing());
|
||||||
EXPECT_TRUE(multitask_menu_view->float_button_for_testing());
|
EXPECT_TRUE(multitask_menu_view->float_button_for_testing());
|
||||||
|
|
||||||
@ -561,6 +561,53 @@ TEST_F(TabletModeMultitaskMenuEventHandlerTest, AdjustedMenuBounds) {
|
|||||||
GetMultitaskMenu()->widget()->GetWindowBoundsInScreen().x());
|
GetMultitaskMenu()->widget()->GetWindowBoundsInScreen().x());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Tests that the split buttons are enabled/disabled based on min sizes.
|
||||||
|
TEST_F(TabletModeMultitaskMenuEventHandlerTest, WindowMinimumSizes) {
|
||||||
|
UpdateDisplay("800x600");
|
||||||
|
aura::test::TestWindowDelegate delegate;
|
||||||
|
std::unique_ptr<aura::Window> window(CreateTestWindowInShellWithDelegate(
|
||||||
|
&delegate, /*id=*/-1, gfx::Rect(800, 600)));
|
||||||
|
|
||||||
|
const gfx::Rect work_area_bounds =
|
||||||
|
display::Screen::GetScreen()->GetPrimaryDisplay().work_area();
|
||||||
|
|
||||||
|
// Set the min width to 0.4 of the work area. Since 1/3 < minWidth <= 1/2,
|
||||||
|
// only the 1/3 option is disabled.
|
||||||
|
delegate.set_minimum_size(
|
||||||
|
gfx::Size(work_area_bounds.width() * 0.4f, work_area_bounds.height()));
|
||||||
|
ShowMultitaskMenu(*window);
|
||||||
|
chromeos::MultitaskMenuView* multitask_menu_view =
|
||||||
|
GetMultitaskMenuView(GetMultitaskMenu());
|
||||||
|
EXPECT_TRUE(multitask_menu_view->half_button_for_testing());
|
||||||
|
EXPECT_TRUE(multitask_menu_view->partial_button()->GetEnabled());
|
||||||
|
ASSERT_FALSE(multitask_menu_view->partial_button()
|
||||||
|
->GetRightBottomButton()
|
||||||
|
->GetEnabled());
|
||||||
|
GetMultitaskMenu()->Reset();
|
||||||
|
|
||||||
|
// Set the min width to 0.6 of the work area. Since 1/2 < minWidth <= 2/3, the
|
||||||
|
// half button is hidden and only the 2/3 option is enabled.
|
||||||
|
delegate.set_minimum_size(
|
||||||
|
gfx::Size(work_area_bounds.width() * 0.6f, work_area_bounds.height()));
|
||||||
|
ShowMultitaskMenu(*window);
|
||||||
|
multitask_menu_view = GetMultitaskMenuView(GetMultitaskMenu());
|
||||||
|
EXPECT_FALSE(multitask_menu_view->half_button_for_testing());
|
||||||
|
EXPECT_TRUE(multitask_menu_view->partial_button()->GetEnabled());
|
||||||
|
ASSERT_FALSE(multitask_menu_view->partial_button()
|
||||||
|
->GetRightBottomButton()
|
||||||
|
->GetEnabled());
|
||||||
|
GetMultitaskMenu()->Reset();
|
||||||
|
|
||||||
|
// Set the min width to 0.7 of the work area. Since minWidth > 2/3, both the
|
||||||
|
// split buttons are hidden.
|
||||||
|
delegate.set_minimum_size(
|
||||||
|
gfx::Size(work_area_bounds.width() * 0.7f, work_area_bounds.height()));
|
||||||
|
ShowMultitaskMenu(*window);
|
||||||
|
multitask_menu_view = GetMultitaskMenuView(GetMultitaskMenu());
|
||||||
|
EXPECT_FALSE(multitask_menu_view->half_button_for_testing());
|
||||||
|
EXPECT_FALSE(multitask_menu_view->partial_button());
|
||||||
|
}
|
||||||
|
|
||||||
// Tests that tap outside the menu will close the menu.
|
// Tests that tap outside the menu will close the menu.
|
||||||
TEST_F(TabletModeMultitaskMenuEventHandlerTest, CloseMultitaskMenuOnTap) {
|
TEST_F(TabletModeMultitaskMenuEventHandlerTest, CloseMultitaskMenuOnTap) {
|
||||||
// Create a display and window that is bigger than the menu.
|
// Create a display and window that is bigger than the menu.
|
||||||
@ -596,7 +643,7 @@ TEST_F(TabletModeMultitaskMenuEventHandlerTest, HiddenButtons) {
|
|||||||
GetMultitaskMenuView(multitask_menu);
|
GetMultitaskMenuView(multitask_menu);
|
||||||
ASSERT_TRUE(multitask_menu_view);
|
ASSERT_TRUE(multitask_menu_view);
|
||||||
EXPECT_FALSE(multitask_menu_view->half_button_for_testing());
|
EXPECT_FALSE(multitask_menu_view->half_button_for_testing());
|
||||||
EXPECT_FALSE(multitask_menu_view->partial_button_for_testing());
|
EXPECT_FALSE(multitask_menu_view->partial_button());
|
||||||
EXPECT_TRUE(multitask_menu_view->full_button_for_testing());
|
EXPECT_TRUE(multitask_menu_view->full_button_for_testing());
|
||||||
EXPECT_FALSE(multitask_menu_view->float_button_for_testing());
|
EXPECT_FALSE(multitask_menu_view->float_button_for_testing());
|
||||||
}
|
}
|
||||||
|
@ -90,7 +90,7 @@ MultitaskMenuView::MultitaskMenuView(
|
|||||||
base::BindRepeating(&MultitaskMenuView::PartialButtonPressed,
|
base::BindRepeating(&MultitaskMenuView::PartialButtonPressed,
|
||||||
base::Unretained(this)),
|
base::Unretained(this)),
|
||||||
window, is_portrait_mode);
|
window, is_portrait_mode);
|
||||||
partial_button_for_testing_ = partial_button.get();
|
partial_button_ = partial_button.get();
|
||||||
AddChildView(CreateButtonContainer(std::move(partial_button),
|
AddChildView(CreateButtonContainer(std::move(partial_button),
|
||||||
IDS_MULTITASK_MENU_PARTIAL_BUTTON_NAME));
|
IDS_MULTITASK_MENU_PARTIAL_BUTTON_NAME));
|
||||||
}
|
}
|
||||||
|
@ -47,9 +47,7 @@ class COMPONENT_EXPORT(CHROMEOS_UI_FRAME) MultitaskMenuView
|
|||||||
SplitButtonView* half_button_for_testing() {
|
SplitButtonView* half_button_for_testing() {
|
||||||
return half_button_for_testing_.get();
|
return half_button_for_testing_.get();
|
||||||
}
|
}
|
||||||
SplitButtonView* partial_button_for_testing() {
|
SplitButtonView* partial_button() { return partial_button_.get(); }
|
||||||
return partial_button_for_testing_.get();
|
|
||||||
}
|
|
||||||
MultitaskButton* full_button_for_testing() {
|
MultitaskButton* full_button_for_testing() {
|
||||||
return full_button_for_testing_.get();
|
return full_button_for_testing_.get();
|
||||||
}
|
}
|
||||||
@ -66,7 +64,7 @@ class COMPONENT_EXPORT(CHROMEOS_UI_FRAME) MultitaskMenuView
|
|||||||
|
|
||||||
// Saved for testing purpose.
|
// Saved for testing purpose.
|
||||||
raw_ptr<SplitButtonView> half_button_for_testing_ = nullptr;
|
raw_ptr<SplitButtonView> half_button_for_testing_ = nullptr;
|
||||||
raw_ptr<SplitButtonView> partial_button_for_testing_ = nullptr;
|
raw_ptr<SplitButtonView> partial_button_ = nullptr;
|
||||||
raw_ptr<MultitaskButton> full_button_for_testing_ = nullptr;
|
raw_ptr<MultitaskButton> full_button_for_testing_ = nullptr;
|
||||||
raw_ptr<MultitaskButton> float_button_for_testing_ = nullptr;
|
raw_ptr<MultitaskButton> float_button_for_testing_ = nullptr;
|
||||||
|
|
||||||
|
@ -81,7 +81,8 @@ class SplitButtonView::SplitButton : public views::Button {
|
|||||||
void OnPaintBackground(gfx::Canvas* canvas) override {
|
void OnPaintBackground(gfx::Canvas* canvas) override {
|
||||||
cc::PaintFlags pattern_flags;
|
cc::PaintFlags pattern_flags;
|
||||||
pattern_flags.setAntiAlias(true);
|
pattern_flags.setAntiAlias(true);
|
||||||
pattern_flags.setColor(button_color_);
|
pattern_flags.setColor(GetEnabled() ? button_color_
|
||||||
|
: kMultitaskButtonDisabledColor);
|
||||||
pattern_flags.setStyle(cc::PaintFlags::kFill_Style);
|
pattern_flags.setStyle(cc::PaintFlags::kFill_Style);
|
||||||
gfx::Rect pattern_bounds = GetLocalBounds();
|
gfx::Rect pattern_bounds = GetLocalBounds();
|
||||||
pattern_bounds.Inset(insets_);
|
pattern_bounds.Inset(insets_);
|
||||||
@ -160,6 +161,10 @@ SplitButtonView::SplitButtonView(SplitButtonType type,
|
|||||||
: gfx::Size(right_bottom_width, kMultitaskHalfButtonHeight));
|
: gfx::Size(right_bottom_width, kMultitaskHalfButtonHeight));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
views::Button* SplitButtonView::GetRightBottomButton() {
|
||||||
|
return static_cast<views::Button*>(right_bottom_button_);
|
||||||
|
}
|
||||||
|
|
||||||
void SplitButtonView::OnButtonHoveredOrPressed() {
|
void SplitButtonView::OnButtonHoveredOrPressed() {
|
||||||
border_color_ = kMultitaskButtonPrimaryHoverColor;
|
border_color_ = kMultitaskButtonPrimaryHoverColor;
|
||||||
fill_color_ = kMultitaskButtonViewHoverColor;
|
fill_color_ = kMultitaskButtonViewHoverColor;
|
||||||
|
@ -38,16 +38,18 @@ class SplitButtonView : public views::BoxLayoutView {
|
|||||||
|
|
||||||
~SplitButtonView() override = default;
|
~SplitButtonView() override = default;
|
||||||
|
|
||||||
|
views::Button* GetRightBottomButton();
|
||||||
|
|
||||||
private:
|
private:
|
||||||
class SplitButton;
|
class SplitButton;
|
||||||
|
|
||||||
|
// Called when either button is hovered or pressed. Updates button colors.
|
||||||
|
void OnButtonHoveredOrPressed();
|
||||||
|
|
||||||
// views::View:
|
// views::View:
|
||||||
void OnPaint(gfx::Canvas* canvas) override;
|
void OnPaint(gfx::Canvas* canvas) override;
|
||||||
void OnThemeChanged() override;
|
void OnThemeChanged() override;
|
||||||
|
|
||||||
// Called when either button is hovered or pressed. Updates button colors.
|
|
||||||
void OnButtonHoveredOrPressed();
|
|
||||||
|
|
||||||
// Pointers to the buttons that are owned by the views hierarchy. The names
|
// Pointers to the buttons that are owned by the views hierarchy. The names
|
||||||
// refer to the physical location of the button, which do not change in RTL
|
// refer to the physical location of the button, which do not change in RTL
|
||||||
// languages.
|
// languages.
|
||||||
|
Reference in New Issue
Block a user