Commit 9f454719 authored by Christopher Grant's avatar Christopher Grant Committed by Commit Bot

VR: Omnibox color and layout refactoring.

This change restructures URL bar and omnibox code in several ways:

- Change all buttons to have transparent backgrounds, such that the
  UX-specified hover colors look correct.
- Enable background color animation on ALL buttons.
- Simplify the omnibox into a layout of transparent-background elements,
  with a single backblane (one element with rounded corners, background
  color and shadow).
- Make better use of layouts to remove instances of anchoring.
- Introduce helper functions to translate UX spec colors to constants.

BUG=821946

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I0dd6c9064cdec520ba45f0ad62aedf8fb4ad1364
Reviewed-on: https://chromium-review.googlesource.com/981050
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: default avatarIan Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545957}
parent bb3fe724
......@@ -30,7 +30,9 @@ Button::Button(base::RepeatingCallback<void()> click_handler,
background->SetType(kTypeButtonBackground);
background->set_bubble_events(true);
background->set_contributes_to_parent_bounds(false);
background->SetTransitionedProperties({TRANSFORM});
background->SetColor(colors_.background);
background->SetTransitionedProperties(
{TRANSFORM, BACKGROUND_COLOR, FOREGROUND_COLOR});
background_ = background.get();
AddChild(std::move(background));
......
......@@ -16,7 +16,7 @@ TextFormatting ConvertClassification(
size_t text_length,
const ColorScheme& color_scheme) {
TextFormatting formatting;
formatting.push_back(TextFormattingAttribute(color_scheme.suggestion_text,
formatting.push_back(TextFormattingAttribute(color_scheme.url_bar_text,
gfx::Range(0, text_length)));
for (size_t i = 0; i < classifications.size(); ++i) {
......@@ -41,11 +41,8 @@ TextFormatting ConvertClassification(
}
if (classifications[i].style & ACMatchClassification::URL) {
formatting.push_back(TextFormattingAttribute(
color_scheme.suggestion_url_text, current_range));
} else if (classifications[i].style & ACMatchClassification::DIM) {
formatting.push_back(TextFormattingAttribute(
color_scheme.suggestion_dim_text, current_range));
formatting.push_back(
TextFormattingAttribute(color_scheme.hyperlink, current_range));
} else if (classifications[i].style & ACMatchClassification::INVISIBLE) {
formatting.push_back(
TextFormattingAttribute(SK_ColorTRANSPARENT, current_range));
......
......@@ -16,7 +16,6 @@ namespace vr {
namespace {
constexpr SkColor kDefaultColor = 0xFF000001;
constexpr SkColor kDimColor = 0xFF000002;
constexpr SkColor kUrlColor = 0xFF000003;
constexpr bool kNoOffset = false;
......@@ -29,21 +28,19 @@ TEST(OmniboxFormatting, MultiLine) {
ACMatchClassification(0, ACMatchClassification::NONE),
ACMatchClassification(1, ACMatchClassification::URL),
ACMatchClassification(2, ACMatchClassification::MATCH),
ACMatchClassification(3, ACMatchClassification::DIM),
ACMatchClassification(4, ACMatchClassification::INVISIBLE),
ACMatchClassification(3, ACMatchClassification::INVISIBLE),
};
size_t string_length = classifications.size();
ColorScheme color_scheme;
memset(&color_scheme, 0, sizeof(color_scheme));
color_scheme.suggestion_text = kDefaultColor;
color_scheme.suggestion_dim_text = kDimColor;
color_scheme.suggestion_url_text = kUrlColor;
color_scheme.url_bar_text = kDefaultColor;
color_scheme.hyperlink = kUrlColor;
TextFormatting formatting =
ConvertClassification(classifications, string_length, color_scheme);
ASSERT_EQ(formatting.size(), 6u);
ASSERT_EQ(formatting.size(), 5u);
EXPECT_EQ(formatting[0].type(), TextFormattingAttribute::COLOR);
EXPECT_EQ(formatting[0].color(), kDefaultColor);
......@@ -62,12 +59,8 @@ TEST(OmniboxFormatting, MultiLine) {
EXPECT_EQ(formatting[3].range(), gfx::Range(2, 3));
EXPECT_EQ(formatting[4].type(), TextFormattingAttribute::COLOR);
EXPECT_EQ(formatting[4].color(), kDimColor);
EXPECT_EQ(formatting[4].color(), SK_ColorTRANSPARENT);
EXPECT_EQ(formatting[4].range(), gfx::Range(3, 4));
EXPECT_EQ(formatting[5].type(), TextFormattingAttribute::COLOR);
EXPECT_EQ(formatting[5].color(), SK_ColorTRANSPARENT);
EXPECT_EQ(formatting[5].range(), gfx::Range(4, 5));
}
struct ElisionTestcase {
......
......@@ -68,7 +68,7 @@ static const char* g_ui_element_name_strings[] = {
"kOmniboxVisibilityControlForAudioPermissionPrompt",
"kOmniboxDmmRoot",
"kOmniboxRoot",
"kOmniboxContainer",
"kOmniboxBackground",
"kOmniboxTextField",
"kOmniboxTextFieldLayout",
"kOmniboxVoiceSearchButton",
......@@ -76,7 +76,6 @@ static const char* g_ui_element_name_strings[] = {
"kOmniboxSuggestions",
"kOmniboxSuggestionsOuterLayout",
"kOmniboxOuterLayout",
"kOmniboxOuterLayoutSpacer",
"kOmniboxShadow",
"k2dBrowsingVisibiltyControlForVoice",
"k2dBrowsingVisibilityControlForPrompt",
......
......@@ -67,7 +67,7 @@ enum UiElementName {
kOmniboxVisibilityControlForAudioPermissionPrompt,
kOmniboxDmmRoot,
kOmniboxRoot,
kOmniboxContainer,
kOmniboxBackground,
kOmniboxTextField,
kOmniboxTextFieldLayout,
kOmniboxVoiceSearchButton,
......@@ -75,7 +75,6 @@ enum UiElementName {
kOmniboxSuggestions,
kOmniboxSuggestionsOuterLayout,
kOmniboxOuterLayout,
kOmniboxOuterLayoutSpacer,
kOmniboxShadow,
k2dBrowsingVisibiltyControlForVoice,
k2dBrowsingVisibilityControlForPrompt,
......
......@@ -25,7 +25,7 @@ void UrlBar::SetToolbarState(const ToolbarState& state) {
texture_->SetToolbarState(state);
}
void UrlBar::SetColors(const UrlBarColors& colors) {
void UrlBar::SetColors(const UrlTextColors& colors) {
texture_->SetColors(colors);
}
......
......@@ -19,7 +19,7 @@ namespace vr {
class UrlBarTexture;
struct ToolbarState;
struct UrlBarColors;
struct UrlTextColors;
class UrlBar : public TexturedElement {
public:
......@@ -29,7 +29,7 @@ class UrlBar : public TexturedElement {
~UrlBar() override;
void SetToolbarState(const ToolbarState& state);
void SetColors(const UrlBarColors& colors);
void SetColors(const UrlTextColors& colors);
private:
UiTexture* GetTexture() const override;
......
......@@ -35,7 +35,7 @@ constexpr float kHeight = kUrlBarHeightDMM;
void SetEmphasis(RenderTextWrapper* render_text,
bool emphasis,
const gfx::Range& range,
const UrlBarColors& colors) {
const UrlTextColors& colors) {
SkColor color = emphasis ? colors.emphasized : colors.deemphasized;
if (range.IsValid()) {
render_text->ApplyColor(color, range);
......@@ -99,7 +99,7 @@ float UrlBarTexture::ToMeters(float pixels) const {
return pixels * kWidth / size_.width();
}
void UrlBarTexture::SetColors(const UrlBarColors& colors) {
void UrlBarTexture::SetColors(const UrlTextColors& colors) {
SetAndDirty(&colors_, colors);
}
......@@ -148,11 +148,10 @@ void UrlBarTexture::Draw(SkCanvas* canvas, const gfx::Size& texture_size) {
// static
// This method replicates behavior in OmniboxView::UpdateTextStyle().
void UrlBarTexture::ApplyUrlStyling(
const base::string16& formatted_url,
void UrlBarTexture::ApplyUrlStyling(const base::string16& formatted_url,
const url::Parsed& parsed,
RenderTextWrapper* render_text,
const UrlBarColors& colors) {
const UrlTextColors& colors) {
const url::Component& scheme = parsed.scheme;
const url::Component& host = parsed.host;
......
......@@ -31,14 +31,14 @@ class UrlBarTexture : public UiTexture {
gfx::Size GetPreferredTextureSize(int width) const override;
gfx::SizeF GetDrawnSize() const override;
void SetColors(const UrlBarColors& colors);
void SetColors(const UrlTextColors& colors);
void SetToolbarState(const ToolbarState& state);
protected:
static void ApplyUrlStyling(const base::string16& formatted_url,
const url::Parsed& parsed,
RenderTextWrapper* render_text,
const UrlBarColors& colors);
const UrlTextColors& colors);
private:
void Draw(SkCanvas* canvas, const gfx::Size& texture_size) override;
......@@ -47,7 +47,7 @@ class UrlBarTexture : public UiTexture {
gfx::SizeF size_;
ToolbarState state_;
UrlBarColors colors_;
UrlTextColors colors_;
base::Callback<void(UiUnsupportedMode)> failure_callback_;
......
......@@ -27,15 +27,10 @@ using security_state::SecurityLevel;
namespace vr {
// TODO(cjgrant): Use ColorScheme instead of hardcoded values
// where it makes sense.
static const SkColor kEmphasizedColor = SK_ColorBLACK;
static const SkColor kDeemphasizedColor = 0xFF5A5A5A;
constexpr SkColor kEmphasizedColor = 0x01010101;
constexpr SkColor kDeemphasizedColor = 0x02020202;
static const SkColor kIncognitoDeemphasizedColor = 0xCCFFFFFF;
static const SkColor kIncognitoEmphasizedColor = 0xFFFFFFFF;
static constexpr int kUrlWidthPixels = 1024;
constexpr int kUrlWidthPixels = 1024;
class TestUrlBarTexture : public UrlBarTexture {
public:
......@@ -62,8 +57,10 @@ class TestUrlBarTexture : public UrlBarTexture {
static void TestUrlStyling(const base::string16& formatted_url,
const url::Parsed& parsed,
security_state::SecurityLevel security_level,
vr::RenderTextWrapper* render_text,
const UrlBarColors& colors) {
vr::RenderTextWrapper* render_text) {
UrlTextColors colors;
colors.deemphasized = kDeemphasizedColor;
colors.emphasized = kEmphasizedColor;
ApplyUrlStyling(formatted_url, parsed, render_text, colors);
}
......@@ -102,7 +99,11 @@ TestUrlBarTexture::TestUrlBarTexture()
base::BindRepeating(&TestUrlBarTexture::OnUnsupportedFeature,
base::Unretained(this))) {
gfx::FontList::SetDefaultFontDescription("Arial, Times New Roman, 15px");
SetColors(ColorScheme::GetColorScheme(ColorScheme::kModeNormal).url_bar);
UrlTextColors colors;
colors.deemphasized = kDeemphasizedColor;
colors.emphasized = kEmphasizedColor;
SetColors(colors);
SetBackgroundColor(SK_ColorBLACK);
SetForegroundColor(SK_ColorWHITE);
}
......@@ -118,12 +119,7 @@ class UrlEmphasisTest : public testing::Test {
url, GetVrFormatUrlTypes(), net::UnescapeRule::NORMAL, &parsed, nullptr,
nullptr);
EXPECT_EQ(formatted_url, base::UTF8ToUTF16(expected_string));
TestUrlBarTexture::TestUrlStyling(
formatted_url, parsed, level, &mock_,
ColorScheme::GetColorScheme(ColorScheme::kModeNormal).url_bar);
TestUrlBarTexture::TestUrlStyling(
formatted_url, parsed, level, &mock_,
ColorScheme::GetColorScheme(ColorScheme::kModeIncognito).url_bar);
TestUrlBarTexture::TestUrlStyling(formatted_url, parsed, level, &mock_);
}
testing::InSequence in_sequence_;
......@@ -143,16 +139,12 @@ TEST(UrlBarTextureTest, WillNotFailOnNonAsciiURLs) {
TEST_F(UrlEmphasisTest, SecureHttpsHost) {
EXPECT_CALL(mock_, SetColor(kDeemphasizedColor));
EXPECT_CALL(mock_, ApplyColor(kEmphasizedColor, gfx::Range(0, 8)));
EXPECT_CALL(mock_, SetColor(kIncognitoDeemphasizedColor));
EXPECT_CALL(mock_, ApplyColor(kIncognitoEmphasizedColor, gfx::Range(0, 8)));
Verify("https://host.com/page", SecurityLevel::SECURE, "host.com/page");
}
TEST_F(UrlEmphasisTest, NotSecureHttpsHost) {
EXPECT_CALL(mock_, SetColor(kDeemphasizedColor));
EXPECT_CALL(mock_, ApplyColor(kEmphasizedColor, gfx::Range(0, 8)));
EXPECT_CALL(mock_, SetColor(kIncognitoDeemphasizedColor));
EXPECT_CALL(mock_, ApplyColor(kIncognitoEmphasizedColor, gfx::Range(0, 8)));
Verify("https://host.com/page", SecurityLevel::HTTP_SHOW_WARNING,
"host.com/page");
}
......@@ -160,8 +152,6 @@ TEST_F(UrlEmphasisTest, NotSecureHttpsHost) {
TEST_F(UrlEmphasisTest, NotSecureHttpHost) {
EXPECT_CALL(mock_, SetColor(kDeemphasizedColor));
EXPECT_CALL(mock_, ApplyColor(kEmphasizedColor, gfx::Range(0, 8)));
EXPECT_CALL(mock_, SetColor(kIncognitoDeemphasizedColor));
EXPECT_CALL(mock_, ApplyColor(kIncognitoEmphasizedColor, gfx::Range(0, 8)));
Verify("http://host.com/page", SecurityLevel::HTTP_SHOW_WARNING,
"host.com/page");
}
......@@ -169,8 +159,6 @@ TEST_F(UrlEmphasisTest, NotSecureHttpHost) {
TEST_F(UrlEmphasisTest, Data) {
EXPECT_CALL(mock_, SetColor(kDeemphasizedColor));
EXPECT_CALL(mock_, ApplyColor(kEmphasizedColor, gfx::Range(0, 4)));
EXPECT_CALL(mock_, SetColor(kIncognitoDeemphasizedColor));
EXPECT_CALL(mock_, ApplyColor(kIncognitoEmphasizedColor, gfx::Range(0, 4)));
Verify("data:text/html,lots of data", SecurityLevel::NONE,
"data:text/html,lots of data");
}
......
This diff is collapsed.
......@@ -17,16 +17,16 @@ struct ButtonColors {
SkColor GetBackgroundColor(bool hovered, bool pressed) const;
SkColor GetForegroundColor(bool disabled) const;
SkColor background = SK_ColorBLACK;
SkColor background_hover = SK_ColorBLACK;
SkColor background_down = SK_ColorBLACK;
SkColor background = SK_ColorTRANSPARENT;
SkColor background_hover = SK_ColorTRANSPARENT;
SkColor background_down = SK_ColorTRANSPARENT;
SkColor foreground = SK_ColorBLACK;
SkColor foreground_disabled = SK_ColorBLACK;
};
struct UrlBarColors {
bool operator==(const UrlBarColors& other) const;
bool operator!=(const UrlBarColors& other) const;
struct UrlTextColors {
bool operator==(const UrlTextColors& other) const;
bool operator!=(const UrlTextColors& other) const;
SkColor deemphasized = SK_ColorBLACK;
SkColor emphasized = SK_ColorBLACK;
};
......@@ -61,16 +61,9 @@ struct ColorScheme {
SkColor floor_grid;
SkColor web_vr_background;
// The foreground color is used for text and sometimes for icons.
SkColor element_foreground;
// The background color is used behind text or icons in the foreground color.
// The related hover and down colors are to be used for buttons.
SkColor element_background;
SkColor element_background_hover;
SkColor element_background_down;
ButtonColors disc_button_colors;
// Specific element background and foregrounds
ButtonColors button_colors;
SkColor loading_indicator_foreground;
SkColor loading_indicator_background;
SkColor exit_warning_foreground;
......@@ -79,8 +72,6 @@ struct ColorScheme {
SkColor web_vr_transient_toast_background;
SkColor exclusive_screen_toast_foreground;
SkColor exclusive_screen_toast_background;
SkColor system_indicator_foreground;
SkColor system_indicator_background;
SkColor modal_prompt_icon_foreground;
SkColor modal_prompt_background;
SkColor modal_prompt_foreground;
......@@ -92,12 +83,16 @@ struct ColorScheme {
ButtonColors prompt_secondary_button_colors;
ButtonColors prompt_primary_button_colors;
ButtonColors url_bar_button;
SkColor url_bar_background;
SkColor url_bar_separator;
SkColor url_bar_text;
SkColor url_bar_hint_text;
SkColor url_bar_default_icon;
SkColor url_bar_dangerous_icon;
UrlBarColors url_bar;
ButtonColors url_bar_button;
UrlTextColors url_text;
SkColor omnibox_background;
TextSelectionColors omnibox_text_selection;
SkColor hyperlink;
ButtonColors indicator;
......@@ -113,17 +108,6 @@ struct ColorScheme {
SkColor speech_recognition_circle_background;
SkColor omnibox_background;
SkColor omnibox_icon;
SkColor omnibox_text;
SkColor omnibox_hint;
TextSelectionColors omnibox_text_selection;
SkColor suggestion_text;
SkColor suggestion_dim_text;
SkColor suggestion_url_text;
ButtonColors omnibox_voice_search_button_colors;
ButtonColors suggestion_button_colors;
SkColor snackbar_background;
SkColor snackbar_foreground;
ButtonColors snackbar_button_colors;
......
......@@ -84,9 +84,6 @@ VrTestContext::VrTestContext() : view_scale_factor_(kDefaultViewScaleFactor) {
base::i18n::InitializeICU();
// TODO(cjgrant): Remove this when the keyboard is enabled by default.
base::FeatureList::InitializeInstance("VrBrowserKeyboard", "");
text_input_delegate_ = std::make_unique<TextInputDelegate>();
keyboard_delegate_ = std::make_unique<TestKeyboardDelegate>();
......@@ -527,6 +524,11 @@ void VrTestContext::OnContentScreenBoundsChanged(const gfx::SizeF& bounds) {}
void VrTestContext::StartAutocomplete(const AutocompleteRequest& request) {
auto result = std::make_unique<OmniboxSuggestions>();
if (request.text.empty()) {
ui_->SetOmniboxSuggestions(std::move(result));
return;
}
// Supply an in-line match if the input matches a canned URL.
base::string16 full_string = base::UTF8ToUTF16("wikipedia.org");
if (!request.prevent_inline_autocomplete && request.text.size() >= 2 &&
......
......@@ -199,7 +199,7 @@ static constexpr float kOmniboxShadowIntensity = 0.4f;
static constexpr int kOmniboxTransitionMs = 300;
static constexpr float kOmniboxTextFieldIconButtonSizeDMM = 0.064f;
static constexpr float kOmniboxTextFieldIconButtonHoverOffsetDMM = 0.012f;
static constexpr float kUrlBarButtonHoverOffsetDMM = 0.012f;
static constexpr float kOmniboxTextFieldRightMargin =
((kOmniboxHeightDMM - kOmniboxTextFieldIconButtonSizeDMM) / 2);
......
This diff is collapsed.
......@@ -97,8 +97,9 @@ const std::set<UiElementName> kElementsVisibleWithVoiceSearchResult = {
kSpeechRecognitionResult, kSpeechRecognitionCircle,
kSpeechRecognitionMicrophoneIcon, kSpeechRecognitionResultBackplane};
static constexpr float kTolerance = 1e-5f;
static constexpr float kSmallDelaySeconds = 0.1f;
constexpr float kTolerance = 1e-5f;
constexpr float kSmallDelaySeconds = 0.1f;
constexpr float kAnimationTimeMs = 300;
MATCHER_P2(SizeFsAreApproximatelyEqual, other, tolerance, "") {
return base::IsApproximatelyEqual(arg.width(), other.width(), tolerance) &&
......@@ -1017,7 +1018,7 @@ TEST_F(UiTest, OmniboxSuggestionBindings) {
model_->push_mode(kModeEditingOmnibox);
OnBeginFrame();
EXPECT_EQ(container->children().size(), 0u);
EXPECT_EQ(NumVisibleInTree(kOmniboxSuggestions), 1);
EXPECT_EQ(NumVisibleInTree(kOmniboxSuggestions), 0);
model_->omnibox_suggestions.emplace_back(OmniboxSuggestion(
base::string16(), base::string16(), ACMatchClassifications(),
......@@ -1030,7 +1031,7 @@ TEST_F(UiTest, OmniboxSuggestionBindings) {
model_->omnibox_suggestions.clear();
OnBeginFrame();
EXPECT_EQ(container->children().size(), 0u);
EXPECT_EQ(NumVisibleInTree(kOmniboxSuggestions), 1);
EXPECT_EQ(NumVisibleInTree(kOmniboxSuggestions), 0);
}
TEST_F(UiTest, OmniboxSuggestionNavigates) {
......@@ -1094,25 +1095,25 @@ TEST_F(UiTest, CloseButtonColorBindings) {
ui_->SetFullscreen(true);
}
ColorScheme scheme = ColorScheme::GetColorScheme(mode);
EXPECT_TRUE(OnBeginFrame());
VerifyButtonColor(button, scheme.button_colors.foreground,
scheme.button_colors.background, "normal");
RunFor(MsToDelta(kAnimationTimeMs));
VerifyButtonColor(button, scheme.disc_button_colors.foreground,
scheme.disc_button_colors.background, "normal");
button->hit_plane()->OnHoverEnter(gfx::PointF(0.5f, 0.5f));
EXPECT_TRUE(OnBeginFrame());
VerifyButtonColor(button, scheme.button_colors.foreground,
scheme.button_colors.background_hover, "hover");
RunFor(MsToDelta(kAnimationTimeMs));
VerifyButtonColor(button, scheme.disc_button_colors.foreground,
scheme.disc_button_colors.background_hover, "hover");
button->hit_plane()->OnButtonDown(gfx::PointF(0.5f, 0.5f));
EXPECT_TRUE(OnBeginFrame());
VerifyButtonColor(button, scheme.button_colors.foreground,
scheme.button_colors.background_down, "down");
RunFor(MsToDelta(kAnimationTimeMs));
VerifyButtonColor(button, scheme.disc_button_colors.foreground,
scheme.disc_button_colors.background_down, "down");
button->hit_plane()->OnMove(gfx::PointF());
EXPECT_TRUE(OnBeginFrame());
VerifyButtonColor(button, scheme.button_colors.foreground,
scheme.button_colors.background, "move");
RunFor(MsToDelta(kAnimationTimeMs));
VerifyButtonColor(button, scheme.disc_button_colors.foreground,
scheme.disc_button_colors.background, "move");
button->hit_plane()->OnButtonUp(gfx::PointF());
EXPECT_TRUE(OnBeginFrame());
VerifyButtonColor(button, scheme.button_colors.foreground,
scheme.button_colors.background, "up");
RunFor(MsToDelta(kAnimationTimeMs));
VerifyButtonColor(button, scheme.disc_button_colors.foreground,
scheme.disc_button_colors.background, "up");
}
}
......
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