0

bento: Fix multi display behavior of name nudges.

Currently if a user has multiple displays and clicks/taps the new desk
button, there is a DeskNameView that is created on each display for the
new desk. Additionally, all of these DeskNameViews request focus,
leading to unexpected behavior. What should happen is that the newly
created DeskNameView on the same DesksBarView as the button that was
clicked/tapped should be focused.

This CL fixes this bug by adding a field to the DesksBarView, which is
set when a new desk button is clicked. Only the DesksBarView with this
new field will have its newly created name view focused.

Test: added
Bug: 1206013
Change-Id: I63b7cb3594a5c6a63ec015e4b0d00ac1dcd740a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2877734
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#881308}
This commit is contained in:
chinsenj
2021-05-10 23:34:42 +00:00
committed by Chromium LUCI CQ
parent 91c136dd7d
commit 65c9dca85e
6 changed files with 111 additions and 10 deletions

@ -290,7 +290,7 @@ DesksBarView::DesksBarView(OverviewGrid* overview_grid)
zero_state_default_desk_button_ = scroll_view_contents_->AddChildView(
std::make_unique<ZeroStateDefaultDeskButton>(this));
zero_state_new_desk_button_ = scroll_view_contents_->AddChildView(
std::make_unique<ZeroStateNewDeskButton>());
std::make_unique<ZeroStateNewDeskButton>(this));
scroll_view_contents_->SetLayoutManager(
std::make_unique<DesksBarScrollViewLayout>(this));
@ -752,9 +752,10 @@ void DesksBarView::UpdateNewMiniViews(bool initializing_bar_view,
}
}
if (!initializing_bar_view) {
// Focus on the newly created name view to encourge users to rename their
// desks.
// If we're not initializing the desk bar and |should_name_nudge_| is true,
// focus on the newly created name view to encourage users to rename their
// desks.
if (!initializing_bar_view && should_name_nudge_) {
auto* newly_added_name_view = mini_views_.back()->desk_name_view();
newly_added_name_view->RequestFocus();
@ -766,6 +767,8 @@ void DesksBarView::UpdateNewMiniViews(bool initializing_bar_view,
auto* highlight_controller = GetHighlightController();
if (highlight_controller->IsFocusHighlightVisible())
highlight_controller->MoveHighlightToView(newly_added_name_view);
should_name_nudge_ = false;
}
Layout();

@ -71,6 +71,10 @@ class ASH_EXPORT DesksBarView : public views::View,
bool dragged_item_over_bar() const { return dragged_item_over_bar_; }
void set_should_name_nudge(bool should_name_nudge) {
should_name_nudge_ = should_name_nudge;
}
// Initializes and creates mini_views for any pre-existing desks, before the
// bar was created. This should only be called after this view has been added
// to a widget, as it needs to call `GetWidget()` when it's performing a
@ -256,6 +260,11 @@ class ASH_EXPORT DesksBarView : public views::View,
// |expanded_state_new_desk_button_| currently.
views::View* scroll_view_contents_ = nullptr;
// If this is true, when `UpdateNewMiniViews()` is called, the newly created
// mini view's name view will be focused and |should_name_nudge_| will be
// reset.
bool should_name_nudge_ = false;
ZeroStateDefaultDeskButton* zero_state_default_desk_button_ = nullptr;
ZeroStateNewDeskButton* zero_state_new_desk_button_ = nullptr;
ExpandedStateNewDeskButton* expanded_state_new_desk_button_ = nullptr;

@ -4135,6 +4135,85 @@ TEST_F(DesksTest, NameNudges) {
}
}
// Tests that name nudges works with multiple displays. When a user
// clicks/touches the new desk button, the newly created DeskNameView that
// resides on the same DesksBarView as the clicked button should be focused.
// See crbug.com/1206013.
TEST_F(DesksTest, NameNudgesMultiDisplay) {
UpdateDisplay("800x800,800x800");
// Start overview.
auto* overview_controller = Shell::Get()->overview_controller();
overview_controller->StartOverview();
EXPECT_TRUE(overview_controller->InOverviewSession());
// Retrieve the desks bar view for each root window.
auto root_windows = Shell::GetAllRootWindows();
ASSERT_EQ(2u, root_windows.size());
const auto* desks_bar_view_1 =
GetOverviewGridForRoot(root_windows[0])->desks_bar_view();
const auto* desks_bar_view_2 =
GetOverviewGridForRoot(root_windows[1])->desks_bar_view();
ASSERT_TRUE(desks_bar_view_1->IsZeroState());
ASSERT_TRUE(desks_bar_view_2->IsZeroState());
// Click on the zero state default desk button for the second root window.
auto* zero_state_default_desk_button_2 =
desks_bar_view_2->zero_state_default_desk_button();
EXPECT_TRUE(zero_state_default_desk_button_2->GetEnabled());
auto* event_generator = GetEventGenerator();
ClickOnView(zero_state_default_desk_button_2, event_generator);
// The desk bar should not be in the zero state anymore and the existing
// desk's name view should be focused, but not cleared.
EXPECT_FALSE(desks_bar_view_2->IsZeroState());
EXPECT_EQ(1u, desks_bar_view_2->mini_views().size());
auto* desk_name_view_2 = desks_bar_view_2->mini_views()[0]->desk_name_view();
EXPECT_TRUE(desk_name_view_2->HasFocus());
EXPECT_EQ(DesksController::GetDeskDefaultName(/*desk_index=*/0),
desk_name_view_2->GetText());
// Restart overview to reset the zero state.
overview_controller->EndOverview();
overview_controller->StartOverview();
desks_bar_view_1 = GetOverviewGridForRoot(root_windows[0])->desks_bar_view();
desks_bar_view_2 = GetOverviewGridForRoot(root_windows[1])->desks_bar_view();
ASSERT_TRUE(desks_bar_view_1->IsZeroState());
ASSERT_TRUE(desks_bar_view_2->IsZeroState());
// Click on the new desk button on the first root window.
auto* new_desk_button_1 = desks_bar_view_1->zero_state_new_desk_button();
EXPECT_TRUE(new_desk_button_1->GetEnabled());
ClickOnView(new_desk_button_1, event_generator);
// There should be 2 desks now and the name view on the first root window
// should be focused. Each new name view on the 2 root windows should be
// empty.
EXPECT_EQ(2u, desks_bar_view_1->mini_views().size());
auto* desk_name_view_1 = desks_bar_view_1->mini_views()[1]->desk_name_view();
desk_name_view_2 = desks_bar_view_2->mini_views()[1]->desk_name_view();
EXPECT_TRUE(desk_name_view_1->HasFocus());
EXPECT_FALSE(desk_name_view_2->HasFocus());
EXPECT_EQ(std::u16string(), desk_name_view_1->GetText());
EXPECT_EQ(std::u16string(), desk_name_view_2->GetText());
// Tap on the new desk button on the second root window.
auto* new_desk_button_2 = desks_bar_view_2->expanded_state_new_desk_button();
EXPECT_TRUE(new_desk_button_2->GetEnabled());
GestureTapOnView(new_desk_button_2, event_generator);
// There should be 3 desks now and the name view on the second root window
// should be focused. Each new name view on the 2 root windows should be
// empty.
EXPECT_EQ(3u, desks_bar_view_1->mini_views().size());
desk_name_view_1 = desks_bar_view_1->mini_views()[2]->desk_name_view();
desk_name_view_2 = desks_bar_view_2->mini_views()[2]->desk_name_view();
EXPECT_FALSE(desk_name_view_1->HasFocus());
EXPECT_TRUE(desk_name_view_2->HasFocus());
EXPECT_EQ(std::u16string(), desk_name_view_1->GetText());
EXPECT_EQ(std::u16string(), desk_name_view_2->GetText());
}
TEST_F(DesksTest, ScrollableDesks) {
UpdateDisplay("201x400");
auto* overview_controller = Shell::Get()->overview_controller();

@ -32,9 +32,11 @@ constexpr int kCornerRadius = 4;
// The button belongs to ExpandedStateNewDeskButton.
class ASH_EXPORT InnerNewDeskButton : public DeskButtonBase {
public:
InnerNewDeskButton(ExpandedStateNewDeskButton* outer_button)
InnerNewDeskButton(ExpandedStateNewDeskButton* outer_button,
DesksBarView* bar_view)
: DeskButtonBase(std::u16string(), kBorderCornerRadius, kCornerRadius),
outer_button_(outer_button) {
outer_button_(outer_button),
bar_view_(bar_view) {
paint_contents_only_ = true;
}
InnerNewDeskButton(const InnerNewDeskButton&) = delete;
@ -53,6 +55,7 @@ class ASH_EXPORT InnerNewDeskButton : public DeskButtonBase {
void OnButtonPressed() override {
auto* controller = DesksController::Get();
if (controller->CanCreateDesks()) {
bar_view_->set_should_name_nudge(true);
controller->NewDesk(DesksCreationRemovalSource::kButton);
UpdateButtonState();
}
@ -86,6 +89,7 @@ class ASH_EXPORT InnerNewDeskButton : public DeskButtonBase {
private:
ExpandedStateNewDeskButton* outer_button_;
DesksBarView* bar_view_;
};
} // namespace
@ -93,7 +97,7 @@ class ASH_EXPORT InnerNewDeskButton : public DeskButtonBase {
ExpandedStateNewDeskButton::ExpandedStateNewDeskButton(DesksBarView* bar_view)
: bar_view_(bar_view),
new_desk_button_(
AddChildView(std::make_unique<InnerNewDeskButton>(this))),
AddChildView(std::make_unique<InnerNewDeskButton>(this, bar_view))),
label_(AddChildView(std::make_unique<views::Label>())) {
SetPaintToLayer();
layer()->SetFillsBoundsOpaquely(false);

@ -186,6 +186,7 @@ gfx::Size ZeroStateDefaultDeskButton::CalculatePreferredSize() const {
}
void ZeroStateDefaultDeskButton::OnButtonPressed() {
bar_view_->set_should_name_nudge(true);
bar_view_->UpdateNewMiniViews(/*initializing_bar_view=*/false,
/*expanding_bar_view=*/true);
}
@ -200,8 +201,9 @@ void ZeroStateDefaultDeskButton::UpdateLabelText() {
// -----------------------------------------------------------------------------
// ZeroStateNewDeskButton:
ZeroStateNewDeskButton::ZeroStateNewDeskButton()
: DeskButtonBase(std::u16string(), kCornerRadius, kCornerRadius) {
ZeroStateNewDeskButton::ZeroStateNewDeskButton(DesksBarView* bar_view)
: DeskButtonBase(std::u16string(), kCornerRadius, kCornerRadius),
bar_view_(bar_view) {
highlight_on_hover_ = false;
}
@ -219,6 +221,7 @@ gfx::Size ZeroStateNewDeskButton::CalculatePreferredSize() const {
}
void ZeroStateNewDeskButton::OnButtonPressed() {
bar_view_->set_should_name_nudge(true);
DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
highlight_on_hover_ = false;
}

@ -104,7 +104,7 @@ class ASH_EXPORT ZeroStateDefaultDeskButton : public DeskButtonBase {
// ExpandedStateNewDeskButton.
class ASH_EXPORT ZeroStateNewDeskButton : public DeskButtonBase {
public:
ZeroStateNewDeskButton();
ZeroStateNewDeskButton(DesksBarView* bar_view);
ZeroStateNewDeskButton(const ZeroStateNewDeskButton&) = delete;
ZeroStateNewDeskButton& operator=(const ZeroStateNewDeskButton&) = delete;
~ZeroStateNewDeskButton() override = default;
@ -118,6 +118,9 @@ class ASH_EXPORT ZeroStateNewDeskButton : public DeskButtonBase {
// views::Button:
void OnMouseEntered(const ui::MouseEvent& event) override;
void OnMouseExited(const ui::MouseEvent& event) override;
private:
DesksBarView* bar_view_;
};
} // namespace ash