Close compat menu if entry button is clicked again
Currently if compat mode button is clicked when the compat menu is
shown, the menu is closed and then gets shown again. It's an unnatural
UX since the user would expect a toggle effect for the button.
This CL fixes the issue by adding a check before (re-)showing the menu,
which is based on a weak pointer of the menu bubble.
Also need to note that in this implementation, the close action (when
the button is clicked when bubble is shown) relies on the delayed
destruction of the bubble object which is triggered by activation change
of the menu bubble.
R=toshikikikuchi
--gtest_filter="*ResizeToggleMenuTest.TestIsBubbleShown"
Test: Manual testing
Test: ash_components_unittests \
Bug: b/192514067
Change-Id: I41dbabf985aee9c205810a771cf10a22663e1d93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3539609
Reviewed-by: Toshiki Kikuchi <toshikikikuchi@chromium.org>
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Qijing Yao <yaoqq@google.com>
Cr-Commit-Position: refs/heads/main@{#985166}
diff --git a/ash/components/arc/compat_mode/compat_mode_button_controller.cc b/ash/components/arc/compat_mode/compat_mode_button_controller.cc
index 5a456cf..418e0e8a 100644
--- a/ash/components/arc/compat_mode/compat_mode_button_controller.cc
+++ b/ash/components/arc/compat_mode/compat_mode_button_controller.cc
@@ -168,6 +168,8 @@
frame_view->GetHeaderView()->GetFrameHeader()->GetCenterButton();
if (!compat_mode_button || !compat_mode_button->GetEnabled())
return;
+ if (resize_toggle_menu_ && resize_toggle_menu_->IsBubbleShown())
+ return;
resize_toggle_menu_.reset();
resize_toggle_menu_ =
std::make_unique<ResizeToggleMenu>(frame_view->frame(), pref_delegate);
diff --git a/ash/components/arc/compat_mode/resize_toggle_menu.cc b/ash/components/arc/compat_mode/resize_toggle_menu.cc
index 1d7fa04..ad0153c 100644
--- a/ash/components/arc/compat_mode/resize_toggle_menu.cc
+++ b/ash/components/arc/compat_mode/resize_toggle_menu.cc
@@ -49,8 +49,14 @@
frame->SetCornerRadius(corner_radius_);
}
+ base::WeakPtr<RoundedCornerBubbleDialogDelegateView> GetWeakPtr() {
+ return weak_factory_.GetWeakPtr();
+ }
+
private:
const int corner_radius_;
+ base::WeakPtrFactory<RoundedCornerBubbleDialogDelegateView> weak_factory_{
+ this};
};
} // namespace
@@ -230,6 +236,7 @@
auto delegate_view =
std::make_unique<RoundedCornerBubbleDialogDelegateView>(kCornerRadius);
+ bubble_view_ = delegate_view->GetWeakPtr();
// Setup delegate.
delegate_view->SetArrow(views::BubbleBorder::Arrow::TOP_CENTER);
@@ -301,6 +308,10 @@
FROM_HERE, auto_close_closure_.callback(), kAutoCloseDelay);
}
+bool ResizeToggleMenu::IsBubbleShown() const {
+ return bubble_view_ && bubble_view_->GetWidget();
+}
+
void ResizeToggleMenu::CloseBubble() {
if (!bubble_widget_ || bubble_widget_->IsClosed())
return;
diff --git a/ash/components/arc/compat_mode/resize_toggle_menu.h b/ash/components/arc/compat_mode/resize_toggle_menu.h
index b3eb77b..76b4bea5 100644
--- a/ash/components/arc/compat_mode/resize_toggle_menu.h
+++ b/ash/components/arc/compat_mode/resize_toggle_menu.h
@@ -79,6 +79,8 @@
intptr_t old) override;
void OnWindowDestroying(aura::Window* window) override;
+ bool IsBubbleShown() const;
+
private:
friend class ResizeToggleMenuTest;
@@ -88,6 +90,8 @@
gfx::Rect GetAnchorRect() const;
+ base::WeakPtr<views::BubbleDialogDelegateView> bubble_view_;
+
std::unique_ptr<views::BubbleDialogDelegateView> MakeBubbleDelegateView(
views::Widget* parent,
gfx::Rect anchor_rect,
@@ -106,8 +110,9 @@
base::CancelableOnceClosure auto_close_closure_;
- // Store only for testing.
views::Widget* bubble_widget_{nullptr};
+
+ // Store only for testing.
MenuButtonView* phone_button_{nullptr};
MenuButtonView* tablet_button_{nullptr};
MenuButtonView* resizable_button_{nullptr};
diff --git a/ash/components/arc/compat_mode/resize_toggle_menu_unittest.cc b/ash/components/arc/compat_mode/resize_toggle_menu_unittest.cc
index 69ca134..f380049 100644
--- a/ash/components/arc/compat_mode/resize_toggle_menu_unittest.cc
+++ b/ash/components/arc/compat_mode/resize_toggle_menu_unittest.cc
@@ -61,7 +61,10 @@
SyncResizeLockPropertyWithMojoState(widget());
}
+ void CloseBubble() { resize_toggle_menu_->CloseBubble(); }
+
views::Widget* widget() { return widget_.get(); }
+ ResizeToggleMenu* resize_toggle_menu() { return resize_toggle_menu_.get(); }
private:
views::Button* GetButtonByCommandId(ResizeCompatMode command_id) {
@@ -265,4 +268,13 @@
widget()->SetFullscreen(false);
}
+// Test that IsBubbleOpen returns the correct state of the bubble
+TEST_F(ResizeToggleMenuTest, TestIsBubbleShown) {
+ EXPECT_TRUE(IsMenuRunning());
+ EXPECT_TRUE(resize_toggle_menu()->IsBubbleShown());
+ CloseBubble();
+ RunPendingMessages();
+ EXPECT_FALSE(resize_toggle_menu()->IsBubbleShown());
+}
+
} // namespace arc
