Bug 1184211 - Replace TwoWayView in SearchBar with RecyclerView. r=sebastian
authorJonathan Almeida [:jonalmeida] <jalmeida@mozilla.com>
Wed, 22 Jul 2015 21:06:33 -0700
changeset 256056 f52bfe71d212
parent 256055 c67439c13ec0
child 256118 cf4a7df98beb
push id14400
push userryanvm@gmail.com
push date2015-08-04 18:46 +0000
treeherderfx-team@f52bfe71d212 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1184211, 1158291
milestone42.0a1
Bug 1184211 - Replace TwoWayView in SearchBar with RecyclerView. r=sebastian - Also includes changes from bug 1158291 applied on top of the re-written view. CLOSED TREE
mobile/android/base/home/BrowserSearch.java
mobile/android/base/home/SearchEngineAdapter.java
mobile/android/base/home/SearchEngineBar.java
mobile/android/base/moz.build
mobile/android/base/resources/layout/search_engine_bar_item.xml
mobile/android/base/resources/layout/search_engine_bar_label.xml
mobile/android/tests/browser/robocop/AboutHomeTest.java
mobile/android/tests/browser/robocop/testAddSearchEngine.java
--- a/mobile/android/base/home/BrowserSearch.java
+++ b/mobile/android/base/home/BrowserSearch.java
@@ -46,17 +46,16 @@ import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.MotionEvent;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.view.ViewStub;
-import android.view.WindowManager;
 import android.view.WindowManager.LayoutParams;
 import android.view.animation.AccelerateInterpolator;
 import android.view.animation.Animation;
 import android.view.animation.TranslateAnimation;
 import android.widget.AdapterView;
 import android.widget.LinearLayout;
 import android.widget.ListView;
 import android.widget.TextView;
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/home/SearchEngineAdapter.java
@@ -0,0 +1,122 @@
+package org.mozilla.gecko.home;
+
+import android.content.Context;
+import android.graphics.drawable.Drawable;
+import android.support.v4.content.ContextCompat;
+import android.support.v4.graphics.drawable.DrawableCompat;
+import android.support.v7.widget.RecyclerView;
+import android.view.LayoutInflater;
+import android.view.View;
+import android.view.ViewGroup;
+import android.widget.ImageView;
+
+import org.mozilla.gecko.R;
+
+import java.util.Collections;
+import java.util.List;
+
+public class SearchEngineAdapter
+        extends RecyclerView.Adapter<SearchEngineAdapter.SearchEngineViewHolder> {
+
+    private static final String LOGTAG = SearchEngineAdapter.class.getSimpleName();
+
+    private static final int VIEW_TYPE_SEARCH_ENGINE = 0;
+    private static final int VIEW_TYPE_LABEL = 1;
+    private final Context mContext;
+
+    private int mContainerWidth;
+    private List<SearchEngine> mSearchEngines = Collections.emptyList();
+
+    public void setSearchEngines(List<SearchEngine> searchEngines) {
+        mSearchEngines = searchEngines;
+        notifyDataSetChanged();
+    }
+
+    /**
+     * The container width is used for setting the appropriate calculated amount of width that
+     * a search engine icon can have. This varies depending on the space available in the
+     * {@link SearchEngineBar}. The setter exists for this attribute, in creating the view in the
+     * adapter after said calculation is done when the search bar is created.
+     * @param iconContainerWidth Width of each search icon.
+     */
+    void setIconContainerWidth(int iconContainerWidth) {
+        mContainerWidth = iconContainerWidth;
+    }
+
+    public static class SearchEngineViewHolder extends RecyclerView.ViewHolder {
+        final private ImageView faviconView;
+
+        public void bindItem(SearchEngine searchEngine) {
+            faviconView.setImageBitmap(searchEngine.getIcon());
+            final String desc = itemView.getResources().getString(R.string.search_bar_item_desc,
+                searchEngine.getEngineIdentifier());
+            itemView.setContentDescription(desc);
+        }
+
+        public SearchEngineViewHolder(View itemView) {
+            super(itemView);
+            faviconView = (ImageView) itemView.findViewById(R.id.search_engine_icon);
+        }
+    }
+
+    public SearchEngineAdapter(Context context) {
+        mContext = context;
+    }
+
+    @Override
+    public int getItemViewType(int position) {
+        return position == 0 ? VIEW_TYPE_LABEL : VIEW_TYPE_SEARCH_ENGINE;
+    }
+
+    public SearchEngine getItem(int position) {
+        // We omit the first position which is where the label currently is.
+        return position == 0 ? null : mSearchEngines.get(position - 1);
+    }
+
+    @Override
+    public SearchEngineViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
+        switch (viewType) {
+            case VIEW_TYPE_LABEL:
+                return new SearchEngineViewHolder(createLabelView(parent));
+            case VIEW_TYPE_SEARCH_ENGINE:
+                return new SearchEngineViewHolder(createSearchEngineView(parent));
+            default:
+                throw new IllegalArgumentException("Unknown view type: "  + viewType);
+        }
+    }
+
+    @Override
+    public void onBindViewHolder(SearchEngineViewHolder holder, int position) {
+        if (position != 0) {
+            holder.bindItem(getItem(position));
+        }
+    }
+
+    @Override
+    public int getItemCount() {
+        return mSearchEngines.size() + 1;
+    }
+
+    private View createLabelView(ViewGroup parent) {
+        View view = LayoutInflater.from(mContext)
+                        .inflate(R.layout.search_engine_bar_label, parent, false);
+        final Drawable icon = DrawableCompat.wrap(
+                ContextCompat.getDrawable(mContext, R.drawable.search_icon_active).mutate());
+        DrawableCompat.setTint(icon, mContext.getResources().getColor(R.color.disabled_grey));
+
+        final ImageView iconView = (ImageView) view.findViewById(R.id.search_engine_label);
+        iconView.setImageDrawable(icon);
+        return view;
+    }
+
+    private View createSearchEngineView(ViewGroup parent) {
+        View view = LayoutInflater.from(mContext)
+                    .inflate(R.layout.search_engine_bar_item, parent, false);
+
+        ViewGroup.LayoutParams params = view.getLayoutParams();
+        params.width = mContainerWidth;
+        view.setLayoutParams(params);
+
+        return view;
+    }
+}
--- a/mobile/android/base/home/SearchEngineBar.java
+++ b/mobile/android/base/home/SearchEngineBar.java
@@ -1,215 +1,146 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
- package org.mozilla.gecko.home;
+package org.mozilla.gecko.home;
 
 import android.content.Context;
 import android.graphics.Canvas;
