Cleanup Icon Row custom code.

chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java

Index: chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java
index ce4cd65badc3f0b64481b46a0613c53d93e3d47b..8ba3e3fa9a6adedb17591f45d6d7301cf5291658 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java
@@ -8,8 +8,11 @@ import android.graphics.drawable.Drawable;
 import android.view.LayoutInflater;
 import android.view.MenuItem;
 import android.view.View;
+import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.widget.BaseAdapter;
+import android.widget.Button;
+import android.widget.ImageButton;
 import android.widget.ListView;
 import android.widget.TextView;
 
@@ -21,14 +24,19 @@ import java.util.List;
  * ListAdapter to customize the view of items in the list.
  */
 class AppMenuAdapter extends BaseAdapter {
+    private static final int VIEW_TYPE_COUNT = 3;
+
     private static final int VIEW_TYPE_MENUITEM = 0;
-    private static final int VIEW_TYPE_COUNT = 1;
+    private static final int VIEW_TYPE_TITLE_BUTTON_MENUITEM = 1;
+    private static final int VIEW_TYPE_THREE_BUTTON_MENUITEM = 2;

[Kibeom Kim (inactive), 2014/03/19 22:04:46]

maybe comment what they represent?

[aurimas (slooooooooow), 2014/03/19 23:31:32]

Done.

 
+    private final AppMenu mAppMenu;
     private final LayoutInflater mInflater;
     private final List<MenuItem> mMenuItems;
     private final int mNumMenuItems;
 
-    public AppMenuAdapter(List<MenuItem> menuItems, LayoutInflater inflater) {
+    public AppMenuAdapter(AppMenu appMenu, List<MenuItem> menuItems, LayoutInflater inflater) {
+        mAppMenu = appMenu;
         mMenuItems = menuItems;
         mInflater = inflater;
         mNumMenuItems = menuItems.size();
@@ -46,6 +54,13 @@ class AppMenuAdapter extends BaseAdapter {
 
     @Override
     public int getItemViewType(int position) {
+        if (getItem(position).hasSubMenu()) {
+            if (getItem(position).getSubMenu().size() == 3) {
+                return VIEW_TYPE_THREE_BUTTON_MENUITEM;
+            } else if (getItem(position).getSubMenu().size() == 2) {
+                return VIEW_TYPE_TITLE_BUTTON_MENUITEM;
+            }
+        }
         return VIEW_TYPE_MENUITEM;
     }
 
@@ -64,37 +79,85 @@ class AppMenuAdapter extends BaseAdapter {
 
     @Override
     public View getView(int position, View convertView, ViewGroup parent) {
-        View rowView = convertView;
-        // A ViewHolder keeps references to children views to avoid unneccessary calls
-        // to findViewById() on each row.
-        ViewHolder holder = null;
-
-        // When convertView is not null, we can reuse it directly, there is no need
-        // to reinflate it.
-        if (rowView == null) {
-            holder = new ViewHolder();
-            rowView = mInflater.inflate(R.layout.menu_item, null);
-            holder.text = (TextView) rowView.findViewById(R.id.menu_item_text);
-            holder.image = (AppMenuItemIcon) rowView.findViewById(R.id.menu_item_icon);
-            rowView.setTag(holder);
+        final MenuItem item = getItem(position);
+        if (getItemViewType(position) == VIEW_TYPE_MENUITEM) {

[Kibeom Kim (inactive), 2014/03/19 22:04:46]

optinal: In this case, I think switch statement will be more readable.

[aurimas (slooooooooow), 2014/03/19 23:31:32]

Done.

+            // A ViewHolder keeps references to children views to avoid unneccessary calls
+            // to findViewById() on each row.
+            ViewHolder holder = null;
+
+            // When convertView is not null, we can reuse it directly, there is no need
+            // to reinflate it.
+            if (convertView == null) {
+                holder = new ViewHolder();
+                convertView = mInflater.inflate(R.layout.menu_item, null);
+                holder.text = (TextView) convertView.findViewById(R.id.menu_item_text);
+                holder.image = (AppMenuItemIcon) convertView.findViewById(R.id.menu_item_icon);
+                convertView.setTag(holder);
+            } else {
+                holder = (ViewHolder) convertView.getTag();
+            }
+            convertView.setOnClickListener(new OnClickListener() {
+                @Override
+                public void onClick(View v) {
+                    mAppMenu.onItemClick(item);
+                }
+            });
+            // Set up the icon.
+            Drawable icon = item.getIcon();
+            holder.image.setImageDrawable(icon);
+            holder.image.setVisibility(icon == null ? View.GONE : View.VISIBLE);
+            holder.image.setChecked(item.isChecked());
+
+            holder.text.setText(item.getTitle());
+            boolean isEnabled = item.isEnabled();
+            // Set the text color (using a color state list).
+            holder.text.setEnabled(isEnabled);
+            // This will ensure that the item is not highlighted when selected.
+            convertView.setEnabled(isEnabled);
+            return convertView;
+        } else if (getItemViewType(position) == VIEW_TYPE_THREE_BUTTON_MENUITEM) {

[Kibeom Kim (inactive), 2014/03/19 22:04:46]

probably not important but just Q: can't we reuse the convertView here and below
else statement?

[aurimas (slooooooooow), 2014/03/19 23:31:32]

Done.

+            convertView = mInflater.inflate(R.layout.three_button_menu_item, null);
+            setupImageButton((ImageButton) convertView.findViewById(R.id.button_one),
+                    item.getSubMenu().getItem(0));
+            setupImageButton((ImageButton) convertView.findViewById(R.id.button_two),
+                    item.getSubMenu().getItem(1));
+            setupImageButton((ImageButton) convertView.findViewById(R.id.button_three),
+                    item.getSubMenu().getItem(2));
+            convertView.setFocusable(false);
+            convertView.setEnabled(false);
+            return convertView;
         } else {

[Kibeom Kim (inactive), 2014/03/19 22:04:46]

I'd either use switch statement or put "assert getItemViewType(position) ==
VIEW_TYPE_TITLE_BUTTON_MENUITE", for readability.

[aurimas (slooooooooow), 2014/03/19 23:31:32]

Done.

-            holder = (ViewHolder) convertView.getTag();
+            convertView = mInflater.inflate(R.layout.title_button_menu_item, null);
+            Button button = (Button) convertView.findViewById(R.id.menu_item_text);
+            final MenuItem textItem = item.getSubMenu().getItem(0);
+            button.setText(textItem.getTitle());
+            button.setEnabled(textItem.isEnabled());
+            button.setFocusable(textItem.isEnabled());
+            button.setOnClickListener(new OnClickListener() {
+                @Override
+                public void onClick(View v) {
+                    mAppMenu.onItemClick(textItem);
+                }
+            });
+            setupImageButton((ImageButton) convertView.findViewById(R.id.button),
+                    item.getSubMenu().getItem(1));
+            convertView.setFocusable(false);
+            convertView.setEnabled(false);
+            return convertView;
         }
-        MenuItem item = getItem(position);
-
-        // Set up the icon.
-        Drawable icon = item.getIcon();
-        holder.image.setImageDrawable(icon);
-        holder.image.setVisibility(icon == null ? View.GONE : View.VISIBLE);
-        holder.image.setChecked(item.isChecked());
-
-        holder.text.setText(item.getTitle());
-        boolean isEnabled = item.isEnabled();
-        // Set the text color (using a color state list).
-        holder.text.setEnabled(isEnabled);
-        // This will ensure that the item is not highlighted when selected.
-        rowView.setEnabled(isEnabled);
-        return rowView;
+    }
+
+    private void setupImageButton(ImageButton button, final MenuItem item) {
+        button.setImageDrawable(item.getIcon());
+        button.setContentDescription(item.getTitle());
+        button.setEnabled(item.isEnabled());
+        button.setFocusable(item.isEnabled());
+        button.setOnClickListener(new OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                mAppMenu.onItemClick(item);
+            }
+        });
     }
 
     static class ViewHolder {