Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Browser UI focus ring should be Brave color cross-platform #1190

Closed
petemill opened this issue Sep 19, 2018 · 1 comment · Fixed by brave/brave-core#476
Closed

Browser UI focus ring should be Brave color cross-platform #1190

petemill opened this issue Sep 19, 2018 · 1 comment · Fixed by brave/brave-core#476
Assignees
@petemill
Labels
feature/user-interface All UI related QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes
Milestone
0.55.x - Release ...

Comments

@petemill
Copy link
Member

Focus rings in Chromium are determined by the Native theme, which can differ per-platform. Right now this seems to be the system setting for light theme, and hard-coded for dark / private theme.
We should change this to hard-coded for each / all themes since our spec specifies specific colors.

Separating this since we don't yet have any overriden Native Themes (dark or light, windows or linux or mac).
@petemill petemill added the feature/user-interface All UI related label Sep 19, 2018
@petemill petemill self-assigned this Sep 19, 2018
@petemill petemill added this to the Releasable builds 0.55.x milestone Sep 19, 2018
@petemill petemill mentioned this issue Sep 19, 2018
Merged
11 tasks
petemill added a commit to brave/brave-core that referenced this issue Sep 19, 2018
@petemill
Specify color for focus ring
725def1 
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.
@bbondy bbondy added the QA/Yes label Sep 27, 2018
@btlechowski
Copy link

btlechowski commented Sep 27, 2018 •
edited by LaurenWags
Loading

Verification passes on

Brave 0.55.10 Chromium: 70.0.3538.22 (Official Build) beta (64-bit)
Revision ac9418ba9c3bd7f6baaffa0b055dfe147e0f8364-refs/branch-heads/3538@{#468}
OS Windows 7

Checked URL Bar for themes Light and Dark.
Checked Windows: Normal. Private, Private with Tor and Guest

Verification Passed on

Brave 0.55.11 Chromium: 70.0.3538.35 (Official Build) beta (64-bit)
Revision 28dcb499844fa40c28d5f62e337876cb936f79f5-refs/branch-heads/3538@{#678}
OS Linux
  • Verified URL bar focus ring is orange for both light and dark theme
  • Verified URL bar focus ring is orange for Normal/Priavte/Guest/Tor windows
  • Verified URL bar focus ring is orange for both GTK+ and Classic Linux themes

Verified passed with

Brave 0.55.11 Chromium: 70.0.3538.35 (Official Build) beta(64-bit)
Revision 28dcb499844fa40c28d5f62e337876cb936f79f5-refs/branch-heads/3538@{#678}
OS Mac OS X
  • Verified URL bar focus ring is orange for both light and dark theme
  • Verified URL bar focus ring is orange for Normal/Private/Guest/Tor windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@petemill petemill

Labels
feature/user-interface All UI related QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes
Projects
None yet
Milestone
0.55.x - Release (Release Channel)
Development

Successfully merging a pull request may close this issue.

Specify color for focus ring brave/brave-core
5 participants
@petemill @bbondy @srirambv @LaurenWags @btlechowski