[Sidebar]: Fix margins

browser/ui/views/frame/vertical_tab_strip_region_view.cc

@@ -11,6 +11,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/functional/bind.h"
 #include "brave/app/vector_icons/vector_icons.h"
 #include "brave/browser/ui/brave_browser.h"
 #include "brave/browser/ui/color/brave_color_id.h"
@@ -32,7 +33,6 @@
 #include "chrome/browser/ui/tabs/tab_style.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_scroll_container.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
@@ -525,9 +525,14 @@ class VerticalTabStripRegionView::HeaderView : public views::View {
       return;
     }
 
+    // Note: We need to copy the vector of children here so we don't modify the
+    // vector while iterating.
+    std::vector<views::View*> children_copy;
+    base::ranges::copy(children(), std::back_inserter(children_copy));
+
     // View::ReorderChildView() didn't work for us. So remove child views and
     // add them again.
-    base::ranges::for_each(children(),
+    base::ranges::for_each(children_copy,
                            [&](views::View* v) { RemoveChildView(v); });
     base::ranges::for_each(new_children, [&](auto* v) { AddChildView(v); });
     layout_->SetFlexForView(spacer_,
@@ -666,9 +671,13 @@ VerticalTabStripRegionView::VerticalTabStripRegionView(
 
   vertical_tab_on_right_.Init(
       brave_tabs::kVerticalTabsOnRight, browser()->profile()->GetPrefs(),
-      base::BindRepeating(
-          &VerticalTabStripRegionView::OnVerticalTabPositionChanged,
-          base::Unretained(this)));
+      base::BindRepeating(&VerticalTabStripRegionView::OnBrowserPanelsMoved,
+                          base::Unretained(this)));
+
+  sidebar_side_.Init(
+      prefs::kSidePanelHorizontalAlignment, prefs,
+      base::BindRepeating(&VerticalTabStripRegionView::OnBrowserPanelsMoved,
+                          base::Unretained(this)));
 
   widget_observation_.Observe(browser_view->GetWidget());
 
@@ -944,7 +953,7 @@ void VerticalTabStripRegionView::OnShowVerticalTabsPrefChanged() {
   UpdateBorder();
 }
 
-void VerticalTabStripRegionView::OnVerticalTabPositionChanged() {
+void VerticalTabStripRegionView::OnBrowserPanelsMoved() {
   UpdateBorder();
   PreferredSizeChanged();
 }
@@ -1212,11 +1221,18 @@ void VerticalTabStripRegionView::UpdateBorder() {
            state_ == State::kFloating;
   };
 
-  int inset = 1 - BraveBrowser::GetRoundedCornersWebViewMargin(browser_);
-  gfx::Insets border_insets =
-      (!vertical_tab_on_right_.GetPrefName().empty() && *vertical_tab_on_right_)
-          ? gfx::Insets::TLBR(0, inset, 0, 0)
-          : gfx::Insets::TLBR(0, 0, 0, inset);
+  // If the sidebar is on the same side as the vertical tab strip, we shouldn't
+  // take away the margin on the vertical tabs, because the sidebar will be
+  // between it and the web_contents.
+  bool is_on_right =
+      !vertical_tab_on_right_.GetPrefName().empty() && *vertical_tab_on_right_;
+  bool sidebar_on_same_side = sidebar_side_.GetValue() == is_on_right;
+  int inset =
+      1 - (sidebar_on_same_side
+               ? 0
+               : BraveBrowser::GetRoundedCornersWebViewMargin(browser_));
+  gfx::Insets border_insets = (is_on_right) ? gfx::Insets::TLBR(0, inset, 0, 0)
+                                            : gfx::Insets::TLBR(0, 0, 0, inset);
 
   if (show_visible_border()) {
     SetBorder(views::CreateSolidSidedBorder(

browser/ui/views/frame/vertical_tab_strip_region_view.h

@@ -144,7 +144,7 @@ class VerticalTabStripRegionView : public views::View,
   void UpdateStateAfterDragAndDropFinished(State original_state);
 
   void OnShowVerticalTabsPrefChanged();
-  void OnVerticalTabPositionChanged();
+  void OnBrowserPanelsMoved();
 
   void UpdateLayout(bool in_destruction = false);
 
@@ -202,6 +202,7 @@ class VerticalTabStripRegionView : public views::View,
   BooleanPrefMember collapsed_pref_;
   BooleanPrefMember expanded_state_per_window_pref_;
   BooleanPrefMember floating_mode_pref_;
+  BooleanPrefMember sidebar_side_;
 
   IntegerPrefMember expanded_width_pref_;
   int expanded_width_ = 220;

browser/ui/views/sidebar/sidebar_control_view.cc

@@ -86,13 +86,14 @@ void SidebarControlView::UpdateBackgroundAndBorder() {
   if (const ui::ColorProvider* color_provider = GetColorProvider()) {
     SetBackground(
         views::CreateSolidBackground(color_provider->GetColor(kColorToolbar)));
-    if (!BraveBrowser::ShouldUseBraveWebViewRoundedCorners(browser_)) {
-      constexpr int kBorderThickness = 1;
-      SetBorder(views::CreateSolidSidedBorder(
-          gfx::Insets::TLBR(0, sidebar_on_left_ ? 0 : kBorderThickness, 0,
-                            sidebar_on_left_ ? kBorderThickness : 0),
-          color_provider->GetColor(kColorToolbarContentAreaSeparator)));
-    }
+    bool open =
+        browser_->sidebar_controller()->model()->active_index().has_value();
+    int border_thickness =
+        1 - (open ? 0 : BraveBrowser::GetRoundedCornersWebViewMargin(browser_));
+    SetBorder(views::CreateSolidSidedBorder(
+        gfx::Insets::TLBR(0, sidebar_on_left_ ? 0 : border_thickness, 0,
+                          sidebar_on_left_ ? border_thickness : 0),
+        color_provider->GetColor(kColorToolbarContentAreaSeparator)));
   }
 }
 
@@ -168,6 +169,13 @@ void SidebarControlView::OnItemRemoved(size_t index) {
   UpdateItemAddButtonState();
 }
 
+void SidebarControlView::OnActiveIndexChanged(std::optional<size_t> old_index,
+                                              std::optional<size_t> new_index) {
+  if (old_index.has_value() != new_index.has_value()) {
+    UpdateBackgroundAndBorder();
+  }
+}
+
 void SidebarControlView::AddChildViews() {
   sidebar_items_view_ =
       AddChildView(std::make_unique<SidebarItemsScrollView>(browser_));

browser/ui/views/sidebar/sidebar_control_view.h

@@ -69,6 +69,8 @@ class SidebarControlView : public views::View,
                    size_t index,
                    bool user_gesture) override;
   void OnItemRemoved(size_t index) override;
+  void OnActiveIndexChanged(std::optional<size_t> old_index,
+                            std::optional<size_t> new_index) override;
 
   void Update();