-import android.graphics.Color;
 import android.graphics.Paint;
-import android.graphics.drawable.Drawable;
+import android.support.v7.widget.LinearLayoutManager;
+import android.support.v7.widget.RecyclerView;
 import android.util.AttributeSet;
 import android.util.DisplayMetrics;
 import android.util.TypedValue;
-import android.view.Gravity;
-import android.view.LayoutInflater;
 import android.view.View;
-import android.view.ViewGroup;
-import android.widget.AdapterView;
-import android.widget.BaseAdapter;
-import android.widget.FrameLayout;
-import android.widget.ImageView;
 
 import org.mozilla.gecko.R;
-import org.mozilla.gecko.util.DrawableUtil;
-import org.mozilla.gecko.widget.TwoWayView;
+import org.mozilla.gecko.mozglue.RobocopTarget;
 
-import java.util.ArrayList;
 import java.util.List;
 
-public class SearchEngineBar extends TwoWayView
-                             implements AdapterView.OnItemClickListener {
-    private static final String LOGTAG = "Gecko" + SearchEngineBar.class.getSimpleName();
+public class SearchEngineBar extends RecyclerView
+        implements RecyclerViewItemClickListener.OnClickListener {
+    private static final String LOGTAG = SearchEngineBar.class.getSimpleName();
 
     private static final float ICON_CONTAINER_MIN_WIDTH_DP = 72;
     private static final float LABEL_CONTAINER_WIDTH_DP = 48;
     private static final float DIVIDER_HEIGHT_DP = 1;
 
     public interface OnSearchBarClickListener {
-        public void onSearchBarClickListener(SearchEngine searchEngine);
+        void onSearchBarClickListener(SearchEngine searchEngine);
     }
 
-    private final SearchEngineAdapter adapter;
-    private final Paint dividerPaint;
-    private final float minIconContainerWidth;
-    private final float dividerHeight;
-    private final int labelContainerWidth;
+    private final SearchEngineAdapter mAdapter;
+    private final LinearLayoutManager mLayoutManager;
+    private final Paint mDividerPaint;
+    private final float mMinIconContainerWidth;
+    private final float mDividerHeight;
+    private final int mLabelContainerWidth;
 
-    private int iconContainerWidth;
-    private OnSearchBarClickListener onSearchBarClickListener;
+    private int mIconContainerWidth;
+    private OnSearchBarClickListener mOnSearchBarClickListener;
 
     public SearchEngineBar(final Context context, final AttributeSet attrs) {
         super(context, attrs);
 
-        dividerPaint = new Paint();
-        dividerPaint.setColor(getResources().getColor(R.color.divider_light));
-        dividerPaint.setStyle(Paint.Style.FILL_AND_STROKE);
+        mDividerPaint = new Paint();
+        mDividerPaint.setColor(getResources().getColor(R.color.divider_light));
+        mDividerPaint.setStyle(Paint.Style.FILL_AND_STROKE);
 
         final DisplayMetrics displayMetrics = getResources().getDisplayMetrics();
-        minIconContainerWidth = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, ICON_CONTAINER_MIN_WIDTH_DP, displayMetrics);
-        dividerHeight = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, DIVIDER_HEIGHT_DP, displayMetrics);
-        labelContainerWidth = (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, LABEL_CONTAINER_WIDTH_DP, displayMetrics);
+        mMinIconContainerWidth = TypedValue.applyDimension(
+                TypedValue.COMPLEX_UNIT_DIP, ICON_CONTAINER_MIN_WIDTH_DP, displayMetrics);
+        mDividerHeight = TypedValue.applyDimension(
+                TypedValue.COMPLEX_UNIT_DIP, DIVIDER_HEIGHT_DP, displayMetrics);
+        mLabelContainerWidth = Math.round(TypedValue.applyDimension(
+                TypedValue.COMPLEX_UNIT_DIP, LABEL_CONTAINER_WIDTH_DP, displayMetrics));
+
+        mIconContainerWidth = Math.round(mMinIconContainerWidth);
 
-        iconContainerWidth =  (int) minIconContainerWidth;
+        mAdapter = new SearchEngineAdapter(context);
+        mAdapter.setIconContainerWidth(mIconContainerWidth);
+        mLayoutManager = new LinearLayoutManager(context);
+        mLayoutManager.setOrientation(LinearLayoutManager.HORIZONTAL);
 
-        adapter = new SearchEngineAdapter();
-        setAdapter(adapter);
-        setOnItemClickListener(this);
+        setAdapter(mAdapter);
+        setLayoutManager(mLayoutManager);
+        addOnItemTouchListener(new RecyclerViewItemClickListener(context, this, this));
     }
 
