Fixed on mouseover doesn't work with right side sidebar

fix brave/brave-browser#26198 Mouse over detect widget's bound was not updated on right sidebar.
brave · Oct 24, 2022 · 591e415 · 591e415
1 parent feaae68
commit 591e415
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 5 deletions.
diff --git a/browser/ui/sidebar/sidebar_browsertest.cc b/browser/ui/sidebar/sidebar_browsertest.cc
@@ -65,6 +65,12 @@ class SidebarBrowserTest : public InProcessBrowserTest {
     return static_cast<BraveBrowserView*>(view)->vertical_tab_strip_host_view_;
   }
 
+  views::Widget* GetEventDetectWidget() {
+    auto* sidebar_container_view =
+        static_cast<SidebarContainerView*>(controller()->sidebar());
+    return sidebar_container_view->GetEventDetectWidget()->widget_.get();
+  }
+
   void SimulateSidebarItemClickAt(int index) const {
     auto* sidebar_container_view =
         static_cast<SidebarContainerView*>(controller()->sidebar());
@@ -231,6 +237,25 @@ class SidebarBrowserTestWithVerticalTabs : public SidebarBrowserTest {
   base::test::ScopedFeatureList feature_list_;
 };
 
+IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, EventDetectWidgetTest) {
+  auto* widget = GetEventDetectWidget();
+  auto* service = SidebarServiceFactory::GetForProfile(browser()->profile());
+  auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
+  auto* prefs = browser()->profile()->GetPrefs();
+
+  // Check widget is located on left side when sidebar on left.
+  prefs->SetBoolean(prefs::kSidePanelHorizontalAlignment, false);
+  service->SetSidebarShowOption(
+      SidebarService::ShowSidebarOption::kShowOnMouseOver);
+  EXPECT_EQ(browser_view->GetWidget()->GetRestoredBounds().x(),
+            widget->GetRestoredBounds().x());
+
+  // Check widget is located on right side when sidebar on right.
+  prefs->SetBoolean(prefs::kSidePanelHorizontalAlignment, true);
+  EXPECT_EQ(browser_view->GetWidget()->GetRestoredBounds().right(),
+            widget->GetRestoredBounds().right());
+}
+
 IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithVerticalTabs,
                        SidebarRightSideTest) {
   auto* prefs = browser()->profile()->GetPrefs();

diff --git a/browser/ui/views/sidebar/sidebar_container_view.cc b/browser/ui/views/sidebar/sidebar_container_view.cc
@@ -125,13 +125,21 @@ void SidebarContainerView::Init() {
   // Hide by default. Visibility will be controlled by show options later.
   DoHideSidebar(false);
   UpdateToolbarButtonVisibility();
+  SetSidebarOnLeft(sidebar_on_left_);
 }
 
 void SidebarContainerView::SetSidebarOnLeft(bool sidebar_on_left) {
+  if (sidebar_on_left_ == sidebar_on_left)
+    return;
+
   sidebar_on_left_ = sidebar_on_left;
-  if (sidebar_control_view_) {
-    sidebar_control_view_->SetSidebarOnLeft(sidebar_on_left_);
-  }
+
+  if (!initialized_)
+    return;
+
+  DCHECK(sidebar_control_view_);
+  sidebar_control_view_->SetSidebarOnLeft(sidebar_on_left_);
+  GetEventDetectWidget()->SetSidebarOnLeft(sidebar_on_left_);
 
   DCHECK(side_panel_);
   side_panel_->SetHorizontalAlignment(
@@ -187,7 +195,6 @@ void SidebarContainerView::AddChildViews() {
   // we want the controls first.
   sidebar_control_view_ =
       AddChildViewAt(std::make_unique<SidebarControlView>(this, browser_), 0);
-  sidebar_control_view_->SetSidebarOnLeft(sidebar_on_left_);
 }
 
 void SidebarContainerView::Layout() {

diff --git a/browser/ui/views/sidebar/sidebar_show_options_event_detect_widget.cc b/browser/ui/views/sidebar/sidebar_show_options_event_detect_widget.cc
@@ -59,15 +59,21 @@ SidebarShowOptionsEventDetectWidget::~SidebarShowOptionsEventDetectWidget() =
 
 void SidebarShowOptionsEventDetectWidget::Show() {
   DCHECK(widget_);
-  widget_->Show();
   AdjustWidgetBounds();
+  widget_->Show();
 }
 
 void SidebarShowOptionsEventDetectWidget::Hide() {
   DCHECK(widget_);
   widget_->Hide();
 }
 
+void SidebarShowOptionsEventDetectWidget::SetSidebarOnLeft(
+    bool sidebar_on_left) {
+  sidebar_on_left_ = sidebar_on_left;
+  AdjustWidgetBounds();
+}
+
 std::unique_ptr<views::Widget>
 SidebarShowOptionsEventDetectWidget::CreateWidget(Delegate* delegate) {
   std::unique_ptr<views::Widget> widget(new views::Widget);
@@ -95,7 +101,11 @@ void SidebarShowOptionsEventDetectWidget::OnViewBoundsChanged(
 void SidebarShowOptionsEventDetectWidget::AdjustWidgetBounds() {
   auto rect = browser_view_->contents_container()->bounds();
   constexpr int kWidgetNarrowWidth = 7;
+  if (!sidebar_on_left_) {
+    rect.set_x(rect.right() - kWidgetNarrowWidth);
+  }
   rect.set_width(kWidgetNarrowWidth);
+
   contents_view_->SetPreferredSize(rect.size());
   widget_->SetBounds(rect);
 }
diff --git a/browser/ui/views/sidebar/sidebar_show_options_event_detect_widget.h b/browser/ui/views/sidebar/sidebar_show_options_event_detect_widget.h
@@ -16,8 +16,14 @@ namespace views {
 class Widget;
 }  // namespace views
 
+namespace sidebar {
+class SidebarBrowserTest;
+}  // namespace sidebar
+
 class BraveBrowserView;
 
+// Monitors mouse event to show sidebar when mouser is around the left or
+// right side of browser window.
 class SidebarShowOptionsEventDetectWidget : public views::ViewObserver,
                                             public views::WidgetDelegate {
  public:
@@ -40,16 +46,20 @@ class SidebarShowOptionsEventDetectWidget : public views::ViewObserver,
 
   void Show();
   void Hide();
+  void SetSidebarOnLeft(bool sidebar_on_left);
 
   // views::ViewObserver overrides:
   void OnViewBoundsChanged(views::View* observed_view) override;
 
  private:
+  friend class sidebar::SidebarBrowserTest;
+
   class ContentsView;
 
   std::unique_ptr<views::Widget> CreateWidget(Delegate* delegate);
   void AdjustWidgetBounds();
 
+  bool sidebar_on_left_ = true;
   raw_ptr<BraveBrowserView> browser_view_ = nullptr;
   raw_ptr<ContentsView> contents_view_ = nullptr;
   raw_ptr<Delegate> delegate_ = nullptr;