Gray horizontal keyboard accessory

ui/android/java/src/org/chromium/ui/autofill/AutofillKeyboardAccessory.java

Index: ui/android/java/src/org/chromium/ui/autofill/AutofillKeyboardAccessory.java
diff --git a/ui/android/java/src/org/chromium/ui/autofill/AutofillKeyboardAccessory.java b/ui/android/java/src/org/chromium/ui/autofill/AutofillKeyboardAccessory.java
index 1c13938eba305a00eecbdfe18f691c03cd3f3bf8..24d9d7c93657624bdd30f95642b7e947ab852315 100644
--- a/ui/android/java/src/org/chromium/ui/autofill/AutofillKeyboardAccessory.java
+++ b/ui/android/java/src/org/chromium/ui/autofill/AutofillKeyboardAccessory.java
@@ -5,26 +5,28 @@
 package org.chromium.ui.autofill;
 
 import android.annotation.SuppressLint;
+import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.accessibility.AccessibilityEvent;
-import android.widget.AdapterView;
-import android.widget.FrameLayout;
-import android.widget.ListAdapter;
-import android.widget.ListView;
+import android.widget.LinearLayout;
+import android.widget.TextView;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.ui.R;
 import org.chromium.ui.base.WindowAndroid;
+import org.chromium.ui.gfx.DeviceDisplayInfo;
 
 /**
  * The Autofill suggestion view that lists relevant suggestions. It sits above the keyboard and
  * below the content area.
  */
