Adjust appearance of omnibox keyword hint view for on-screen keyboards.

chrome/browser/ui/views/location_bar/keyword_hint_view.cc

Index: chrome/browser/ui/views/location_bar/keyword_hint_view.cc
diff --git a/chrome/browser/ui/views/location_bar/keyword_hint_view.cc b/chrome/browser/ui/views/location_bar/keyword_hint_view.cc
index 3ec9238e726e19a7d5bbed14a86a8f79f355b3fe..a1b73ec23b8d75784231188f0852948e6c1a3b70 100644
--- a/chrome/browser/ui/views/location_bar/keyword_hint_view.cc
+++ b/chrome/browser/ui/views/location_bar/keyword_hint_view.cc
@@ -17,34 +17,35 @@
 #include "components/search_engines/template_url_service.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/color_utils.h"
+#include "ui/keyboard/keyboard_util.h"
 #include "ui/strings/grit/ui_strings.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/label.h"
 
 namespace {
 
-// This view sort of looks like a tab key.
-class TabKeyBubbleView : public views::Label {
+// This view kinda looks like a keyboard key.

[Peter Kasting, 2017/04/07 18:14:15]

Nit: Less colloquial: "looks somewhat like"

[Evan Stade, 2017/04/08 00:07:22]

Done.

+class LooksPressableView : public views::Label {

[Peter Kasting, 2017/04/07 18:14:15]

Nit: KeyboardKeyView?  KeyLikeView?  KeyChipView?

The only reason I don't like "looks pressable" is because we already have
buttons and things that look pressable, but this is distinct from those.

[Evan Stade, 2017/04/08 00:07:22]

buttons actually ARE pressable though. Changed to KeyboardKeyView

  public:
-  explicit TabKeyBubbleView(const gfx::FontList& font_list);
-  ~TabKeyBubbleView() override;
+  explicit LooksPressableView(const gfx::FontList& font_list);
+  ~LooksPressableView() override;
 
  private:
   // views::Label:
   gfx::Size GetPreferredSize() const override;
 
-  DISALLOW_COPY_AND_ASSIGN(TabKeyBubbleView);
+  DISALLOW_COPY_AND_ASSIGN(LooksPressableView);
 };
 
-TabKeyBubbleView::TabKeyBubbleView(const gfx::FontList& font_list)
-    : views::Label(l10n_util::GetStringUTF16(IDS_APP_TAB_KEY), {font_list}) {
+LooksPressableView::LooksPressableView(const gfx::FontList& font_list)
+    : views::Label(base::string16(), {font_list}) {
   SetBorder(views::CreateEmptyBorder(
       gfx::Insets(LocationBarView::kBubbleVerticalPadding, 0)));
 }
 
-TabKeyBubbleView::~TabKeyBubbleView() {}
+LooksPressableView::~LooksPressableView() {}
 
-gfx::Size TabKeyBubbleView::GetPreferredSize() const {
+gfx::Size LooksPressableView::GetPreferredSize() const {
   gfx::Size size = views::Label::GetPreferredSize();
   constexpr int kPaddingInsideBorder = 5;
   // Even though the border is 1 px thick visibly, it takes 1 DIP logically.
@@ -58,34 +59,31 @@ KeywordHintView::KeywordHintView(views::ButtonListener* listener,
                                  Profile* profile,
                                  const gfx::FontList& font_list,
                                  const gfx::FontList& bubble_font_list,
-                                 int bubble_height,
+                                 int chip_height,
                                  SkColor text_color,
                                  SkColor background_color)
     : CustomButton(listener),
       profile_(profile),
       leading_label_(nullptr),
-      tab_key_view_(nullptr),
+      chip_view_(new LooksPressableView(bubble_font_list)),
       trailing_label_(nullptr),
-      tab_key_height_(bubble_height) {
+      chip_view_height_(chip_height) {
   leading_label_ =
       CreateLabel(font_list, text_color, background_color);
-  TabKeyBubbleView* tab_key = new TabKeyBubbleView(bubble_font_list);
-  tab_key->SetEnabledColor(text_color);
+  chip_view_->SetEnabledColor(text_color);
   bool inverted = color_utils::IsDark(background_color);
   SkColor tab_bg_color =
       inverted ? SK_ColorWHITE : SkColorSetA(text_color, 0x13);
   SkColor tab_border_color = inverted ? SK_ColorWHITE : text_color;
-  tab_key->SetBackgroundColor(tab_bg_color);
-  tab_key->set_background(
+  chip_view_->SetBackgroundColor(tab_bg_color);
+  chip_view_->set_background(
       new BackgroundWith1PxBorder(tab_bg_color, tab_border_color));
-  tab_key_view_ = tab_key;
-  AddChildView(tab_key_view_);
+  AddChildView(chip_view_);
   trailing_label_ =
       CreateLabel(font_list, text_color, background_color);
 }
 
-KeywordHintView::~KeywordHintView() {
-}
+KeywordHintView::~KeywordHintView() {}
 
 void KeywordHintView::SetKeyword(const base::string16& keyword) {
   keyword_ = keyword;
@@ -97,24 +95,38 @@ void KeywordHintView::SetKeyword(const base::string16& keyword) {
   if (!url_service)
     return;
 
-  std::vector<size_t> content_param_offsets;
   bool is_extension_keyword;
   base::string16 short_name(
       url_service->GetKeywordShortName(keyword, &is_extension_keyword));
-  int message_id = is_extension_keyword ?
-      IDS_OMNIBOX_EXTENSION_KEYWORD_HINT : IDS_OMNIBOX_KEYWORD_HINT;
-  const base::string16 keyword_hint = l10n_util::GetStringFUTF16(
-      message_id, base::string16(), short_name, &content_param_offsets);
-  DCHECK_EQ(2U, content_param_offsets.size());
-  leading_label_->SetText(
-      keyword_hint.substr(0, content_param_offsets.front()));
-  trailing_label_->SetText(keyword_hint.substr(content_param_offsets.front()));
+
+  if (keyboard::IsKeyboardVisible()) {
+    int message_id = is_extension_keyword
+                         ? IDS_OMNIBOX_EXTENSION_KEYWORD_HINT_TOUCH
+                         : IDS_OMNIBOX_KEYWORD_HINT_TOUCH;
+    chip_view_->SetText(l10n_util::GetStringFUTF16(message_id, short_name));
+
+    leading_label_->SetText(base::string16());

[Peter Kasting, 2017/04/07 18:14:15]

I kinda wonder if in this case we should just not have these children at all,
but I think it would end up making the code more complicated.

[Evan Stade, 2017/04/08 00:07:22]

the other option being to set them invisible, but an empty label seems to be
roughly the same. So yea, I too believe this to be simplest.

+    trailing_label_->SetText(base::string16());
+  } else {
+    chip_view_->SetText(l10n_util::GetStringUTF16(IDS_APP_TAB_KEY));
+
+    std::vector<size_t> content_param_offsets;
+    int message_id = is_extension_keyword ? IDS_OMNIBOX_EXTENSION_KEYWORD_HINT
+                                          : IDS_OMNIBOX_KEYWORD_HINT;
+    const base::string16 keyword_hint = l10n_util::GetStringFUTF16(
+        message_id, base::string16(), short_name, &content_param_offsets);
+    DCHECK_EQ(2U, content_param_offsets.size());
+    leading_label_->SetText(
+        keyword_hint.substr(0, content_param_offsets.front()));
+    trailing_label_->SetText(
+        keyword_hint.substr(content_param_offsets.front()));
+  }
 }
 
 gfx::Size KeywordHintView::GetPreferredSize() const {
   // Height will be ignored by the LocationBarView.
   return gfx::Size(leading_label_->GetPreferredSize().width() +
-                       tab_key_view_->GetPreferredSize().width() +
+                       chip_view_->GetPreferredSize().width() +
                        trailing_label_->GetPreferredSize().width() +
                        LocationBarView::kIconInteriorPadding,
                    0);
@@ -122,20 +134,20 @@ gfx::Size KeywordHintView::GetPreferredSize() const {
 
 gfx::Size KeywordHintView::GetMinimumSize() const {
   // Height will be ignored by the LocationBarView.
-  return tab_key_view_->GetPreferredSize();
+  return chip_view_->GetPreferredSize();
 }
 
 void KeywordHintView::Layout() {
-  int tab_width = tab_key_view_->GetPreferredSize().width();
-  bool show_labels = (width() != tab_width);
+  int pressable_width = chip_view_->GetPreferredSize().width();

[Peter Kasting, 2017/04/07 18:14:15]

Nit: |chip_width|?

[Evan Stade, 2017/04/08 00:07:22]

sorry, did a bunch of renames and missed this one in the end. fixed.

+  bool show_labels = (width() != pressable_width);
   gfx::Size leading_size(leading_label_->GetPreferredSize());
   leading_label_->SetBounds(0, 0, show_labels ? leading_size.width() : 0,
                             height());
-  tab_key_view_->SetBounds(leading_label_->bounds().right(),
-                           (height() - tab_key_height_) / 2, tab_width,
-                           tab_key_height_);
+  chip_view_->SetBounds(leading_label_->bounds().right(),
+                        (height() - chip_view_height_) / 2, pressable_width,
+                        chip_view_height_);
   gfx::Size trailing_size(trailing_label_->GetPreferredSize());
-  trailing_label_->SetBounds(tab_key_view_->bounds().right(), 0,
+  trailing_label_->SetBounds(chip_view_->bounds().right(), 0,
                              show_labels ? trailing_size.width() : 0, height());
 }