Http Bad: Put icon on the left of warning message and make value and label in one line on Android

ui/android/java/src/org/chromium/ui/DropdownAdapter.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.ui;
 
 import android.content.Context;
 import android.graphics.Color;
 import android.graphics.Typeface;
 import android.text.TextUtils;
@@ skipping 34 matching lines @@
             DropdownItem item = getItem(i);
             if (item.isEnabled() && !item.isGroupHeader()) {
                 return false;
             }
         }
         return true;
     }
 
     @Override
     public View getView(int position, View convertView, ViewGroup parent) {
+        DropdownItem item = getItem(position);
+
         View layout = convertView;
         if (convertView == null) {
             LayoutInflater inflater =
                     (LayoutInflater) mContext.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
-            layout = inflater.inflate(R.layout.dropdown_item, null);
+            if (item.isIconOnLeft()) {
+                layout = inflater.inflate(R.layout.dropdown_item_left_icon, null);

[Ted C, 2016/11/28 18:29:11]

You can't actually use different layouts.  getView attempts to reuse views via
the passed in convertView.  By using different layouts, we are not going to be
able to distinguish between the two types and then are going to need to create a
new view each time, which is not a good thing.

If you do want to use different layouts (although I don't recommend it in this
case), then you'd need to overwrite getItemViewType and getViewTypeCount on the
adapter to inform them of these different types and have it reuse them
accordingly.

By using the same layout (with potentially two imageviews), you'll need to be
careful to always reset any previous state.

i.e. something like this:

ImageView iconViewStart = (ImageView) layout.findViewById(
        R.id.dropdown_icon_start);
ImageView iconViewEnd = (ImageView) layout.findViewById(
        R.id.dropdown_icon_end);
if (isIconAtStart()) {
    iconViewEnd.setVisibility(GONE);
} else {
    iconViewStart.setVisibility(GONE);
}

ImageView iconView = isIconAtStart() ? iconViewStart : iconViewEnd;

... existing image behavior ...

[lshang, 2016/11/30 10:35:43]

Done.

+            } else {
+                layout = inflater.inflate(R.layout.dropdown_item, null);
+            }
             layout.setBackground(new DropdownDividerDrawable());
         }
 
         DropdownDividerDrawable divider = (DropdownDividerDrawable) layout.getBackground();
         int height = mContext.getResources().getDimensionPixelSize(R.dimen.dropdown_item_height);
         if (position == 0) {
             divider.setColor(Color.TRANSPARENT);
         } else {
             int dividerHeight = mContext.getResources().getDimensionPixelSize(
                     R.dimen.dropdown_item_divider_height);
             height += dividerHeight;
             divider.setHeight(dividerHeight);
             if (mSeparators != null && mSeparators.contains(position)) {
                 divider.setColor(ApiCompatibilityUtils.getColor(mContext.getResources(),
                         R.color.dropdown_dark_divider_color));
             } else {
                 divider.setColor(ApiCompatibilityUtils.getColor(mContext.getResources(),
                         R.color.dropdown_divider_color));
             }
         }
 
-        DropdownItem item = getItem(position);
-
         // Note: trying to set the height of the root LinearLayout breaks accessibility,
         // so we have to adjust the height of this LinearLayout that wraps the TextViews instead.
         // If you need to modify this layout, don't forget to test it with TalkBack and make sure
         // it doesn't regress.
         // http://crbug.com/429364
-        View wrapper = layout.findViewById(R.id.dropdown_label_wrapper);
+        LinearLayout wrapper = (LinearLayout) layout.findViewById(R.id.dropdown_label_wrapper);
         if (item.isMultilineLabel()) height = LayoutParams.WRAP_CONTENT;
+        wrapper.setOrientation(item.getLabelSublabelOrientation());
         wrapper.setLayoutParams(new LinearLayout.LayoutParams(0, height, 1));
 
         TextView labelView = (TextView) layout.findViewById(R.id.dropdown_label);
         labelView.setText(item.getLabel());
         labelView.setSingleLine(!item.isMultilineLabel());

[Ted C, 2016/11/28 18:29:12]

Do the labels align correctly per the mocks with this change? 

In the mocks, it looks like it should be
+----------------------------------+
| Label ...... ws ....... SubLabel |
+----------------------------------+

But looking at this, it seems like it would be:
+----------------------------------+
| Label  SubLabel ....... ws ..... |
+----------------------------------+

Because dropdown_label's width is wrap_content, it doesn't seem like it would
do what you want.

I think in this case, you'd want to set the layout params (based on the is all
on one line flag) to be:

if (all on one line) {
     labelView.setLayoutParams(new LinearLayout.LayoutParams(
             0, LayoutParams.WRAP_CONTENT, 1));
} else {
     labelView.setLayoutParams(new LinearLayout.LayoutParams(
             LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT, 0));
}

I "believe" that should get it to what you want.

[lshang, 2016/11/30 10:35:43]

You're amazing! Yeah I tried fixing it by gravity and it didn't work.
Layout_weight is interesting. Now this is in line with the mocks. And I took out
the else part to keep the padding before label in normal entry.

 
         labelView.setEnabled(item.isEnabled());
         if (item.isGroupHeader()) {
             labelView.setTypeface(null, Typeface.BOLD);
         } else {
             labelView.setTypeface(null, Typeface.NORMAL);
         }
 
         labelView.setTextColor(ApiCompatibilityUtils.getColor(
                 mContext.getResources(), item.getLabelFontColorResId()));
@@ skipping 25 matching lines @@
         return mAreAllItemsEnabled;
     }
 
     @Override
     public boolean isEnabled(int position) {
         if (position < 0 || position >= getCount()) return false;
         DropdownItem item = getItem(position);
         return item.isEnabled() && !item.isGroupHeader();
     }
 }