Skip to content

Commit

Permalink
Specify color for focus ring
Browse files Browse the repository at this point in the history
Fix brave/brave-browser#1190

In Chromium, this is specified in Native Theme, and overriden on macOS from the OS-level focus border color, but only for light theme (i.e. not incognito). In lieu of overriding all the Native Themes, this provides the 1 specific color to the FocusRing view. Whilst we may very well subclass all the Native Themes at some point, this is a pain-free way to get there for this feature of the design spec now.
  • Loading branch information
petemill committed Sep 19, 2018
1 parent c557639 commit 725def1
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 0 deletions.
6 changes: 6 additions & 0 deletions browser/ui/views/frame/brave_browser_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ BraveBrowserFrame::~BraveBrowserFrame() {
}

const ui::NativeTheme* BraveBrowserFrame::GetNativeTheme() const {
// Gets the platform-specific override for NativeTheme,
// unless we're in dark mode in which case get cross-platform
// dark theme.
// TODO: Have platform-specific version of dark theme too.
#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_CHROMEOS)
BraveThemeType active_builtin_theme =
BraveThemeService::GetActiveBraveThemeType(
Expand All @@ -28,5 +32,7 @@ const ui::NativeTheme* BraveBrowserFrame::GetNativeTheme() const {
return ui::NativeThemeDarkAura::instance();
}
#endif
// Each platform will implement ui::NativeTheme::GetInstanceForNativeUi
// separately, which Widget::GetNativeTheme calls.
return views::Widget::GetNativeTheme();
}
49 changes: 49 additions & 0 deletions chromium_src/ui/views/controls/focus_ring.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "base/no_destructor.h"
#include "ui/gfx/skia_util.h"
#include "ui/gfx/color_palette.h"

// include header first so that GetNativeTheme redefine doesn't flow to View
#include "ui/views/controls/focus_ring.h"

// Override the Focus Ring's color.
// In Chromium, this is specified via platform-specfic native theme,
// using kColorId_FocusedBorderColor. However, only macOS Light native theme
// overrides this. Since we do not have a Brave version of either
// platform-specific, or common versions, and we only want to override a single
// color, we use this micro-theme for the FocusRingView.
namespace {

class FocusRingTheme {
public:
SkColor GetSystemColor(int id) {
// At the time of implementation, only two Color IDs were possible.
// If this changes, consider overriding NativeTheme, or moving to
// ThemeProperties.
DCHECK(id == ui::NativeTheme::kColorId_FocusedBorderColor ||
id == ui::NativeTheme::kColorId_AlertSeverityHigh);
// Must be colors that are OK on dark or light bg since this is
// a very simplistic implementation.
switch (id) {
case ui::NativeTheme::kColorId_FocusedBorderColor:
return SkColorSetRGB(0xfb, 0x54, 0x2b);
case ui::NativeTheme::kColorId_AlertSeverityHigh:
return SkColorSetRGB(0xf4, 0x34, 0x05);
}
return gfx::kPlaceholderColor;
}
};

FocusRingTheme* GetFocusRingTheme() {
static base::NoDestructor<FocusRingTheme> instance;
return instance.get();
}

}

#define GetNativeTheme GetFocusRingTheme
#include "../../../../ui/views/controls/focus_ring.cc"
#undef GetNativeTheme

0 comments on commit 725def1

Please sign in to comment.