From 081e17d359df888ecd253359d2d8c390852ddbd9 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Fri, 13 Dec 2024 07:46:29 +0900 Subject: [PATCH 1/3] Added separator to inactive split view tab fix https://github.com/brave/brave-browser/issues/42304 Also applied latest split view tab's background color. --- browser/ui/color/brave_color_id.h | 15 +++++------ browser/ui/tabs/brave_tab_color_mixer.cc | 26 +++++++++----------- browser/ui/views/tabs/brave_tab_container.cc | 22 ++++++++++++++++- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/browser/ui/color/brave_color_id.h b/browser/ui/color/brave_color_id.h index 019a49ab415a..e81904152c0d 100644 --- a/browser/ui/color/brave_color_id.h +++ b/browser/ui/color/brave_color_id.h @@ -126,13 +126,14 @@ #define BRAVE_SPLIT_VIEW_COLOR_IDS \ - E_CPONLY(kColorBraveSplitViewTileBackground) \ - E_CPONLY(kColorBraveSplitViewActiveWebViewBorder) \ - E_CPONLY(kColorBraveSplitViewInactiveWebViewBorder) \ - E_CPONLY(kColorBraveSplitViewMenuButtonIcon) \ - E_CPONLY(kColorBraveSplitViewMenuButtonBackground) \ - E_CPONLY(kColorBraveSplitViewMenuButtonBorder) \ - E_CPONLY(kColorBraveSplitViewMenuItemIcon) \ + E_CPONLY(kColorBraveSplitViewTileBackgroundHorizontal) \ + E_CPONLY(kColorBraveSplitViewTileBackgroundVertical) \ + E_CPONLY(kColorBraveSplitViewActiveWebViewBorder) \ + E_CPONLY(kColorBraveSplitViewInactiveWebViewBorder) \ + E_CPONLY(kColorBraveSplitViewMenuButtonIcon) \ + E_CPONLY(kColorBraveSplitViewMenuButtonBackground) \ + E_CPONLY(kColorBraveSplitViewMenuButtonBorder) \ + E_CPONLY(kColorBraveSplitViewMenuItemIcon) \ E_CPONLY(kColorBraveSplitViewUrl) diff --git a/browser/ui/tabs/brave_tab_color_mixer.cc b/browser/ui/tabs/brave_tab_color_mixer.cc index 23da39543782..61f068899043 100644 --- a/browser/ui/tabs/brave_tab_color_mixer.cc +++ b/browser/ui/tabs/brave_tab_color_mixer.cc @@ -21,13 +21,6 @@ void AddBraveTabThemeColorMixer(ui::ColorProvider* provider, const ui::ColorProviderKey& key) { auto& mixer = provider->AddMixer(); - // This is the default dark theme. We need this because we customize some of - // the default dark colors. - auto is_grayscale_dark = - key.color_mode == ui::ColorProviderKey::ColorMode::kDark && - key.user_color_source == - ui::ColorProviderKey::UserColorSource::kGrayscale; - mixer[kColorBraveVerticalTabActiveBackground] = { kColorTabBackgroundInactiveFrameActive}; mixer[kColorBraveVerticalTabInactiveBackground] = {kColorToolbar}; @@ -43,11 +36,10 @@ void AddBraveTabThemeColorMixer(ui::ColorProvider* provider, mixer[kColorBraveVerticalTabNTBShortcutTextColor] = { kColorTabForegroundActiveFrameActive}; - // Unfortunately, we need to specify a different blend amount in the default - // dark theme, as we override the Upstream colors. - mixer[kColorBraveSplitViewTileBackground] = - ui::BlendTowardMaxContrast(kColorTabBackgroundInactiveFrameActive, - (is_grayscale_dark ? 0.15 : 0.075) * 0xFF); + mixer[kColorBraveSplitViewTileBackgroundHorizontal] = { + nala::kColorDesktopbrowserTabbarSplitViewBackgroundHorizontal}; + mixer[kColorBraveSplitViewTileBackgroundVertical] = { + nala::kColorDesktopbrowserTabbarSplitViewBackgroundVertical}; mixer[kColorBraveSplitViewMenuItemIcon] = {nala::kColorIconDefault}; mixer[kColorBraveSplitViewUrl] = {nala::kColorTextTertiary}; mixer[kColorBraveSplitViewMenuButtonBorder] = {nala::kColorDividerSubtle}; @@ -74,7 +66,10 @@ void AddBraveTabPrivateThemeColorMixer(ui::ColorProvider* provider, mixer.GetResultColor(kColorTabBackgroundActiveFrameActive)}; mixer[kColorBraveVerticalTabInactiveBackground] = { mixer.GetResultColor(kColorToolbar)}; - mixer[kColorBraveSplitViewTileBackground] = {SkColorSetRGB(0x2A, 0xD, 0x58)}; + mixer[kColorBraveSplitViewTileBackgroundHorizontal] = { + SkColorSetRGB(0x2A, 0xD, 0x58)}; + mixer[kColorBraveSplitViewTileBackgroundVertical] = { + kColorBraveSplitViewTileBackgroundHorizontal}; } void AddBraveTabTorThemeColorMixer(ui::ColorProvider* provider, @@ -84,7 +79,10 @@ void AddBraveTabTorThemeColorMixer(ui::ColorProvider* provider, mixer.GetResultColor(kColorTabBackgroundActiveFrameActive)}; mixer[kColorBraveVerticalTabInactiveBackground] = { mixer.GetResultColor(kColorToolbar)}; - mixer[kColorBraveSplitViewTileBackground] = {SkColorSetRGB(0x35, 0x0B, 0x49)}; + mixer[kColorBraveSplitViewTileBackgroundHorizontal] = { + SkColorSetRGB(0x35, 0x0B, 0x49)}; + mixer[kColorBraveSplitViewTileBackgroundVertical] = { + kColorBraveSplitViewTileBackgroundHorizontal}; } } // namespace tabs diff --git a/browser/ui/views/tabs/brave_tab_container.cc b/browser/ui/views/tabs/brave_tab_container.cc index fb8f7a4a8db5..7f3b57550c11 100644 --- a/browser/ui/views/tabs/brave_tab_container.cc +++ b/browser/ui/views/tabs/brave_tab_container.cc @@ -23,6 +23,7 @@ #include "brave/browser/ui/views/tabs/brave_tab_group_header.h" #include "brave/browser/ui/views/tabs/brave_tab_strip.h" #include "brave/browser/ui/views/tabs/vertical_tab_utils.h" +#include "brave/ui/color/nala/nala_color_id.h" #include "cc/paint/paint_flags.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/color/chrome_color_id.h" @@ -340,9 +341,28 @@ void BraveTabContainer::PaintBoundingBoxForTile(gfx::Canvas& canvas, DCHECK(cp); cc::PaintFlags flags; - flags.setColor(cp->GetColor(kColorBraveSplitViewTileBackground)); + flags.setColor(cp->GetColor( + is_vertical_tab ? kColorBraveSplitViewTileBackgroundVertical + : kColorBraveSplitViewTileBackgroundHorizontal)); canvas.DrawRoundRect(bounding_rects, kRadius, flags); + + auto active_tab_handle = + tab_strip_model->GetTabHandleAt(tab_strip_model->active_index()); + if (!is_vertical_tab && active_tab_handle != tile.first && + active_tab_handle != tile.second) { + constexpr int kSplitViewSeparatorHeight = 24; + auto separator_top = bounding_rects.top_center(); + CHECK_GT(bounding_rects.height(), kSplitViewSeparatorHeight); + // Calculate gap between tab bounds top and separator top. + const int gap = (bounding_rects.height() - kSplitViewSeparatorHeight) / 2; + separator_top.Offset(0, gap); + auto separator_bottom = separator_top; + separator_bottom.Offset(0, kSplitViewSeparatorHeight); + canvas.DrawLine( + separator_top, separator_bottom, + cp->GetColor(nala::kColorDesktopbrowserTabbarSplitViewDivider)); + } } void BraveTabContainer::OnUnlockLayout() { From 30c47979f384f85e0dcdb9f5c9bfb90bc9abb8bb Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Fri, 13 Dec 2024 22:56:00 +0900 Subject: [PATCH 2/3] Applied latest split view bg color to priv/tor window --- browser/ui/color/brave_color_id.h | 1 + browser/ui/tabs/brave_tab_color_mixer.cc | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/browser/ui/color/brave_color_id.h b/browser/ui/color/brave_color_id.h index e81904152c0d..82c87802ef08 100644 --- a/browser/ui/color/brave_color_id.h +++ b/browser/ui/color/brave_color_id.h @@ -128,6 +128,7 @@ #define BRAVE_SPLIT_VIEW_COLOR_IDS \ E_CPONLY(kColorBraveSplitViewTileBackgroundHorizontal) \ E_CPONLY(kColorBraveSplitViewTileBackgroundVertical) \ + E_CPONLY(kColorBraveSplitViewTileDivider) \ E_CPONLY(kColorBraveSplitViewActiveWebViewBorder) \ E_CPONLY(kColorBraveSplitViewInactiveWebViewBorder) \ E_CPONLY(kColorBraveSplitViewMenuButtonIcon) \ diff --git a/browser/ui/tabs/brave_tab_color_mixer.cc b/browser/ui/tabs/brave_tab_color_mixer.cc index 61f068899043..0c743580b5a6 100644 --- a/browser/ui/tabs/brave_tab_color_mixer.cc +++ b/browser/ui/tabs/brave_tab_color_mixer.cc @@ -40,6 +40,8 @@ void AddBraveTabThemeColorMixer(ui::ColorProvider* provider, nala::kColorDesktopbrowserTabbarSplitViewBackgroundHorizontal}; mixer[kColorBraveSplitViewTileBackgroundVertical] = { nala::kColorDesktopbrowserTabbarSplitViewBackgroundVertical}; + mixer[kColorBraveSplitViewTileDivider] = { + nala::kColorDesktopbrowserTabbarSplitViewDivider}; mixer[kColorBraveSplitViewMenuItemIcon] = {nala::kColorIconDefault}; mixer[kColorBraveSplitViewUrl] = {nala::kColorTextTertiary}; mixer[kColorBraveSplitViewMenuButtonBorder] = {nala::kColorDividerSubtle}; @@ -67,9 +69,11 @@ void AddBraveTabPrivateThemeColorMixer(ui::ColorProvider* provider, mixer[kColorBraveVerticalTabInactiveBackground] = { mixer.GetResultColor(kColorToolbar)}; mixer[kColorBraveSplitViewTileBackgroundHorizontal] = { - SkColorSetRGB(0x2A, 0xD, 0x58)}; + nala::kColorPrimitivePrivateWindow10}; mixer[kColorBraveSplitViewTileBackgroundVertical] = { kColorBraveSplitViewTileBackgroundHorizontal}; + mixer[kColorBraveSplitViewTileDivider] = { + nala::kColorPrimitivePrivateWindow20}; } void AddBraveTabTorThemeColorMixer(ui::ColorProvider* provider, @@ -80,9 +84,10 @@ void AddBraveTabTorThemeColorMixer(ui::ColorProvider* provider, mixer[kColorBraveVerticalTabInactiveBackground] = { mixer.GetResultColor(kColorToolbar)}; mixer[kColorBraveSplitViewTileBackgroundHorizontal] = { - SkColorSetRGB(0x35, 0x0B, 0x49)}; + nala::kColorPrimitiveTorWindow10}; mixer[kColorBraveSplitViewTileBackgroundVertical] = { kColorBraveSplitViewTileBackgroundHorizontal}; + mixer[kColorBraveSplitViewTileDivider] = {nala::kColorPrimitiveTorWindow20}; } } // namespace tabs From 1e4887afa821c0652eef20184345269b4660e780 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Fri, 13 Dec 2024 21:43:46 +0900 Subject: [PATCH 3/3] Updated active/hover bg color of inactive tab in split view fix https://github.com/brave/brave-browser/issues/42879 Also adjusted tab's bg color of vertical tab. --- browser/ui/color/brave_color_id.h | 1 - browser/ui/color/brave_color_mixer.cc | 7 +++ browser/ui/tabs/brave_tab_color_mixer.cc | 4 -- .../views/tabs/brave_tab_style_views.inc.cc | 52 +++++++++++-------- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/browser/ui/color/brave_color_id.h b/browser/ui/color/brave_color_id.h index 82c87802ef08..32c363832532 100644 --- a/browser/ui/color/brave_color_id.h +++ b/browser/ui/color/brave_color_id.h @@ -113,7 +113,6 @@ E_CPONLY(kColorBraveVerticalTabSeparator) \ E_CPONLY(kColorBraveVerticalTabActiveBackground) \ E_CPONLY(kColorBraveVerticalTabInactiveBackground) \ - E_CPONLY(kColorBraveVerticalTabInactiveHoverBackground) \ E_CPONLY(kColorBraveVerticalTabNTBIconColor) \ E_CPONLY(kColorBraveVerticalTabNTBTextColor) \ E_CPONLY(kColorBraveVerticalTabNTBShortcutTextColor) diff --git a/browser/ui/color/brave_color_mixer.cc b/browser/ui/color/brave_color_mixer.cc index 5a293a4956ca..ee10ec85782d 100644 --- a/browser/ui/color/brave_color_mixer.cc +++ b/browser/ui/color/brave_color_mixer.cc @@ -724,4 +724,11 @@ void AddBravifiedTabStripColorMixer(ui::ColorProvider* provider, ui::AlphaBlend({nala::kColorDividerStrong}, {kColorTabBackgroundInactiveFrameActive}, 0.75 * 0xff); mixer[kColorTabDividerFrameInactive] = {kColorTabDividerFrameActive}; + + if (key.color_mode == ui::ColorProviderKey::ColorMode::kDark) { + mixer[kColorTabBackgroundActiveFrameActive] = { + nala::kColorDesktopbrowserTabbarActiveTabHorizontal}; + mixer[kColorTabBackgroundInactiveHoverFrameActive] = { + nala::kColorDesktopbrowserTabbarHoverTabHorizontal}; + } } diff --git a/browser/ui/tabs/brave_tab_color_mixer.cc b/browser/ui/tabs/brave_tab_color_mixer.cc index 0c743580b5a6..120a3cb9f16d 100644 --- a/browser/ui/tabs/brave_tab_color_mixer.cc +++ b/browser/ui/tabs/brave_tab_color_mixer.cc @@ -24,10 +24,6 @@ void AddBraveTabThemeColorMixer(ui::ColorProvider* provider, mixer[kColorBraveVerticalTabActiveBackground] = { kColorTabBackgroundInactiveFrameActive}; mixer[kColorBraveVerticalTabInactiveBackground] = {kColorToolbar}; - mixer[kColorBraveVerticalTabInactiveHoverBackground] = - ui::AlphaBlend(kColorBraveVerticalTabActiveBackground, - kColorBraveVerticalTabInactiveBackground, - /* 40% opacity */ 0.4 * SK_AlphaOPAQUE); mixer[kColorBraveVerticalTabSeparator] = {kColorToolbarContentAreaSeparator}; mixer[kColorBraveVerticalTabNTBIconColor] = { kColorTabForegroundInactiveFrameActive}; diff --git a/browser/ui/views/tabs/brave_tab_style_views.inc.cc b/browser/ui/views/tabs/brave_tab_style_views.inc.cc index 18103af8fe5f..dd1bca0354fb 100644 --- a/browser/ui/views/tabs/brave_tab_style_views.inc.cc +++ b/browser/ui/views/tabs/brave_tab_style_views.inc.cc @@ -7,6 +7,7 @@ // depending on what's defined in anonymous namespace of tab_style_views.cc #include "brave/browser/ui/tabs/split_view_browser_data.h" +#include "brave/ui/color/nala/nala_color_id.h" namespace { @@ -169,15 +170,6 @@ SkPath BraveVerticalTabStyle::GetPath( } } - const bool is_tab_tiled = IsTabTiled(tab()); - - if (is_tab_tiled && !tab()->IsActive() && - (path_type == TabStyle::PathType::kBorder || - path_type == TabStyle::PathType::kFill)) { - // We don't want inactive tab in a tile to have border or fill. - return SkPath(); - } - const bool is_pinned = tab()->data().pinned; // Calculate the bounds of the actual path. @@ -216,7 +208,7 @@ SkPath BraveVerticalTabStyle::GetPath( } } - if (is_tab_tiled && path_type != TabStyle::PathType::kHitTest) { + if (IsTabTiled(tab()) && path_type != TabStyle::PathType::kHitTest) { if (ShouldShowVerticalTabs()) { constexpr auto kPaddingForVerticalTab = 4; tab_top += scale * kPaddingForVerticalTab; @@ -397,28 +389,46 @@ void BraveVerticalTabStyle::PaintTab(gfx::Canvas* canvas) const { SkColor BraveVerticalTabStyle::GetTargetTabBackgroundColor( TabStyle::TabSelectionState selection_state, bool hovered) const { + const ui::ColorProvider* cp = tab()->GetColorProvider(); + if (!cp) { + return gfx::kPlaceholderColor; + } + + // Tab in tile doesn't have background in inactive state. + // In split view tile, we don't have selected tab's background. + // When any tab in a tile is clicked, the other tab in a same tile + // is also selected because clicking is start point of dragging. + // Because of that, whenever click a tab in a tile, the other tab's + // background is changed as its becomes selected tab. + // It's not easy to know whether selected state is from clicking or + // dragging here. As having selected tab state in a tile is not a + // common state, I think it's fine to not have that state in a tile. + if (IsTabTiled(tab()) && !tab()->IsActive() && !hovered) { + return cp->GetColor(ShouldShowVerticalTabs() + ? kColorBraveSplitViewTileBackgroundVertical + : kColorBraveSplitViewTileBackgroundHorizontal); + } + if (!ShouldShowVerticalTabs()) { return BraveTabStyleViews::GetTargetTabBackgroundColor(selection_state, hovered); } - if (selection_state == TabStyle::TabSelectionState::kSelected) { - // Use the same color if th tab is selected via multiselection. - return BraveTabStyleViews::GetTargetTabBackgroundColor(selection_state, - hovered); + if (tab()->IsActive()) { + return cp->GetColor(nala::kColorDesktopbrowserTabbarActiveTabVertical); } - const ui::ColorProvider* cp = tab()->GetColorProvider(); - if (!cp) { - return gfx::kPlaceholderColor; + if (hovered) { + return cp->GetColor(nala::kColorDesktopbrowserTabbarHoverTabVertical); } - if (selection_state == TabStyle::TabSelectionState::kInactive) { - return cp->GetColor(hovered ? kColorBraveVerticalTabInactiveHoverBackground - : kColorBraveVerticalTabInactiveBackground); + if (selection_state == TabStyle::TabSelectionState::kSelected) { + // Use the same color if th tab is selected via multiselection. + return BraveTabStyleViews::GetTargetTabBackgroundColor(selection_state, + hovered); } - return cp->GetColor(kColorBraveVerticalTabActiveBackground); + return cp->GetColor(kColorBraveVerticalTabInactiveBackground); } bool BraveVerticalTabStyle::ShouldShowVerticalTabs() const {