Bug 1400072: Make num tiles a constant. r=liuche draft
authorMichael Comella <michael.l.comella@gmail.com>
Fri, 15 Sep 2017 10:30:08 -0700
changeset 665722 7ea79634c5e03fdc17a9df977f231f48244c3ca3
parent 665721 4a58a064889664fb4d220f53c1af90ad02e85d5c
child 665723 f6c0e40c150ec1419b5fecb5ef6b8e5f8b534373
push id80150
push usermichael.l.comella@gmail.com
push dateFri, 15 Sep 2017 21:05:44 +0000
reviewersliuche
bugs1400072, 1397854
milestone57.0a1
Bug 1400072: Make num tiles a constant. r=liuche Since it's a constant, there's no reason to keep passing it around everywhere. Also, the code that relied on it being a dynamic value is probably broken so there's no reason to continue passing it around. That being said, there is bug 1397854, which would have TopSites be 4 rows with no pages, but this code is quite messy so I think it'd be worth trying to refactor this code further before trying to implement that. MozReview-Commit-ID: IoMNHVt67c4
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamRecyclerAdapter.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/TopPanelRow.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPage.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPageAdapter.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPagerAdapter.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
@@ -140,17 +140,17 @@ public class ActivityStreamPanel extends
             w = needed;
 
             setPadding(padding, 0, padding, 0);
         }
 
         // Now calculate how large an individual tile is
         final int tilesSize = (w - (TopSitesPage.NUM_COLUMNS * tileMargin) - tileMargin) / TopSitesPage.NUM_COLUMNS;
 
-        adapter.setTileSize(TopSitesPage.NUM_COLUMNS * TopSitesPage.NUM_ROWS, tilesSize);
+        adapter.setTileSize(tilesSize);
     }
 
     private class HighlightsCallbacks implements LoaderManager.LoaderCallbacks<List<Highlight>> {
         @Override
         public Loader<List<Highlight>> onCreateLoader(int id, Bundle args) {
             return new HighlightsLoader(getContext(), HIGHLIGHTS_CANDIDATES, HIGHLIGHTS_LIMIT);
         }
 
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamRecyclerAdapter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamRecyclerAdapter.java
@@ -55,17 +55,16 @@ public class StreamRecyclerAdapter exten
 
     // Content sections available on the Activity Stream page. These may be hidden if the sections are disabled.
     private final RowItemType[] ACTIVITY_STREAM_SECTIONS = { RowItemType.TOP_PANEL, RowItemType.TOP_STORIES_TITLE, RowItemType.HIGHLIGHTS_TITLE };
     public static final int MAX_TOP_STORIES = 3;
 
     private HomePager.OnUrlOpenListener onUrlOpenListener;
     private HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener;
 
-    private int tiles;
     private int tilesSize;
 
     public enum RowItemType {
         TOP_PANEL (-2), // RecyclerView.NO_ID is -1, so start hard-coded stableIds at -2.
         TOP_STORIES_TITLE(-3),
         TOP_STORIES_ITEM(-1), // There can be multiple Top Stories items so caller should handle as a special case.
         HIGHLIGHTS_TITLE (-4),
         HIGHLIGHTS_EMPTY_STATE(-5),
@@ -100,20 +99,18 @@ public class StreamRecyclerAdapter exten
         topStoriesQueue = Collections.emptyList();
     }
 
     void setOnUrlOpenListeners(HomePager.OnUrlOpenListener onUrlOpenListener, HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener) {
         this.onUrlOpenListener = onUrlOpenListener;
         this.onUrlOpenInBackgroundListener = onUrlOpenInBackgroundListener;
     }
 
-    public void setTileSize(int tiles, int tilesSize) {
+    public void setTileSize(int tilesSize) {
         this.tilesSize = tilesSize;
-        this.tiles = tiles;
-
         notifyDataSetChanged();
     }
 
     @Override
     public int getItemViewType(int position) {
         if (position >= recyclerViewModel.size()) {
             throw new IllegalArgumentException("Requested position does not exist");
         }
@@ -171,17 +168,17 @@ public class StreamRecyclerAdapter exten
 
     @Override
     public void onBindViewHolder(StreamViewHolder holder, int position) {
         int type = getItemViewType(position);
         if (type == RowItemType.HIGHLIGHT_ITEM.getViewType()) {
             final Highlight highlight = (Highlight) recyclerViewModel.get(position);
             ((WebpageItemRow) holder).bind(highlight, position, tilesSize);
         } else if (type == RowItemType.TOP_PANEL.getViewType()) {
-            ((TopPanelRow) holder).bind(topSitesCursor, tiles, tilesSize);
+            ((TopPanelRow) holder).bind(topSitesCursor, tilesSize);
         } else if (type == RowItemType.TOP_STORIES_ITEM.getViewType()) {
             final TopStory story = (TopStory) recyclerViewModel.get(position);
             ((WebpageItemRow) holder).bind(story, position, tilesSize);
         }
     }
 
     @Override
     public void onItemClicked(RecyclerView recyclerView, int position, View v) {
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/TopPanelRow.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/TopPanelRow.java
@@ -48,19 +48,19 @@ public class TopPanelRow extends StreamV
     public TopPanelRow(View itemView, HomePager.OnUrlOpenListener onUrlOpenListener, HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener) {
         super(itemView);
 
         topSitesPager = (ViewPager) itemView.findViewById(R.id.topsites_pager);
         topSitesPager.setAdapter(new TopSitesPagerAdapter(itemView.getContext(), onUrlOpenListener, onUrlOpenInBackgroundListener));
         topSitesPager.addOnPageChangeListener(swipeListener);
     }
 
-    public void bind(Cursor cursor, int tiles, int tilesSize) {
+    public void bind(Cursor cursor, int tilesSize) {
         final TopSitesPagerAdapter adapter = (TopSitesPagerAdapter) topSitesPager.getAdapter();
-        adapter.setTilesSize(tiles, tilesSize);
+        adapter.setTilesSize(tilesSize);
         adapter.swapCursor(cursor);
 
         final Resources resources = itemView.getResources();
         final int tilesMargin = resources.getDimensionPixelSize(R.dimen.activity_stream_base_margin);
 
         final int rows = cursor == null || cursor.getCount() > 4 ? 2 : 1;
 
         ViewGroup.LayoutParams layoutParams = topSitesPager.getLayoutParams();
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPage.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPage.java
@@ -9,16 +9,17 @@ import android.support.annotation.Nullab
 import android.support.v7.widget.GridLayoutManager;
 import android.support.v7.widget.RecyclerView;
 import android.util.AttributeSet;
 
 public class TopSitesPage extends RecyclerView {
 
     public static final int NUM_COLUMNS = 4;
     public static final int NUM_ROWS = 2;
+    public static final int NUM_TILES = NUM_COLUMNS * NUM_ROWS;
 
     public TopSitesPage(Context context, @Nullable AttributeSet attrs) {
         super(context, attrs);
 
         setLayoutManager(new GridLayoutManager(getContext(), NUM_COLUMNS));
     }
 
     public TopSitesPageAdapter getAdapter() {
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPageAdapter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPageAdapter.java
@@ -24,29 +24,27 @@ import org.mozilla.gecko.widget.Recycler
 
 import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
 
 /* package-local */ class TopSitesPageAdapter extends RecyclerView.Adapter<TopSitesCard> implements RecyclerViewClickSupport.OnItemClickListener {
     private List<TopSite> topSites;
     private final int pageNumber;
-    private int tiles;
     private int tilesSize;
 
     private final HomePager.OnUrlOpenListener onUrlOpenListener;
     private final HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener;
 
-    /* package-local */ TopSitesPageAdapter(Context context, int pageNumber, int tiles, int tilesSize,
+    /* package-local */ TopSitesPageAdapter(Context context, int pageNumber, int tilesSize,
                                HomePager.OnUrlOpenListener onUrlOpenListener, HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener) {
         setHasStableIds(true);
 
         this.topSites = new ArrayList<>();
         this.pageNumber = pageNumber;
-        this.tiles = tiles;
         this.tilesSize = tilesSize;
 
         this.onUrlOpenListener = onUrlOpenListener;
         this.onUrlOpenInBackgroundListener = onUrlOpenInBackgroundListener;
     }
 
     /**
      * @param startIndex The first item that this topsites group should show. This item, and the following
@@ -54,17 +52,17 @@ import java.util.List;
      */
     public void swapCursor(Cursor cursor, int startIndex) {
         topSites.clear();
 
         if (cursor == null) {
             return;
         }
 
-        for (int i = 0; i < tiles && startIndex + i < cursor.getCount(); i++) {
+        for (int i = 0; i < TopSitesPage.NUM_TILES && startIndex + i < cursor.getCount(); i++) {
             cursor.moveToPosition(startIndex + i);
 
             topSites.add(TopSite.fromCursor(cursor));
         }
 
         notifyDataSetChanged();
     }
 
@@ -118,14 +116,14 @@ import java.util.List;
         return topSites.get(position).getId();
     }
 
     /**
      * This assumes that every TopSite page up to the current one has the same number of tiles.
      * relativePosition must range from 0 to {number of tiles on the current page}.
      */
     private int getTopSiteAbsolutePosition(int relativePosition) {
-        if (relativePosition < 0 || relativePosition > tiles) {
+        if (relativePosition < 0 || relativePosition > TopSitesPage.NUM_TILES) {
             throw new IllegalArgumentException("Illegal relative top site position encountered");
         }
-        return relativePosition + pageNumber * tiles;
+        return relativePosition + pageNumber * TopSitesPage.NUM_TILES;
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPagerAdapter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPagerAdapter.java
@@ -14,25 +14,26 @@ import android.view.ViewParent;
 
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.home.HomePager;
 import org.mozilla.gecko.widget.RecyclerViewClickSupport;
 
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.mozilla.gecko.activitystream.homepanel.topsites.TopSitesPage.NUM_TILES;
+
 /**
  * The primary / top-level TopSites adapter: it handles the ViewPager, and also handles
  * all lower-level Adapters that populate the individual topsite items.
  */
 public class TopSitesPagerAdapter extends PagerAdapter {
     public static final int PAGES = 2;
     public static final int SUGGESTED_SITES_MAX_PAGES = 2;
 
-    private int tiles;
     private int tilesSize;
 
     private final List<TopSitesPage> pages;
 
     private final Context context;
     private final HomePager.OnUrlOpenListener onUrlOpenListener;
     private final HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener;
 
@@ -43,19 +44,18 @@ public class TopSitesPagerAdapter extend
                                 HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener) {
         pages = new ArrayList<>(PAGES);
 
         this.context = context;
         this.onUrlOpenListener = onUrlOpenListener;
         this.onUrlOpenInBackgroundListener = onUrlOpenInBackgroundListener;
     }
 
-    public void setTilesSize(int tiles, int tilesSize) {
+    public void setTilesSize(int tilesSize) {
         this.tilesSize = tilesSize;
-        this.tiles = tiles;
     }
 
     @Override
     public int getCount() {
         return Math.min(count, 4);
     }
 
     @Override
@@ -91,32 +91,32 @@ public class TopSitesPagerAdapter extend
     @Override
     public void destroyItem(ViewGroup container, int position, Object object) {
         container.removeView((View) object);
     }
 
     public void swapCursor(Cursor cursor) {
         // Divide while rounding up: 0 items = 0 pages, 1-ITEMS_PER_PAGE items = 1 page, etc.
         if (cursor != null) {
-            count = (cursor.getCount() - 1) / tiles + 1;
+            count = (cursor.getCount() - 1) / TopSitesPage.NUM_TILES + 1;
         } else {
             count = 0;
         }
 
         // Try to only add/remove pages if really needed: this minimises the amount of UI work that
         // happens when e.g. only one topsite has moved or has been added.
         final int pageDelta = count - pages.size();
 
         if (pageDelta > 0) {
             final LayoutInflater inflater = LayoutInflater.from(context);
             for (int i = 0; i < pageDelta; i++) {
                 final TopSitesPage page = (TopSitesPage) inflater.inflate(R.layout.activity_stream_topsites_page, null, false);
 
                 final TopSitesPageAdapter adapter = new TopSitesPageAdapter(
-                        context, i, tiles, tilesSize, onUrlOpenListener, onUrlOpenInBackgroundListener);
+                        context, i, tilesSize, onUrlOpenListener, onUrlOpenInBackgroundListener);
                 page.setAdapter(adapter);
                 RecyclerViewClickSupport.addTo(page).setOnItemClickListener(adapter);
                 pages.add(page);
             }
         } else if (pageDelta < 0) {
             for (int i = 0; i > pageDelta; i--) {
                 final TopSitesPage page = pages.get(pages.size() - 1);
 
@@ -127,14 +127,14 @@ public class TopSitesPagerAdapter extend
             }
         } else {
             // do nothing: we will be updating all the pages below
         }
 
         int startIndex = 0;
         for (TopSitesPage page : pages) {
             page.getAdapter().swapCursor(cursor, startIndex);
-            startIndex += tiles;
+            startIndex += NUM_TILES;
         }
 
         notifyDataSetChanged();
     }
 }