diff --git a/browser/ui/views/frame/brave_browser_view_layout.cc b/browser/ui/views/frame/brave_browser_view_layout.cc index 3f790d22e2a8..b84cf4b1ca0b 100644 --- a/browser/ui/views/frame/brave_browser_view_layout.cc +++ b/browser/ui/views/frame/brave_browser_view_layout.cc @@ -67,8 +67,9 @@ void BraveBrowserViewLayout::Layout(views::View* host) { } void BraveBrowserViewLayout::LayoutVerticalTabs() { - if (!vertical_tab_strip_host_.get()) + if (!vertical_tab_strip_host_.get()) { return; + } if (!tabs::utils::ShouldShowVerticalTabs(browser_view_->browser())) { vertical_tab_strip_host_->SetBorder(nullptr); @@ -108,10 +109,11 @@ void BraveBrowserViewLayout::LayoutVerticalTabs() { insets = AdjustInsetsConsideringFrameBorder(insets); #endif - if (insets.IsEmpty()) + if (insets.IsEmpty()) { vertical_tab_strip_host_->SetBorder(nullptr); - else + } else { vertical_tab_strip_host_->SetBorder(views::CreateEmptyBorder(insets)); + } const auto width = vertical_tab_strip_host_->GetPreferredSize().width() + insets.width(); @@ -147,8 +149,9 @@ int BraveBrowserViewLayout::LayoutTabStripRegion(int top) { int BraveBrowserViewLayout::LayoutBookmarkAndInfoBars(int top, int browser_view_y) { - if (!vertical_tab_strip_host_ || !ShouldPushBookmarkBarForVerticalTabs()) + if (!vertical_tab_strip_host_ || !ShouldPushBookmarkBarForVerticalTabs()) { return BrowserViewLayout::LayoutBookmarkAndInfoBars(top, browser_view_y); + } auto new_rect = vertical_layout_rect_; new_rect.Inset(GetInsetsConsideringVerticalTabHost()); @@ -157,8 +160,9 @@ int BraveBrowserViewLayout::LayoutBookmarkAndInfoBars(int top, } int BraveBrowserViewLayout::LayoutInfoBar(int top) { - if (!vertical_tab_strip_host_) + if (!vertical_tab_strip_host_) { return BrowserViewLayout::LayoutInfoBar(top); + } if (ShouldPushBookmarkBarForVerticalTabs()) { // Insets are already applied from LayoutBookmarkAndInfoBar(). @@ -204,8 +208,8 @@ void BraveBrowserViewLayout::LayoutSideBar(gfx::Rect& contents_bounds) { #endif gfx::Rect separator_bounds; - - if (sidebar_container_->sidebar_on_left()) { + const bool on_left = sidebar_container_->sidebar_on_left(); + if (on_left) { contents_bounds.set_x(contents_bounds.x() + sidebar_bounds.width()); // When vertical tabs and the sidebar are adjacent, add a separator between @@ -224,10 +228,22 @@ void BraveBrowserViewLayout::LayoutSideBar(gfx::Rect& contents_bounds) { sidebar_bounds.set_x(contents_bounds.right()); } - // Side panel doesn't need margin as sidebar UI and contents container - // will have margins if needed. gfx::Insets panel_margins = GetContentsMargins(); - panel_margins.set_left_right(0, 0); + if (BraveBrowser::ShouldUseBraveWebViewRoundedCorners( + browser_view_->browser())) { + // In rounded mode, there is already a gap between the sidebar and the main + // contents view, so we only remove from the margin from that side (we need + // to keep it between the sidebar controls and the sidebar content). + if (on_left) { + panel_margins.set_right(0); + } else { + panel_margins.set_left(0); + } + } else { + // Side panel doesn't need margin as sidebar UI and contents container + // will have margins if needed. + panel_margins.set_left_right(0, 0); + } sidebar_container_->side_panel()->SetProperty(views::kMarginsKey, panel_margins); @@ -250,14 +266,20 @@ void BraveBrowserViewLayout::UpdateContentsContainerInsets( // Control contents's margin with sidebar & vertical tab state. gfx::Insets contents_margins = GetContentsMargins(); + // In rounded corners mode, we need to include a little margin so we have + // somewhere to draw the shadow. + int contents_margin_for_rounded_corners = + BraveContentsViewUtil::GetRoundedCornersWebViewMargin( + browser_view_->browser()); + // Don't need contents container's left or right margin with vertical tab as // vertical tab itself has sufficient padding. if (tabs::utils::ShouldShowVerticalTabs(browser_view_->browser()) && !IsFullscreenForBrowser()) { if (tabs::utils::IsVerticalTabOnRight(browser_view_->browser())) { - contents_margins.set_right(0); + contents_margins.set_right(contents_margin_for_rounded_corners); } else { - contents_margins.set_left(0); + contents_margins.set_left(contents_margin_for_rounded_corners); } } @@ -282,9 +304,9 @@ void BraveBrowserViewLayout::UpdateContentsContainerInsets( // If sidebar is shown in left-side, contents container doens't need its // left margin. if (sidebar_container_->sidebar_on_left()) { - contents_margins.set_left(0); + contents_margins.set_left(contents_margin_for_rounded_corners); } else { - contents_margins.set_right(0); + contents_margins.set_right(contents_margin_for_rounded_corners); } contents_container_bounds.Inset(contents_margins); } diff --git a/browser/ui/views/frame/brave_contents_view_util.cc b/browser/ui/views/frame/brave_contents_view_util.cc index f552fd8e1e84..35140dc3cc98 100644 --- a/browser/ui/views/frame/brave_contents_view_util.cc +++ b/browser/ui/views/frame/brave_contents_view_util.cc @@ -5,6 +5,8 @@ #include "brave/browser/ui/views/frame/brave_contents_view_util.h" +#include "brave/browser/ui/brave_browser.h" +#include "chrome/browser/ui/browser.h" #include "ui/compositor/layer.h" #include "ui/views/view.h" @@ -12,10 +14,9 @@ namespace { constexpr ViewShadow::ShadowParameters kShadow{ .offset_x = 0, - .offset_y = 1, - .blur_radius = 4, - .shadow_color = SkColorSetA(SK_ColorBLACK, 0.07 * 255)}; - + .offset_y = 0, + .blur_radius = BraveContentsViewUtil::kMarginThickness, + .shadow_color = SkColorSetA(SK_ColorBLACK, 0.1 * 255)}; } // namespace std::unique_ptr BraveContentsViewUtil::CreateShadow( @@ -26,3 +27,9 @@ std::unique_ptr BraveContentsViewUtil::CreateShadow( view->layer()->SetIsFastRoundedCorner(true); return shadow; } + +int BraveContentsViewUtil::GetRoundedCornersWebViewMargin(Browser* browser) { + return BraveBrowser::ShouldUseBraveWebViewRoundedCorners(browser) + ? BraveContentsViewUtil::kMarginThickness + : 0; +} diff --git a/browser/ui/views/frame/brave_contents_view_util.h b/browser/ui/views/frame/brave_contents_view_util.h index 937f4d4fe819..42aaee9a282f 100644 --- a/browser/ui/views/frame/brave_contents_view_util.h +++ b/browser/ui/views/frame/brave_contents_view_util.h @@ -10,6 +10,8 @@ #include "brave/browser/ui/views/view_shadow.h" +class Browser; + namespace views { class View; } @@ -28,6 +30,10 @@ class BraveContentsViewUtil { // Creates a drop shadow for the specified content area view. static std::unique_ptr CreateShadow(views::View* view); + + // If rounded corners are enabled, returns the additional margin required to + // get the shadow to display properly. Otherwise 0. + static int GetRoundedCornersWebViewMargin(Browser* browser); }; #endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_CONTENTS_VIEW_UTIL_H_ diff --git a/browser/ui/views/frame/vertical_tab_strip_region_view.cc b/browser/ui/views/frame/vertical_tab_strip_region_view.cc index b46447d25cf4..9f9838d79e1c 100644 --- a/browser/ui/views/frame/vertical_tab_strip_region_view.cc +++ b/browser/ui/views/frame/vertical_tab_strip_region_view.cc @@ -11,10 +11,12 @@ #include #include +#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" #include "brave/browser/ui/tabs/brave_tab_prefs.h" +#include "brave/browser/ui/views/frame/brave_contents_view_util.h" #include "brave/browser/ui/views/tabs/brave_new_tab_button.h" #include "brave/browser/ui/views/tabs/brave_tab_search_button.h" #include "brave/browser/ui/views/tabs/brave_tab_strip_layout_helper.h" @@ -656,7 +658,6 @@ VerticalTabStripRegionView::VerticalTabStripRegionView( base::BindRepeating( &VerticalTabStripRegionView::OnFloatingModePrefChanged, base::Unretained(this))); - OnFloatingModePrefChanged(); #if BUILDFLAG(IS_MAC) show_toolbar_on_fullscreen_pref_.Init( @@ -667,9 +668,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()); @@ -680,6 +685,9 @@ VerticalTabStripRegionView::VerticalTabStripRegionView( BrowserList::GetInstance()->end()) << "Browser shouldn't be added at this point."; BrowserList::AddObserver(this); + + // Note: This should happen after all the PrefMembers have been initialized. + OnFloatingModePrefChanged(); } VerticalTabStripRegionView::~VerticalTabStripRegionView() { @@ -945,7 +953,7 @@ void VerticalTabStripRegionView::OnShowVerticalTabsPrefChanged() { UpdateBorder(); } -void VerticalTabStripRegionView::OnVerticalTabPositionChanged() { +void VerticalTabStripRegionView::OnBrowserPanelsMoved() { UpdateBorder(); PreferredSizeChanged(); } @@ -1091,8 +1099,10 @@ void VerticalTabStripRegionView::OnBoundsChanged( #if DCHECK_IS_ON() if (auto width = GetContentsBounds().width(); width && !IsBrowserFullscren()) { - CHECK_GE(width, tabs::kVerticalTabMinWidth + - tabs::kMarginForVerticalTabContainers * 2); + CHECK_GE( + width, + tabs::kVerticalTabMinWidth + tabs::kMarginForVerticalTabContainers * 2 - + BraveContentsViewUtil::GetRoundedCornersWebViewMargin(browser_)); } #endif } @@ -1213,10 +1223,19 @@ void VerticalTabStripRegionView::UpdateBorder() { state_ == State::kFloating; }; - gfx::Insets border_insets = - (!vertical_tab_on_right_.GetPrefName().empty() && *vertical_tab_on_right_) - ? gfx::Insets::TLBR(0, 1, 0, 0) - : gfx::Insets::TLBR(0, 0, 0, 1); + // 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 + : BraveContentsViewUtil::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( diff --git a/browser/ui/views/frame/vertical_tab_strip_region_view.h b/browser/ui/views/frame/vertical_tab_strip_region_view.h index f978512297f6..41e0d3942eea 100644 --- a/browser/ui/views/frame/vertical_tab_strip_region_view.h +++ b/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); @@ -228,6 +228,7 @@ class VerticalTabStripRegionView : public views::View, fullscreen_observation_{this}; BooleanPrefMember vertical_tab_on_right_; + BooleanPrefMember sidebar_side_; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/browser/ui/views/sidebar/sidebar_container_view.cc b/browser/ui/views/sidebar/sidebar_container_view.cc index 47e050963485..f690bf18e907 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.cc +++ b/browser/ui/views/sidebar/sidebar_container_view.cc @@ -531,7 +531,11 @@ void SidebarContainerView::ShowSidebar(bool show_side_panel) { animation_start_width_ = width(); animation_end_width_ = sidebar_control_view_->GetPreferredSize().width(); if (show_side_panel) { - animation_end_width_ += side_panel_->GetPreferredSize().width(); + // Note: as margins of |side_panel_| are part of |width()| we need to add + // them when calculating the ideal width of the contents. + animation_end_width_ += + side_panel_->GetPreferredSize().width() + + side_panel_->GetProperty(views::kMarginsKey)->width(); } // Don't need event detect widget when sidebar gets visible. diff --git a/browser/ui/views/sidebar/sidebar_control_view.cc b/browser/ui/views/sidebar/sidebar_control_view.cc index 4157e4bb8726..17247b198c35 100644 --- a/browser/ui/views/sidebar/sidebar_control_view.cc +++ b/browser/ui/views/sidebar/sidebar_control_view.cc @@ -11,6 +11,7 @@ #include "brave/browser/ui/sidebar/sidebar_controller.h" #include "brave/browser/ui/sidebar/sidebar_service_factory.h" #include "brave/browser/ui/sidebar/sidebar_utils.h" +#include "brave/browser/ui/views/frame/brave_contents_view_util.h" #include "brave/browser/ui/views/sidebar/sidebar_item_add_button.h" #include "brave/browser/ui/views/sidebar/sidebar_items_scroll_view.h" #include "brave/components/l10n/common/localization_util.h" @@ -19,8 +20,10 @@ #include "brave/grit/brave_generated_resources.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser_command_controller.h" +#include "chrome/browser/ui/browser_window/public/browser_window_features.h" #include "chrome/browser/ui/color/chrome_color_id.h" #include "chrome/browser/ui/singleton_tabs.h" +#include "chrome/browser/ui/views/side_panel/side_panel_ui.h" #include "chrome/common/pref_names.h" #include "components/prefs/pref_service.h" #include "ui/base/l10n/l10n_util.h" @@ -86,13 +89,11 @@ 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))); - } + int border_thickness = + 1 - BraveContentsViewUtil::GetRoundedCornersWebViewMargin(browser_); + SetBorder(views::CreateEmptyBorder( + gfx::Insets::TLBR(0, sidebar_on_left_ ? 0 : border_thickness, 0, + sidebar_on_left_ ? border_thickness : 0))); } } diff --git a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_web_ui_view.cc b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_web_ui_view.cc index e83e9f1a6b9d..cb35df44d575 100644 --- a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_web_ui_view.cc +++ b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_web_ui_view.cc @@ -10,10 +10,7 @@ void SidePanelWebUIView::AddedToWidget() { if (base::FeatureList::IsEnabled(features::kBraveWebViewRoundedCorners)) { - // Side panel WebUI views are positioned at the bottom of the side panel. In - // order to maintain rounded corners around the side panel, give the web - // contents native view rounded corners on the bottom. - constexpr auto radius = BraveContentsViewUtil::kBorderRadius; - holder()->SetCornerRadii(gfx::RoundedCornersF(0, 0, radius, radius)); + constexpr auto kRadius = BraveContentsViewUtil::kBorderRadius; + holder()->SetCornerRadii(gfx::RoundedCornersF(kRadius)); } }