Commit e1e12c69 authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

Update omnibox selection/hover colors to match spec; this improves contrast.

See comment 8 on the bug for the spec link.

Bug: 883451
Change-Id: I218eb090519bb44d37afd75bf0fb7ad66c323949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1756711
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688247}
parent ba5a78a9
...@@ -49,9 +49,7 @@ SkColor GetOmniboxColor(OmniboxPart part, ...@@ -49,9 +49,7 @@ SkColor GetOmniboxColor(OmniboxPart part,
// For high contrast, selected rows use inverted colors to stand out more. // For high contrast, selected rows use inverted colors to stand out more.
ui::NativeTheme* native_theme = ui::NativeTheme::GetInstanceForNativeUi(); ui::NativeTheme* native_theme = ui::NativeTheme::GetInstanceForNativeUi();
bool high_contrast = native_theme && native_theme->UsesHighContrastColors(); bool high_contrast = native_theme && native_theme->UsesHighContrastColors();
bool selected = state == OmniboxPartState::SELECTED || if (high_contrast && (state == OmniboxPartState::SELECTED))
state == OmniboxPartState::HOVERED_AND_SELECTED;
if (high_contrast && selected)
dark = !dark; dark = !dark;
switch (part) { switch (part) {
...@@ -66,15 +64,15 @@ SkColor GetOmniboxColor(OmniboxPart part, ...@@ -66,15 +64,15 @@ SkColor GetOmniboxColor(OmniboxPart part,
case OmniboxPart::LOCATION_BAR_SELECTED_KEYWORD: case OmniboxPart::LOCATION_BAR_SELECTED_KEYWORD:
return dark ? gfx::kGoogleGrey100 : gfx::kGoogleBlue600; return dark ? gfx::kGoogleGrey100 : gfx::kGoogleBlue600;
case OmniboxPart::RESULTS_BACKGROUND: { case OmniboxPart::RESULTS_BACKGROUND: {
// High contrast mode needs a darker base - Grey 800 with 14% white // High contrast mode needs a darker base - Grey 800 with 16% white
// overlaid on it (see below) is hard to produce good contrast ratios // overlaid on it (see below) is hard to produce good contrast ratios
// against with colors other than white. // against with colors other than white.
const SkColor dark_base_color = const SkColor dark_base_color =
high_contrast ? gfx::kGoogleGrey900 : gfx::kGoogleGrey800; high_contrast ? gfx::kGoogleGrey900 : gfx::kGoogleGrey800;
const SkColor base_color = dark ? dark_base_color : SK_ColorWHITE; const SkColor base_color = dark ? dark_base_color : SK_ColorWHITE;
// The spec calls for transparent black (or white) overlays for hover (8%) // The spec calls for transparent black (or white) overlays for hover
// and select (6%), which can overlap (for 14%). Pre-blend these with the // (10%) and select (16%). Pre-blend these with the background for the
// background for the best text AA result. // best text AA result.
return color_utils::BlendTowardMaxContrast( return color_utils::BlendTowardMaxContrast(
base_color, base_color,
gfx::ToRoundedInt(GetOmniboxStateOpacity(state) * SK_AlphaOPAQUE)); gfx::ToRoundedInt(GetOmniboxStateOpacity(state) * SK_AlphaOPAQUE));
...@@ -132,18 +130,7 @@ SkColor GetOmniboxSecurityChipColor( ...@@ -132,18 +130,7 @@ SkColor GetOmniboxSecurityChipColor(
} }
float GetOmniboxStateOpacity(OmniboxPartState state) { float GetOmniboxStateOpacity(OmniboxPartState state) {
switch (state) { DCHECK_LE(state, OmniboxPartState::SELECTED);
case OmniboxPartState::NORMAL: constexpr float kOpacities[3] = {0.00f, 0.10f, 0.16f};
return 0; return kOpacities[static_cast<size_t>(state)];
case OmniboxPartState::HOVERED:
return 0.08f;
case OmniboxPartState::SELECTED:
return 0.06f;
case OmniboxPartState::HOVERED_AND_SELECTED:
return GetOmniboxStateOpacity(OmniboxPartState::HOVERED) +
GetOmniboxStateOpacity(OmniboxPartState::SELECTED);
default:
NOTREACHED();
return 0;
}
} }
...@@ -35,7 +35,6 @@ enum class OmniboxPartState { ...@@ -35,7 +35,6 @@ enum class OmniboxPartState {
NORMAL, NORMAL,
HOVERED, HOVERED,
SELECTED, SELECTED,
HOVERED_AND_SELECTED,
// Applicable to LOCATION_BAR_SECURITY_CHIP only. // Applicable to LOCATION_BAR_SECURITY_CHIP only.
CHIP_DEFAULT, CHIP_DEFAULT,
......
...@@ -233,10 +233,8 @@ bool OmniboxResultView::IsSelected() const { ...@@ -233,10 +233,8 @@ bool OmniboxResultView::IsSelected() const {
} }
OmniboxPartState OmniboxResultView::GetThemeState() const { OmniboxPartState OmniboxResultView::GetThemeState() const {
if (IsSelected()) { if (IsSelected())
return is_hovered_ ? OmniboxPartState::HOVERED_AND_SELECTED return OmniboxPartState::SELECTED;
: OmniboxPartState::SELECTED;
}
return is_hovered_ ? OmniboxPartState::HOVERED : OmniboxPartState::NORMAL; return is_hovered_ ? OmniboxPartState::HOVERED : OmniboxPartState::NORMAL;
} }
......
...@@ -154,8 +154,7 @@ TEST_F(OmniboxResultViewTest, MouseDragWithLeftButtonSelectsThisResult) { ...@@ -154,8 +154,7 @@ TEST_F(OmniboxResultViewTest, MouseDragWithLeftButtonSelectsThisResult) {
// Left button drag should select. // Left button drag should select.
result_view()->OnMouseDragged( result_view()->OnMouseDragged(
CreateEvent(ui::ET_MOUSE_DRAGGED, ui::EF_LEFT_MOUSE_BUTTON)); CreateEvent(ui::ET_MOUSE_DRAGGED, ui::EF_LEFT_MOUSE_BUTTON));
EXPECT_EQ(OmniboxPartState::HOVERED_AND_SELECTED, EXPECT_EQ(OmniboxPartState::SELECTED, result_view()->GetThemeState());
result_view()->GetThemeState());
EXPECT_TRUE(popup_view()->IsSelectedIndex(kTestResultViewIndex)); EXPECT_TRUE(popup_view()->IsSelectedIndex(kTestResultViewIndex));
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment