From 5d2d6f37d07ba0920118ed26246f925f2c72c6ba Mon Sep 17 00:00:00 2001 From: Jay Harris Date: Tue, 1 Oct 2024 11:59:40 +1300 Subject: [PATCH] [Rounded Corners]: Fix shadows --- browser/ui/brave_browser.cc | 7 +++++ browser/ui/brave_browser.h | 1 + .../views/frame/brave_browser_view_layout.cc | 27 ++++++++++++------- .../views/frame/brave_contents_view_util.cc | 7 +++-- .../frame/vertical_tab_strip_region_view.cc | 10 ++++--- .../ui/views/side_panel/brave_side_panel.cc | 9 ++++++- 6 files changed, 43 insertions(+), 18 deletions(-) diff --git a/browser/ui/brave_browser.cc b/browser/ui/brave_browser.cc index 350f9f2cef03..a3dbdfd94aa0 100644 --- a/browser/ui/brave_browser.cc +++ b/browser/ui/brave_browser.cc @@ -18,6 +18,7 @@ #include "brave/browser/ui/brave_file_select_utils.h" #include "brave/browser/ui/tabs/brave_tab_prefs.h" #include "brave/browser/ui/tabs/features.h" +#include "brave/browser/ui/views/frame/brave_contents_view_util.h" #include "brave/components/constants/pref_names.h" #include "chrome/browser/lifetime/browser_close_manager.h" #include "chrome/browser/profiles/profile.h" @@ -57,6 +58,12 @@ bool BraveBrowser::ShouldUseBraveWebViewRoundedCorners(Browser* browser) { browser->is_type_normal(); } +int BraveBrowser::GetRoundedCornersWebViewMargin(Browser* browser) { + return ShouldUseBraveWebViewRoundedCorners(browser) + ? BraveContentsViewUtil::kMarginThickness + : 0; +} + BraveBrowser::BraveBrowser(const CreateParams& params) : Browser(params) { #if defined(TOOLKIT_VIEWS) if (!sidebar::CanUseSidebar(this)) { diff --git a/browser/ui/brave_browser.h b/browser/ui/brave_browser.h index f4e02d2b1e3d..15b70fe01ce2 100644 --- a/browser/ui/brave_browser.h +++ b/browser/ui/brave_browser.h @@ -32,6 +32,7 @@ class BraveBrowser : public Browser { BraveBrowser& operator=(const BraveBrowser&) = delete; static bool ShouldUseBraveWebViewRoundedCorners(Browser* browser); + static int GetRoundedCornersWebViewMargin(Browser* browser); // Browser overrides: void ScheduleUIUpdate(content::WebContents* source, diff --git a/browser/ui/views/frame/brave_browser_view_layout.cc b/browser/ui/views/frame/brave_browser_view_layout.cc index 3f790d22e2a8..2af91c9c9bee 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(). @@ -250,14 +254,19 @@ 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 = + BraveBrowser::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 +291,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..2095f458554e 100644 --- a/browser/ui/views/frame/brave_contents_view_util.cc +++ b/browser/ui/views/frame/brave_contents_view_util.cc @@ -12,10 +12,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( 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 5146193900b1..ff092698b35c 100644 --- a/browser/ui/views/frame/vertical_tab_strip_region_view.cc +++ b/browser/ui/views/frame/vertical_tab_strip_region_view.cc @@ -32,6 +32,7 @@ #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" @@ -483,8 +484,8 @@ class VerticalTabStripRegionView::HeaderView : public views::View { void UpdateTabSearchButtonVisibility() { tab_search_button_->SetVisible( tab_search_button_->GetPreferredSize().width() + - toggle_button_->GetPreferredSize().width() <= - width()); + toggle_button_->GetPreferredSize().width() <= + width()); if (!tab_search_button_->GetVisible()) { // When it's not visible, move tab search button. Otherwise, // TabSearchBubble will be anchored to wrong position as LayoutManager @@ -1211,10 +1212,11 @@ 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, 1, 0, 0) - : gfx::Insets::TLBR(0, 0, 0, 1); + ? 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/side_panel/brave_side_panel.cc b/browser/ui/views/side_panel/brave_side_panel.cc index ca323448b455..898f6f45a2dd 100644 --- a/browser/ui/views/side_panel/brave_side_panel.cc +++ b/browser/ui/views/side_panel/brave_side_panel.cc @@ -72,9 +72,16 @@ bool BraveSidePanel::IsRightAligned() { void BraveSidePanel::UpdateBorder() { if (BraveBrowser::ShouldUseBraveWebViewRoundedCorners( browser_view_->browser())) { + // In rounded corners mode we need to add the margin to the web contents + // container instead of the sidebar, so we have somewhere to render the + // shadow. + int content_margin = + -BraveBrowser::GetRoundedCornersWebViewMargin(browser_view_->browser()); // Use a negative top border to hide the separator inserted by the upstream // side panel implementation. - SetBorder(views::CreateEmptyBorder(gfx::Insets::TLBR(-1, 0, 0, 0))); + SetBorder(views::CreateEmptyBorder( + gfx::Insets::TLBR(-1, IsRightAligned() ? content_margin : 0, 0, + IsRightAligned() ? 0 : content_margin))); return; }