-    @Override
-    public void onItemClick(final AdapterView<?> parent, final View view, final int position,
-            final long id) {
-        if (onSearchBarClickListener == null) {
-            throw new IllegalStateException(
-                    OnSearchBarClickListener.class.getSimpleName() + " is not initialized");
-        }
-
-        if (position == 0) {
-            // Ignore click on label
-            return;
-        }
-
-        final SearchEngine searchEngine = adapter.getItem(position);
-        onSearchBarClickListener.onSearchBarClickListener(searchEngine);
+    public void setSearchEngines(List<SearchEngine> searchEngines) {
+        mAdapter.setSearchEngines(searchEngines);
     }
 
-    protected void setOnSearchBarClickListener(final OnSearchBarClickListener listener) {
-        onSearchBarClickListener = listener;
-    }
-
-    protected void setSearchEngines(final List<SearchEngine> searchEngines) {
-        adapter.setSearchEngines(searchEngines);
+    public void setOnSearchBarClickListener(OnSearchBarClickListener listener) {
+        mOnSearchBarClickListener = listener;
     }
 
     @Override
     protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
         super.onMeasure(widthMeasureSpec, heightMeasureSpec);
 
-        final int searchEngineCount = adapter.getCount() - 1;
+        final int searchEngineCount = mAdapter.getItemCount() - 1;
 
         if (searchEngineCount > 0) {
-            final int availableWidth = getMeasuredWidth() - labelContainerWidth;
+            final int availableWidth = getMeasuredWidth() - mLabelContainerWidth;
             final double searchEnginesToDisplay;
 
-            if (searchEngineCount * minIconContainerWidth <= availableWidth) {
+            if (searchEngineCount * mMinIconContainerWidth <= availableWidth) {
                 // All search engines fit int: So let's just display all.
                 searchEnginesToDisplay = searchEngineCount;
             } else {
                 // If only (n) search engines fit into the available space then display (n - 0.5): The last search
                 // engine will be cut-off to show ability to scroll this view
-
-                searchEnginesToDisplay = Math.floor(availableWidth / minIconContainerWidth) - 0.5;
+                searchEnginesToDisplay = Math.floor(availableWidth / mMinIconContainerWidth) - 0.5;
             }
 
             // Use all available width and spread search engine icons
             final int availableWidthPerContainer = (int) (availableWidth / searchEnginesToDisplay);
 
-            if (availableWidthPerContainer != iconContainerWidth) {
-                iconContainerWidth = availableWidthPerContainer;
-                adapter.notifyDataSetChanged();
+            if (availableWidthPerContainer != mIconContainerWidth) {
+                mIconContainerWidth = availableWidthPerContainer;
             }
+            mAdapter.setIconContainerWidth(mIconContainerWidth);
         }
     }
 
     @Override
-    protected void onDraw(Canvas canvas) {
+    public void onDraw(Canvas canvas) {
         super.onDraw(canvas);
 
-        canvas.drawRect(0, 0, getWidth(), dividerHeight, dividerPaint);
+        canvas.drawRect(0, 0, getWidth(), mDividerHeight, mDividerPaint);
     }
 
-    public class SearchEngineAdapter extends BaseAdapter {
-        private static final int VIEW_TYPE_SEARCH_ENGINE = 0;
-        private static final int VIEW_TYPE_LABEL = 1;
-
-        List<SearchEngine> searchEngines = new ArrayList<>();
-
-        public void setSearchEngines(final List<SearchEngine> searchEngines) {
-            this.searchEngines = searchEngines;
-            notifyDataSetChanged();
+    @Override
+    public void onClick(View view, int position) {
+        if (mOnSearchBarClickListener == null) {
+            throw new IllegalStateException(
+                    OnSearchBarClickListener.class.getSimpleName() + " is not initializer."
+            );
         }
 
-        @Override
-        public int getCount() {
-            // Adding offset for label at position 0 (Bug 1172071)
-            return searchEngines.size() + 1;
-        }
-
-        @Override
-        public SearchEngine getItem(final int position) {
-            // Returning null for the label at position 0 (Bug 1172071)
-            return position == 0 ? null : searchEngines.get(position - 1);
-        }
-
-        @Override
-        public long getItemId(final int position) {
-            return position;
-        }
-
-        @Override
-        public int getItemViewType(int position) {
-            return position == 0 ? VIEW_TYPE_LABEL : VIEW_TYPE_SEARCH_ENGINE;
-        }
-
-        @Override
-        public int getViewTypeCount() {
-            return 2;
+        if (position == 0) {
+            return;
         }
 
-        @Override
-        public View getView(final int position, final View convertView, final ViewGroup parent) {
-            if (position == 0) {
-                return getLabelView(convertView, parent);
-            } else {
-                return getSearchEngineView(position, convertView, parent);
-            }
-        }
+        final SearchEngine searchEngine = mAdapter.getItem(position);
+        mOnSearchBarClickListener.onSearchBarClickListener(searchEngine);
+    }
 
-        private View getLabelView(View view, final ViewGroup parent) {
-            if (view == null) {
-                view = LayoutInflater.from(getContext()).inflate(R.layout.search_engine_bar_label, parent, false);
-            }
-
-            final Drawable icon =
-                    DrawableUtil.tintDrawable(parent.getContext(), R.drawable.search_icon_active, R.color.disabled_grey);
-
-            final ImageView iconView = (ImageView) view.findViewById(R.id.search_engine_label);
-            iconView.setImageDrawable(icon);
-            iconView.setScaleType(ImageView.ScaleType.FIT_XY);
+    @Override
+    public void onLongClick(View view, int position) {
+        // do nothing
+    }
 
-            return view;
-        }
-
-        private View getSearchEngineView(final int position, View view, final ViewGroup parent) {
-            if (view == null) {
-                view = LayoutInflater.from(getContext()).inflate(R.layout.search_engine_bar_item, parent, false);
-            }
-
-            LayoutParams params = (LayoutParams) view.getLayoutParams();
-            params.width = iconContainerWidth;
-            view.setLayoutParams(params);
-
-            final ImageView faviconView = (ImageView) view.findViewById(R.id.search_engine_icon);
-            final SearchEngine searchEngine = getItem(position);
-            faviconView.setImageBitmap(searchEngine.getIcon());
-
-            final String desc = getResources().getString(R.string.search_bar_item_desc, searchEngine.getEngineIdentifier());
-            view.setContentDescription(desc);
-
-            return view;
-        }
+    /**
+     * We manually add the override for getAdapter because we see this method getting stripped
+     * out during compile time by aggressive proguard rules.
+     */
+    @RobocopTarget
+    @Override
+    public SearchEngineAdapter getAdapter() {
+        return mAdapter;
     }
 }
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -347,16 +347,17 @@ gbjar.sources += [
     'home/RecyclerViewItemClickListener.java',
     'home/RemoteTabsBaseFragment.java',
     'home/RemoteTabsExpandableListFragment.java',
     'home/RemoteTabsExpandableListState.java',
     'home/RemoteTabsPanel.java',
     'home/RemoteTabsSplitPlaneFragment.java',
     'home/RemoteTabsStaticFragment.java',
     'home/SearchEngine.java',
+    'home/SearchEngineAdapter.java',
     'home/SearchEngineBar.java',
     'home/SearchEngineRow.java',
     'home/SearchLoader.java',
     'home/SimpleCursorLoader.java',
     'home/SpacingDecoration.java',
     'home/TabMenuStrip.java',
     'home/TabMenuStripLayout.java',
     'home/TopSitesGridItemView.java',
--- a/mobile/android/base/resources/layout/search_engine_bar_item.xml
+++ b/mobile/android/base/resources/layout/search_engine_bar_item.xml
@@ -12,16 +12,17 @@
 
      The actual width of the FrameLayout is calculated at runtime by the
      SearchEngineBar class to spread the icons across the device's width. -->
 <FrameLayout
     xmlns:android="http://schemas.android.com/apk/res/android"
     android:id="@+id/search_engine_icon_container"
     android:layout_width="72dp"
     android:layout_height="match_parent"
+    android:clickable="true"
     android:background="@color/pressed_about_page_header_grey">
 
     <!-- Width & height are set to make the Favicons as sharp as possible
          based on asset size. -->
     <ImageView
         android:id="@+id/search_engine_icon"
         android:layout_width="24dp"
         android:layout_height="24dp"
--- a/mobile/android/base/resources/layout/search_engine_bar_label.xml
+++ b/mobile/android/base/resources/layout/search_engine_bar_label.xml
@@ -11,11 +11,11 @@
     android:layout_width="48dp"
     android:layout_height="match_parent">
 
     <ImageView
         android:id="@+id/search_engine_label"
         android:layout_width="16dp"
         android:layout_height="16dp"
         android:layout_gravity="center"
-        android:scaleType="fitCenter"/>
+        android:scaleType="fitXY"/>
 
 </FrameLayout>
--- a/mobile/android/tests/browser/robocop/AboutHomeTest.java
+++ b/mobile/android/tests/browser/robocop/AboutHomeTest.java
@@ -5,16 +5,17 @@
 package org.mozilla.gecko.tests;
 
 import java.util.ArrayList;
 
 import org.mozilla.gecko.Actions;
 import org.mozilla.gecko.home.HomePager;
 
 import android.support.v4.view.ViewPager;
+import android.support.v7.widget.RecyclerView;
 import android.text.TextUtils;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.ListAdapter;
 import android.widget.ListView;
 import android.widget.TabWidget;
 import android.widget.TextView;
 
@@ -203,17 +204,17 @@ abstract class AboutHomeTest extends Pix
     }
 
     private void clickAboutHomeTab(AboutHomeTabs tab) {
         mSolo.clickOnText(tab.toString().replace("_", " "));
     }
 
     /**
      * Swipes to an about:home tab.
-     * @param int swipeVector Value and direction to swipe (go left for negative, right for positive).
+     * @param swipeVector swipeVector Value and direction to swipe (go left for negative, right for positive).
      */
     private void swipeAboutHome(int swipeVector) {
         // Increase swipe width, which will especially impact tablets.
         int swipeWidth = mDriver.getGeckoWidth() - 1;
         int swipeHeight = mDriver.getGeckoHeight() / 2;
 
         if (swipeVector >= 0) {
             // Emulate swipe motion from right to left.
@@ -227,18 +228,16 @@ abstract class AboutHomeTest extends Pix
                 mActions.drag(0, swipeWidth, swipeHeight, swipeHeight);
                 mSolo.sleep(100);
             }
         }
     }
 
     /**
      * This method can be used to open the different tabs of about:home.
-     *
-     * @param AboutHomeTabs enum item
      */
     protected void openAboutHomeTab(AboutHomeTabs tab) {
         focusUrlBar();
         ViewPager pager = mSolo.getView(ViewPager.class, 0);
         final int currentTabIndex = pager.getCurrentItem();
         int tabOffset;
 
         // Handle tablets by just clicking the visible tab title.
--- a/mobile/android/tests/browser/robocop/testAddSearchEngine.java
+++ b/mobile/android/tests/browser/robocop/testAddSearchEngine.java
@@ -152,17 +152,17 @@ public class testAddSearchEngine extends
                 }
 
                 SearchEngineBar searchEngineBar = (SearchEngineBar) mSolo.getView(R.id.search_engine_bar);
                 if (searchEngineBar == null || searchEngineBar.getAdapter() == null) {
                     return false;
                 }
 
                 final int actualCount = searchResultList.getAdapter().getCount()
-                        + searchEngineBar.getAdapter().getCount()
+                        + searchEngineBar.getAdapter().getItemCount()
                         - 1; // Subtract one for the search engine bar label (Bug 1172071)
 
                 return (actualCount == expectedCount);
             }
         }, MAX_WAIT_TEST_MS);
 
         // Exit about:home
         mSolo.goBack();