From 725def1fdca811f948368718ec0aff2d664b19a3 Mon Sep 17 00:00:00 2001 From: petemill Date: Tue, 18 Sep 2018 19:14:50 -0700 Subject: [PATCH] Specify color for focus ring Fix https://github.com/brave/brave-browser/issues/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. --- browser/ui/views/frame/brave_browser_frame.cc | 6 +++ chromium_src/ui/views/controls/focus_ring.cc | 49 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 chromium_src/ui/views/controls/focus_ring.cc diff --git a/browser/ui/views/frame/brave_browser_frame.cc b/browser/ui/views/frame/brave_browser_frame.cc index d33c12838e49..3721b1ef0a14 100644 --- a/browser/ui/views/frame/brave_browser_frame.cc +++ b/browser/ui/views/frame/brave_browser_frame.cc @@ -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( @@ -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(); } \ No newline at end of file diff --git a/chromium_src/ui/views/controls/focus_ring.cc b/chromium_src/ui/views/controls/focus_ring.cc new file mode 100644 index 000000000000..9002a9dcb0c0 --- /dev/null +++ b/chromium_src/ui/views/controls/focus_ring.cc @@ -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 instance; + return instance.get(); +} + +} + +#define GetNativeTheme GetFocusRingTheme +#include "../../../../ui/views/controls/focus_ring.cc" +#undef GetNativeTheme