Skip to content

Commit

Permalink
Updated active/hover bg color of inactive tab in split view
Browse files Browse the repository at this point in the history
fix brave/brave-browser#42879

Also adjusted tab's bg color of vertical tab.
  • Loading branch information
simonhong committed Dec 14, 2024
1 parent 30c4797 commit 1e4887a
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 26 deletions.
1 change: 0 additions & 1 deletion browser/ui/color/brave_color_id.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions browser/ui/color/brave_color_mixer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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};
}
}
4 changes: 0 additions & 4 deletions browser/ui/tabs/brave_tab_color_mixer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
52 changes: 31 additions & 21 deletions browser/ui/views/tabs/brave_tab_style_views.inc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 1e4887a

Please sign in to comment.