New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 866689 link

Starred by 4 users
Status: Fixed
Owner:
kylixrd@chromium.org
Closed: Aug 15
Cc:
█ abdulsyed@chromium.org
pkasting@chromium.org
est...@chromium.org
Components:
EstimatedDays: ----
NextAction: 2018-08-15
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocked on:
issue 862664


Sign in to add a comment

Tab separators should be based on tab text color

Project Member Reported by pkasting@chromium.org, Jul 23 
Right now we take the separator background color and compute the opposite luma to create a base for the separator, then blend that 30% with the background.

Instead, we should use the background tab text color as the base, and blend 46% against the background, or enough to maintain a 1.84 contrast ratio, whichever is greater.

Comment 1 Deleted

Comment 2 by robliao@chromium.org, Jul 31

Labels: Group-Themes

Comment 3 by pkasting@chromium.org, Aug 7

Owner: kylixrd@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1fb00d5bfc13f0e8fa821936791655a1929abb56

commit 1fb00d5bfc13f0e8fa821936791655a1929abb56
Author: Peter Kasting <pkasting@chromium.org>
Date: Tue Aug 14 15:42:40 2018

Use tab foreground color blended with the background color for separator.

Bug:  866689 
Change-Id: I33ba303ddad0bec0de0624f98444c252ff5251ba
Reviewed-on: https://chromium-review.googlesource.com/1173752
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582922}
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/frame/browser_non_client_frame_view.h
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.cc
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.h
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/1fb00d5bfc13f0e8fa821936791655a1929abb56/chrome/browser/ui/views/tabs/tab_strip_controller.h

Comment 5 by pkasting@chromium.org, Aug 14

Labels: M-69 Merge-Request-69

Comment 6 by gov...@chromium.org, Aug 14

Cc: abdulsyed@chromium.org
NextAction: 2018-08-15
Pls update bug with canary result tomorrow and provide merge justification. Thank you.

Comment 7 by pkasting@chromium.org, Aug 14 

Merge justification is that for custom themes, the new tab separators in M69 could previously be colors that weren't really related to the theme at all (e.g. we got blueish separators for a black and red theme that's one of the top five Chrome themes).  The new separators should be related to the tab text colors and thus look less broken (on that same theme, the separators are now red like the text).

Comment 8 by monor...@bugs.chromium.org, Aug 15 

The NextAction date has arrived: 2018-08-15

Comment 9 by pkasting@chromium.org, Aug 15 

Verified working in canary.

Comment 10 by gov...@chromium.org, Aug 15

Labels: -Merge-Request-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #7 and #9. Please merge. Thank you.
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 15

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9666fe12830daa2c4200064010059469278c8fa

commit b9666fe12830daa2c4200064010059469278c8fa
Author: Peter Kasting <pkasting@chromium.org>
Date: Wed Aug 15 15:35:53 2018

Use tab foreground color blended with the background color for separator.

Bug:  866689 
Change-Id: I33ba303ddad0bec0de0624f98444c252ff5251ba
Reviewed-on: https://chromium-review.googlesource.com/1173752
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#582922}(cherry picked from commit 1fb00d5bfc13f0e8fa821936791655a1929abb56)
Reviewed-on: https://chromium-review.googlesource.com/1175465
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#638}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/frame/browser_non_client_frame_view.h
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.cc
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.h
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/b9666fe12830daa2c4200064010059469278c8fa/chrome/browser/ui/views/tabs/tab_strip_controller.h

Comment 12 by pkasting@chromium.org, Aug 15

Status: Fixed (was: Assigned)
►

Sign in to add a comment