[Remoting Android] Navigation Menu Cleanups

remoting/android/java/src/org/chromium/chromoting/NavigationMenuAdapter.java

Index: remoting/android/java/src/org/chromium/chromoting/NavigationMenuAdapter.java
diff --git a/remoting/android/java/src/org/chromium/chromoting/NavigationMenuAdapter.java b/remoting/android/java/src/org/chromium/chromoting/NavigationMenuAdapter.java
index db07d2df267bae26b1d2ed6463a395eff4ddd166..cd266352d34f0748a2258b8abfa2e2cdc1180d0a 100644
--- a/remoting/android/java/src/org/chromium/chromoting/NavigationMenuAdapter.java
+++ b/remoting/android/java/src/org/chromium/chromoting/NavigationMenuAdapter.java
@@ -6,15 +6,16 @@ package org.chromium.chromoting;
 
 import android.app.Activity;
 import android.content.Context;
-import android.content.Intent;
+import android.graphics.drawable.Drawable;
+import android.support.v4.content.ContextCompat;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.AdapterView;
 import android.widget.ArrayAdapter;
 import android.widget.ListView;
+import android.widget.TextView;
 
-import org.chromium.chromoting.help.CreditsActivity;
 import org.chromium.chromoting.help.FeedbackSender;
 import org.chromium.chromoting.help.HelpContext;
 import org.chromium.chromoting.help.HelpSingleton;
@@ -29,10 +30,13 @@ public class NavigationMenuAdapter extends ArrayAdapter<NavigationMenuAdapter.Na
      * Defines a menu item.
      */
     public static class NavigationMenuItem {
-        private int mLayoutResourceId;
+        private String mText;
+        private Drawable mIcon;
         private Runnable mCallback;
-        public NavigationMenuItem(int layoutResourceId, Runnable callback) {
-            mLayoutResourceId = layoutResourceId;
+
+        public NavigationMenuItem(String text, Drawable icon, Runnable callback) {
+            mText = text;
+            mIcon = icon;
             mCallback = callback;
         }
     }
@@ -41,15 +45,9 @@ public class NavigationMenuAdapter extends ArrayAdapter<NavigationMenuAdapter.Na
         ListView navigationMenu = (ListView) activity.getLayoutInflater()
                 .inflate(R.layout.navigation_list, null);
 
-        NavigationMenuItem creditsItem = new NavigationMenuItem(R.menu.credits_list_item,
-                new Runnable() {
-                    @Override
-                    public void run() {
-                        activity.startActivity(new Intent(activity, CreditsActivity.class));
-                    }
-                });
-
-        NavigationMenuItem feedbackItem = new NavigationMenuItem(R.menu.feedback_list_item,
+        NavigationMenuItem feedbackItem = new NavigationMenuItem(
+                activity.getResources().getString(R.string.actionbar_send_feedback),
+                getIcon(activity, R.drawable.ic_announcement),
                 new Runnable() {
                     @Override
                     public void run() {
@@ -57,7 +55,9 @@ public class NavigationMenuAdapter extends ArrayAdapter<NavigationMenuAdapter.Na
                     }
                 });
 
-        NavigationMenuItem helpItem = new NavigationMenuItem(R.menu.help_list_item,
+        NavigationMenuItem helpItem = new NavigationMenuItem(
+                activity.getResources().getString(R.string.actionbar_help),
+                getIcon(activity, R.drawable.ic_help),
                 new Runnable() {
                     @Override
                     public void run() {
@@ -66,14 +66,21 @@ public class NavigationMenuAdapter extends ArrayAdapter<NavigationMenuAdapter.Na
                     }
                 });
 
-        NavigationMenuItem[] navigationMenuItems = { creditsItem, feedbackItem, helpItem };
+        NavigationMenuItem[] navigationMenuItems = { feedbackItem, helpItem };
         NavigationMenuAdapter adapter = new NavigationMenuAdapter(activity, navigationMenuItems);
         navigationMenu.setAdapter(adapter);
         navigationMenu.setOnItemClickListener(adapter);
         return navigationMenu;
     }
 
-    public NavigationMenuAdapter(Context context, NavigationMenuItem[] objects) {
+    /** Returns the drawable of |drawableId| that can be used to draw icon for a navigation item. */
+    private static Drawable getIcon(Activity activity, int drawableId) {
+        Drawable drawable = ContextCompat.getDrawable(activity, drawableId);

[Lambros, 2016/10/03 21:10:03]

ContextCompat is fine, I think. If you run into any build issues with it, you
can use instead:
org.chromium.base.ApiCompatibilityUtils.getDrawable()

I'm not sure why Chrome has this, given that ContextCompat already does the same
job?

[Yuwei, 2016/10/03 22:06:21]

No idea... Done anyway. Maybe it's API level independent?

+        drawable.setBounds(0, 0, drawable.getIntrinsicWidth(), drawable.getIntrinsicHeight());
+        return drawable;
+    }
+
+    private NavigationMenuAdapter(Context context, NavigationMenuItem[] objects) {
         super(context, -1, objects);
     }
 
@@ -84,7 +91,10 @@ public class NavigationMenuAdapter extends ArrayAdapter<NavigationMenuAdapter.Na
             NavigationMenuItem item = getItem(position);
             LayoutInflater inflater =
                     (LayoutInflater) getContext().getSystemService(Context.LAYOUT_INFLATER_SERVICE);
-            convertView = inflater.inflate(item.mLayoutResourceId, null);
+            TextView textView = (TextView) inflater.inflate(R.menu.navigation_menu_item, null);

[Lambros, 2016/10/03 21:10:03]

s/null/parent/ ?
I think we are meant to use the ViewGroup parent for layout inflation?
See https://possiblemobile.com/2013/05/layout-inflation-as-intended/

[Yuwei, 2016/10/03 22:06:21]

Done.

+            textView.setCompoundDrawables(item.mIcon, null, null, null);

[Lambros, 2016/10/03 21:10:03]

I don't know if this is legitimate?
I think this line, and the next line, need to be after the 'if' block? I think
Views can be recycled between items.
So a non-null |convertView| might originate from a different navigation item, so
you need to replace the icon and text that might be previously set to different
values?

[Yuwei, 2016/10/03 22:06:21]

Done. Didn't know that Android will reuse views for different item. Weird design
O_o

+            textView.setText(item.mText);
+            convertView = textView;
         }
         return convertView;
     }