-public class AutofillKeyboardAccessory extends ListView implements AdapterView.OnItemClickListener,
-        WindowAndroid.KeyboardVisibilityListener {
+public class AutofillKeyboardAccessory extends LinearLayout
+        implements WindowAndroid.KeyboardVisibilityListener, View.OnClickListener {
     private final WindowAndroid mWindowAndroid;
     private final AutofillKeyboardAccessoryDelegate mAutofillCallback;
+    private final int mMaximumNameWidthPx;
+    private final int mMaximumLabelWidthPx;
 
     /**
      * An interface to handle the touch interaction with an AutofillKeyboardAccessory object.
@@ -56,12 +58,16 @@ public class AutofillKeyboardAccessory extends ListView implements AdapterView.O
         mWindowAndroid = windowAndroid;
         mAutofillCallback = autofillCallback;
 
+        int deviceWidthPx = DeviceDisplayInfo.create(getContext()).getDisplayWidth();

[newt (away), 2015/08/03 21:31:24]

This logic is likely to break in multiwindow mode, and probably won't produce
great results on tablets.

[please use gerrit instead, 2015/08/05 00:28:08]

What do you recommend? From what I understand, UX wants to play around with
implementation of their specs before making any more changes. This is behind a
flag, so it won't break for everyone.

[newt (away), 2015/08/05 20:41:40]

It's fine to land this as is, but I'd also suggest seeing how it looks with a
min and max width for these elements, or just a fixed width.

+        mMaximumNameWidthPx = deviceWidthPx / 2;
+        mMaximumLabelWidthPx = deviceWidthPx / 4;
+
         mWindowAndroid.addKeyboardVisibilityListener(this);
-        setOnItemClickListener(this);
-        setContentDescription(getContext().getString(
-                R.string.autofill_popup_content_description));
-        setBackgroundColor(getResources().getColor(
-                R.color.keyboard_accessory_suggestion_background_color));
+        setContentDescription(getContext().getString(R.string.autofill_popup_content_description));
+
+        int horizontalPaddingPx = getResources().getDimensionPixelSize(
+                R.dimen.keyboard_accessory_half_padding);
+        setPadding(horizontalPaddingPx, 0, horizontalPaddingPx, 0);
     }
 
     /**
@@ -71,29 +77,44 @@ public class AutofillKeyboardAccessory extends ListView implements AdapterView.O
      */
     @SuppressLint("InlinedApi")
     public void showWithSuggestions(AutofillSuggestion[] suggestions, boolean isRtl) {

[newt (away), 2015/08/03 21:31:24]

Time to delete SuggestionAdapter.java and autofill_suggestion_item.xml?

[please use gerrit instead, 2015/08/05 00:28:08]

Good point. I missed that they were used only here. Done.

-        setAdapter(new SuggestionAdapter(getContext(), suggestions));
-        ApiCompatibilityUtils.setLayoutDirection(
-                this, isRtl ? View.LAYOUT_DIRECTION_RTL : View.LAYOUT_DIRECTION_LTR);
+        removeAllViews();
+        for (int i = 0; i < suggestions.length; ++i) {

[newt (away), 2015/08/03 21:31:24]

for (AutofillSuggestion suggestion : suggestions)

It produces equivalent code (or possibly slightly better) and is simpler.

[please use gerrit instead, 2015/08/05 00:28:08]

Done.

+            View touchTarget = LayoutInflater.from(getContext()).inflate(
+                    R.layout.autofill_keyboard_accessory_item, this, false);
+            touchTarget.setOnClickListener(this);
+            TextView name = (TextView) touchTarget.findViewById(

[newt (away), 2015/08/03 21:31:24]

"name" or "label" or "sublabel" or "value"? Let's pick something and be
consistent. The code down below reads:

    name.setText(...getLabel());

    label.setText(...getSublabel());

Talk about trying to confuse your readers ;)

[please use gerrit instead, 2015/08/05 00:28:08]

Done.

+                    R.id.autofill_keyboard_accessory_item_name);
+            name.setMaxWidth(mMaximumNameWidthPx);
+
+            if (suggestions[i].getIconId() != 0) {
+                ApiCompatibilityUtils.setCompoundDrawablesRelativeWithIntrinsicBounds(
+                        name, suggestions[i].getIconId(), 0, 0, 0);
+            }
+
+            if (suggestions[i].getLabel() != null && !suggestions[i].getLabel().isEmpty()) {

[newt (away), 2015/08/03 21:31:24]

if (!TextUtils.isEmpty(suggestions[i].getLabel()))

But... why would the name/label ever be empty? If so, why are we showing an
empty view to the user?

[please use gerrit instead, 2015/08/05 00:28:08]

An empty name can be used to show an icon-only item. Currently "autofill
settings" and "scan credit card" are planned to be icon-only. Until recently,
"CLEAR FORM" was going to be an icon-only item as well, but that has recently
changed.

An empty label means that other fields in the form will not be filled. This
happens when the user has saved information for only one field in a form.

+                name.setText(suggestions[i].getLabel());
 
-        int height = ViewGroup.LayoutParams.WRAP_CONTENT;
-        // Limit the visible number of suggestions (others are accessible via scrolling).
-        final int suggestionLimit = 2;
-        ListAdapter listAdapter = getAdapter();
-        if (listAdapter.getCount() > suggestionLimit) {
-            height = 0;
-            for (int i = 0; i < suggestionLimit; i++) {
-                View listItem = listAdapter.getView(i, null, this);
-                height += listItem.getLayoutParams().height;
+                if (suggestions[i].getSublabel() != null

[newt (away), 2015/08/03 21:31:25]

TextUtils.isEmpty()

[please use gerrit instead, 2015/08/05 00:28:08]

Done.

+                        && !suggestions[i].getSublabel().isEmpty()) {
+                    TextView label = (TextView) touchTarget.findViewById(
+                            R.id.autofill_keyboard_accessory_item_label);
+                    label.setText(suggestions[i].getSublabel());
+                    label.setVisibility(View.VISIBLE);
+                    label.setMaxWidth(mMaximumLabelWidthPx);
+                }
             }
+
+            addView(touchTarget);
         }
 
-        setLayoutParams(new FrameLayout.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT,
-                height));
+        ApiCompatibilityUtils.setLayoutDirection(
+                this, isRtl ? View.LAYOUT_DIRECTION_RTL : View.LAYOUT_DIRECTION_LTR);
 
         if (getParent() == null) {
             ViewGroup container = mWindowAndroid.getKeyboardAccessoryView();
             container.addView(this);
             container.setVisibility(View.VISIBLE);
+            container.setHorizontalScrollBarEnabled(false);

[newt (away), 2015/08/03 21:31:24]

Since this doesn't change dynamically, I'd just set android:scrollbars="none" in
main.xml.

[please use gerrit instead, 2015/08/05 00:28:08]

Done.

             sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED);
         }
     }
@@ -110,15 +131,23 @@ public class AutofillKeyboardAccessory extends ListView implements AdapterView.O
     }
 
     @Override
-    public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
-        mAutofillCallback.suggestionSelected(position);
-    }
-
-    @Override
     public void keyboardVisibilityChanged(boolean isShowing) {
         if (!isShowing) {
             dismiss();
             mAutofillCallback.dismissed();
         }
     }
+
+    @Override
+    public void onClick(View v) {
+        int count = getChildCount();
+        for (int i = 0; i < count; i++) {
+            if (getChildAt(i) == v) {
+                mAutofillCallback.suggestionSelected(i);
+                return;
+            }
+        }
+
+        assert false;
+    }
 }