Skip to content

Commit

Permalink
Adjust sidebar colors
Browse files Browse the repository at this point in the history
fix brave/brave-browser#21590

So far, sidebar can't aligned with custom theme
because sidebar only knows brave's dark/light theme color.

With below changes, sidebar colors are aligned well with custom theme also.

Used toolbar color as a sidebar background color.
Used toolbar button's hover color for sidebar item/arrow button hover color.
Picked proper color between light/dark color for builtin item icon color based on toolbar color.
Used toolbar area separator color(separator between toolbar and web contents) for sidebar border.
  • Loading branch information
simonhong committed Mar 16, 2022
1 parent b57d4b5 commit 92c133d
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 62 deletions.
39 changes: 39 additions & 0 deletions browser/themes/brave_theme_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
#include "base/numerics/safe_conversions.h"
#include "brave/browser/themes/brave_dark_mode_utils.h"
#include "brave/browser/themes/theme_properties.h"
#include "brave/components/sidebar/buildflags/buildflags.h"
#include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/omnibox/omnibox_theme.h"
#include "ui/gfx/color_palette.h"
#include "ui/gfx/color_utils.h"
#include "ui/native_theme/native_theme.h"

#if defined(OS_LINUX)
Expand Down Expand Up @@ -104,6 +106,43 @@ SkColor BraveThemeHelper::GetDefaultColor(
if (!incognito && (is_tor_ || is_guest_)) {
incognito = true;
}

#if BUILDFLAG(ENABLE_SIDEBAR)
switch (id) {
// Pick most contrast color between our light and dark colors based on
// current toolbar color.
case BraveThemeProperties::COLOR_SIDEBAR_ITEM_DRAG_INDICATOR_COLOR:
case BraveThemeProperties::COLOR_SIDEBAR_ADD_BUTTON_DISABLED:
case BraveThemeProperties::COLOR_SIDEBAR_BUTTON_BASE:
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_NORMAL:
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_DISABLED: {
const auto toolbar_color =
GetColor(ThemeProperties::COLOR_TOOLBAR, incognito, theme_supplier);
const auto base_button_color_light = MaybeGetDefaultColorForBraveUi(
id, incognito, is_tor_,
dark_mode::BraveDarkModeType::BRAVE_DARK_MODE_TYPE_LIGHT);
const auto base_button_color_dark = MaybeGetDefaultColorForBraveUi(
id, incognito, is_tor_,
dark_mode::BraveDarkModeType::BRAVE_DARK_MODE_TYPE_DARK);
DCHECK(base_button_color_light && base_button_color_dark);
return color_utils::PickContrastingColor(base_button_color_light.value(),
base_button_color_dark.value(),
toolbar_color);
}
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_BACKGROUND_HOVERED:
case BraveThemeProperties::COLOR_SIDEBAR_ITEM_BACKGROUND_HOVERED: {
// Copied from chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h
// to use same hover background with toolbar button.
constexpr float kToolbarInkDropHighlightVisibleOpacity = 0.08f;
return SkColorSetA(GetColor(ThemeProperties::COLOR_TOOLBAR_INK_DROP,
incognito, theme_supplier),
0xFF * kToolbarInkDropHighlightVisibleOpacity);
}
default:
break;
}
#endif

const dark_mode::BraveDarkModeType type =
dark_mode::GetActiveBraveDarkModeType();
const absl::optional<SkColor> braveColor =
Expand Down
47 changes: 1 addition & 46 deletions browser/themes/theme_properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ absl::optional<SkColor> MaybeGetDefaultColorForBraveLightUi(int id) {
case BraveThemeProperties::COLOR_MENU_ITEM_SUB_TEXT_COLOR:
return SkColorSetRGB(0x86, 0x8E, 0x96);
#if BUILDFLAG(ENABLE_SIDEBAR)
case BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND:
return SkColorSetRGB(0xF3, 0xF3, 0xF5);
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_BACKGROUND_HOVERED:
return SkColorSetRGB(0xE3, 0xE3, 0xE3);
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_NORMAL:
return SkColorSetRGB(0x21, 0x25, 0x29);
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_DISABLED:
Expand All @@ -71,12 +67,6 @@ absl::optional<SkColor> MaybeGetDefaultColorForBraveLightUi(int id) {
return SkColorSetRGB(0x49, 0x50, 0x57);
case BraveThemeProperties::COLOR_SIDEBAR_ADD_BUTTON_DISABLED:
return SkColorSetARGB(0X66, 0x49, 0x50, 0x57);
case BraveThemeProperties::COLOR_SIDEBAR_BORDER:
return SkColorSetRGB(0xD5, 0xD5, 0xDC);
case BraveThemeProperties::COLOR_SIDEBAR_PANEL_BORDER:
return SkColorSetARGB(0x21, 0x00, 0x00, 0x00);
case BraveThemeProperties::COLOR_SIDEBAR_ITEM_BACKGROUND:
return SkColorSetRGB(0xE8, 0xE8, 0xE8);
case BraveThemeProperties::COLOR_SIDEBAR_ITEM_DRAG_INDICATOR_COLOR:
return SkColorSetRGB(0x21, 0x25, 0x29);
case BraveThemeProperties::COLOR_SIDEBAR_SEPARATOR:
Expand Down Expand Up @@ -165,10 +155,6 @@ absl::optional<SkColor> MaybeGetDefaultColorForBraveDarkUi(int id) {
case BraveThemeProperties::COLOR_MENU_ITEM_SUB_TEXT_COLOR:
return SkColorSetRGB(0x84, 0x88, 0x9C);
#if BUILDFLAG(ENABLE_SIDEBAR)
case BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND:
return kDarkToolbar;
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_BACKGROUND_HOVERED:
return SkColorSetRGB(0x42, 0x45, 0x51);
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_DISABLED:
return SkColorSetARGB(0x8A, 0xAE, 0xB1, 0xC2);
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_NORMAL:
Expand All @@ -177,12 +163,6 @@ absl::optional<SkColor> MaybeGetDefaultColorForBraveDarkUi(int id) {
return SkColorSetRGB(0xC2, 0xC4, 0xCF);
case BraveThemeProperties::COLOR_SIDEBAR_ADD_BUTTON_DISABLED:
return SkColorSetARGB(0x66, 0xC2, 0xC4, 0xCF);
case BraveThemeProperties::COLOR_SIDEBAR_BORDER:
return SkColorSetRGB(0x3B, 0x3E, 0x4F);
case BraveThemeProperties::COLOR_SIDEBAR_PANEL_BORDER:
return SkColorSetARGB(0x21, 0x00, 0x00, 0x00);
case BraveThemeProperties::COLOR_SIDEBAR_ITEM_BACKGROUND:
return SkColorSetRGB(0x41, 0x44, 0x51);
case BraveThemeProperties::COLOR_SIDEBAR_ITEM_DRAG_INDICATOR_COLOR:
return SkColorSetRGB(0xC2, 0xC4, 0xCF);
case BraveThemeProperties::COLOR_SIDEBAR_SEPARATOR:
Expand Down Expand Up @@ -253,18 +233,6 @@ absl::optional<SkColor> MaybeGetDefaultColorForPrivateUi(int id) {
return kDarkToolbarIcon;
case ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON_INACTIVE:
return color_utils::AlphaBlend(kDarkToolbarIcon, kPrivateToolbar, 0.3f);
#if BUILDFLAG(ENABLE_SIDEBAR)
case BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND:
return kPrivateToolbar;
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_BACKGROUND_HOVERED:
return SkColorSetRGB(0x4B, 0x3E, 0x78);
case BraveThemeProperties::COLOR_SIDEBAR_BORDER:
return SkColorSetRGB(0x3B, 0x3E, 0x4F);
case BraveThemeProperties::COLOR_SIDEBAR_PANEL_BORDER:
return SkColorSetARGB(0x21, 0x00, 0x00, 0x00);
case BraveThemeProperties::COLOR_SIDEBAR_ITEM_BACKGROUND:
return SkColorSetRGB(0x4B, 0x3E, 0x78);
#endif
case BraveThemeProperties::COLOR_FOR_TEST:
return BraveThemeProperties::kPrivateColorForTest;
// The rest is covered by a dark-appropriate value
Expand Down Expand Up @@ -295,21 +263,8 @@ absl::optional<SkColor> MaybeGetDefaultColorForPrivateTorUi(int id) {
case ThemeProperties::COLOR_TAB_BACKGROUND_ACTIVE_FRAME_INACTIVE:
return kPrivateTorToolbar;
case ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON_INACTIVE:
return color_utils::AlphaBlend(kDarkToolbarIcon,
kPrivateTorToolbar,
return color_utils::AlphaBlend(kDarkToolbarIcon, kPrivateTorToolbar,
0.3f);
#if BUILDFLAG(ENABLE_SIDEBAR)
case BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND:
return kPrivateTorToolbar;
case BraveThemeProperties::COLOR_SIDEBAR_ARROW_BACKGROUND_HOVERED:
return SkColorSetRGB(0x5F, 0x42, 0x6F);
case BraveThemeProperties::COLOR_SIDEBAR_BORDER:
return SkColorSetRGB(0x3B, 0x3E, 0x4F);
case BraveThemeProperties::COLOR_SIDEBAR_PANEL_BORDER:
return SkColorSetARGB(0x21, 0x00, 0x00, 0x00);
case BraveThemeProperties::COLOR_SIDEBAR_ITEM_BACKGROUND:
return SkColorSetRGB(0x5F, 0x42, 0x6F);
#endif
// The rest is covered by a private value
default:
return MaybeGetDefaultColorForPrivateUi(id);
Expand Down
11 changes: 4 additions & 7 deletions browser/themes/theme_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,18 @@ enum ThemeProperties {
COLOR_TOGGLE_BUTTON_TRACK_OFF_COLOR,
COLOR_MENU_ITEM_SUB_TEXT_COLOR,
#if BUILDFLAG(ENABLE_SIDEBAR)
COLOR_SIDEBAR_ADD_BUTTON_DISABLED,
COLOR_SIDEBAR_BACKGROUND,
COLOR_SIDEBAR_BUTTON_BASE,
COLOR_SIDEBAR_BORDER,
COLOR_SIDEBAR_PANEL_BORDER,
COLOR_SIDEBAR_ITEM_BACKGROUND,
COLOR_SIDEBAR_ITEM_DRAG_INDICATOR_COLOR,
COLOR_SIDEBAR_ADD_BUBBLE_BACKGROUND,
COLOR_SIDEBAR_ADD_BUTTON_DISABLED,
COLOR_SIDEBAR_ADD_BUBBLE_HEADER_TEXT,
COLOR_SIDEBAR_ADD_BUBBLE_ITEM_TEXT_NORMAL,
COLOR_SIDEBAR_ADD_BUBBLE_ITEM_TEXT_HOVERED,
COLOR_SIDEBAR_ADD_BUBBLE_ITEM_TEXT_BACKGROUND_HOVERED,
COLOR_SIDEBAR_ARROW_BACKGROUND_HOVERED,
COLOR_SIDEBAR_ARROW_NORMAL,
COLOR_SIDEBAR_ARROW_DISABLED,
COLOR_SIDEBAR_BUTTON_BASE,
COLOR_SIDEBAR_ITEM_BACKGROUND_HOVERED,
COLOR_SIDEBAR_ITEM_DRAG_INDICATOR_COLOR,
COLOR_SIDEBAR_SEPARATOR,
#endif
#if BUILDFLAG(ENABLE_SPEEDREADER)
Expand Down
7 changes: 4 additions & 3 deletions browser/ui/views/sidebar/sidebar_container_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "brave/browser/ui/views/sidebar/sidebar_control_view.h"
#include "brave/browser/ui/views/sidebar/sidebar_panel_webview.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "content/public/browser/browser_context.h"
Expand Down Expand Up @@ -161,13 +162,13 @@ void SidebarContainerView::UpdateBackgroundAndBorder() {
if (const ui::ThemeProvider* theme_provider = GetThemeProvider()) {
constexpr int kBorderThickness = 1;
// Fill background because panel's color uses alpha value.
SetBackground(views::CreateSolidBackground(theme_provider->GetColor(
BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND)));
SetBackground(views::CreateSolidBackground(
theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR)));
if (sidebar_panel_webview_ && sidebar_panel_webview_->GetVisible()) {
SetBorder(views::CreateSolidSidedBorder(
0, 0, 0, kBorderThickness,
theme_provider->GetColor(
BraveThemeProperties::COLOR_SIDEBAR_PANEL_BORDER)));
ThemeProperties::COLOR_TOOLBAR_CONTENT_AREA_SEPARATOR)));
} else {
// Don't need right side border when panel is closed.
SetBorder(nullptr);
Expand Down
8 changes: 5 additions & 3 deletions browser/ui/views/sidebar/sidebar_control_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "brave/components/sidebar/sidebar_service.h"
#include "brave/grit/brave_generated_resources.h"
#include "brave/grit/brave_theme_resources.h"
#include "chrome/browser/themes/theme_properties.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/base/resource/resource_bundle.h"
Expand Down Expand Up @@ -115,11 +116,12 @@ void SidebarControlView::OnThemeChanged() {
void SidebarControlView::UpdateBackgroundAndBorder() {
if (const ui::ThemeProvider* theme_provider = GetThemeProvider()) {
constexpr int kBorderThickness = 1;
SetBackground(views::CreateSolidBackground(theme_provider->GetColor(
BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND)));
SetBackground(views::CreateSolidBackground(
theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR)));
SetBorder(views::CreateSolidSidedBorder(
0, 0, 0, kBorderThickness,
theme_provider->GetColor(BraveThemeProperties::COLOR_SIDEBAR_BORDER)));
theme_provider->GetColor(
ThemeProperties::COLOR_TOOLBAR_CONTENT_AREA_SEPARATOR)));
}
}

Expand Down
2 changes: 1 addition & 1 deletion browser/ui/views/sidebar/sidebar_item_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void SidebarItemView::OnPaintBackground(gfx::Canvas* canvas) {
canvas->FillRect(
GetLocalBounds(),
theme_provider->GetColor(
BraveThemeProperties::COLOR_SIDEBAR_ITEM_BACKGROUND));
BraveThemeProperties::COLOR_SIDEBAR_ITEM_BACKGROUND_HOVERED));
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions browser/ui/views/sidebar/sidebar_items_scroll_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "brave/components/sidebar/sidebar_service.h"
#include "brave/grit/brave_generated_resources.h"
#include "cc/paint/paint_flags.h"
#include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/browser_list.h"
#include "ui/base/clipboard/clipboard.h"
#include "ui/base/clipboard/clipboard_format_type.h"
Expand Down Expand Up @@ -61,8 +62,8 @@ class SidebarItemsArrowView : public views::ImageButton {

void OnPaintBackground(gfx::Canvas* canvas) override {
if (const ui::ThemeProvider* theme_provider = GetThemeProvider()) {
const SkColor background_color = theme_provider->GetColor(
BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND);
const SkColor background_color =
theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR);
gfx::Rect bounds = GetContentsBounds();
canvas->FillRect(bounds, background_color);

Expand Down

0 comments on commit 92c133d

Please sign in to comment.