Bug 1383733 - Show two rows of top sites. r=mcomella
authorSebastian Kaspari <s.kaspari@gmail.com>
Wed, 02 Aug 2017 20:11:40 +0200
changeset 642637 c9f1712d723df619689fffe77a3dd4f57183e0e5
parent 642636 b9e3f9520cf424ced4b7bf567ca2b45d577ee327
child 642638 91fbc9dada78e23913c3313273144402f5bfc05c
push id72833
push userbmo:emilio+bugs@crisal.io
push dateTue, 08 Aug 2017 16:50:16 +0000
reviewersmcomella
bugs1383733
milestone57.0a1
Bug 1383733 - Show two rows of top sites. r=mcomella MozReview-Commit-ID: 1EHeCejXoFf
mobile/android/app/src/main/res/layout/activity_stream_topsites_card.xml
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/TopPanel.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPage.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesPagerAdapter.java
--- a/mobile/android/app/src/main/res/layout/activity_stream_topsites_card.xml
+++ b/mobile/android/app/src/main/res/layout/activity_stream_topsites_card.xml
@@ -1,15 +1,16 @@
 <?xml version="1.0" encoding="utf-8"?>
 <FrameLayout
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:gecko="http://schemas.android.com/apk/res-auto"
     xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
-    android:layout_height="match_parent">
+    android:layout_height="match_parent"
+    android:layout_marginBottom="@dimen/activity_stream_base_margin">
 
     <org.mozilla.gecko.widget.FaviconView
         android:id="@+id/favicon"
         android:layout_width="match_parent"
         android:layout_height="match_parent"
         gecko:enableRoundCorners="false"
         tools:background="@drawable/favicon_globe"
         android:scaleType="centerInside"
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
@@ -39,18 +39,18 @@ public class ActivityStreamPanel extends
      */
     private static final int HIGHLIGHTS_CANDIDATES = 500;
 
     /**
      * Number of highlights that should be returned (max).
      */
     private static final int HIGHLIGHTS_LIMIT = 10;
 
-    private static final int MINIMUM_TILES = 4;
-    private static final int MAXIMUM_TILES = 6;
+    public static final int TOP_SITES_COLUMNS = 4;
+    public static final int TOP_SITES_ROWS = 2;
 
     private int desiredTileWidth;
     private int desiredTilesHeight;
     private int tileMargin;
 
     public ActivityStreamPanel(Context context, AttributeSet attrs) {
         super(context, attrs);
 
@@ -98,38 +98,45 @@ public class ActivityStreamPanel extends
 
         adapter.swapTopSitesCursor(null);
     }
 
     @Override
     protected void onSizeChanged(int w, int h, int oldw, int oldh) {
         super.onSizeChanged(w, h, oldw, oldh);
 
-        int tiles = (w - tileMargin) / (desiredTileWidth + tileMargin);
+        // This code does two things:
+        //  * Calculate the size of the tiles we want to display (using a desired width as anchor point)
+        //  * Add padding to the left/right side if there's too much space that we do not need
+
 
-        if (tiles < MINIMUM_TILES) {
-            tiles = MINIMUM_TILES;
+        // Calculate how many tiles fit into the available horizontal space if we are using our
+        // desired tile width.
+        int fittingTiles = (w - tileMargin) / (desiredTileWidth + tileMargin);
 
+        if (fittingTiles <= TOP_SITES_COLUMNS) {
+            // We can't fit all tiles (or they fit exactly) if we are using the desired tiles width.
+            // We will still show all tiles but they might be smaller than what we would like them to be.
             setPadding(0, 0, 0, 0);
-        } else if (tiles > MAXIMUM_TILES) {
-            tiles = MAXIMUM_TILES;
+        } else if (fittingTiles > TOP_SITES_COLUMNS) {
+            // We can fit more tiles than we want to display. Calculate how much space we need and
+            // use the remaining space as padding on the left and right.
+            int needed = TOP_SITES_COLUMNS * (desiredTileWidth + tileMargin) + tileMargin;
+            int padding = (w - needed) / 2;
 
-            // Use the remaining space as padding
-            int needed = tiles * (desiredTileWidth + tileMargin) + tileMargin;
-            int padding = (w - needed) / 2;
+            // With the padding applied we have less space available for the tiles
             w = needed;
 
             setPadding(padding, 0, padding, 0);
-        } else {
-            setPadding(0, 0, 0, 0);
         }
 
-        final int tilesSize = (w - (tiles * tileMargin) - tileMargin) / tiles;
+        // Now calculate how large an individual tile is
+        final int tilesSize = (w - (TOP_SITES_COLUMNS * tileMargin) - tileMargin) / TOP_SITES_COLUMNS;
 
-        adapter.setTileSize(tiles, tilesSize);
+        adapter.setTileSize(TOP_SITES_COLUMNS * TOP_SITES_ROWS, 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);
         }
 
@@ -142,21 +149,23 @@ public class ActivityStreamPanel extends
         public void onLoaderReset(Loader<List<Highlight>> loader) {
             adapter.swapHighlights(Collections.<Highlight>emptyList());
         }
     }
 
     private class TopSitesCallback implements LoaderManager.LoaderCallbacks<Cursor> {
         @Override
         public Loader<Cursor> onCreateLoader(int id, Bundle args) {
+            final int topSitesPerPage = TOP_SITES_COLUMNS * TOP_SITES_ROWS;
+
             final Context context = getContext();
             return BrowserDB.from(context).getActivityStreamTopSites(
                     context,
-                    MAXIMUM_TILES * TopSitesPagerAdapter.SUGGESTED_SITES_MAX_PAGES,
-                    MAXIMUM_TILES * TopSitesPagerAdapter.PAGES);
+                    topSitesPerPage * TopSitesPagerAdapter.SUGGESTED_SITES_MAX_PAGES,
+                    topSitesPerPage * TopSitesPagerAdapter.PAGES);
         }
 
         @Override
         public void onLoadFinished(Loader<Cursor> loader, Cursor data) {
             adapter.swapTopSitesCursor(data);
         }
 
         @Override
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/TopPanel.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/TopPanel.java
@@ -56,18 +56,20 @@ public class TopPanel extends StreamItem
     public void bind(Cursor cursor, int tiles, int tilesSize) {
         final TopSitesPagerAdapter adapter = (TopSitesPagerAdapter) topSitesPager.getAdapter();
         adapter.setTilesSize(tiles, 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();
-        layoutParams.height = tilesSize + tilesMargin * 2;
+        layoutParams.height = (tilesSize * rows) + (tilesMargin * 2);
         topSitesPager.setLayoutParams(layoutParams);
 
         // Reset the page position: binding a new Cursor means that topsites reverts to the first page,
         // no event is sent in that case, but we need to know the right page number to send correct
         // page swipe events
         swipeListener.currentPosition = 0;
     }
 }
\ No newline at end of file
--- 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
@@ -5,30 +5,22 @@
 package org.mozilla.gecko.activitystream.homepanel.topsites;
 
 import android.content.Context;
 import android.support.annotation.Nullable;
 import android.support.v7.widget.GridLayoutManager;
 import android.support.v7.widget.RecyclerView;
 import android.util.AttributeSet;
 
+import org.mozilla.gecko.activitystream.homepanel.ActivityStreamPanel;
 import org.mozilla.gecko.home.HomePager;
 
-public class TopSitesPage
-        extends RecyclerView {
-    public TopSitesPage(Context context,
-                        @Nullable AttributeSet attrs) {
+public class TopSitesPage extends RecyclerView {
+    public TopSitesPage(Context context, @Nullable AttributeSet attrs) {
         super(context, attrs);
 
-        setLayoutManager(new GridLayoutManager(context, 1));
+        setLayoutManager(new GridLayoutManager(getContext(), ActivityStreamPanel.TOP_SITES_COLUMNS));
     }
 
-    public void setTiles(int tiles) {
-        setLayoutManager(new GridLayoutManager(getContext(), tiles));
-    }
-
-    private HomePager.OnUrlOpenListener onUrlOpenListener;
-    private HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener;
-
     public TopSitesPageAdapter getAdapter() {
         return (TopSitesPageAdapter) super.getAdapter();
     }
 }
--- 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
@@ -19,17 +19,17 @@ import org.mozilla.gecko.widget.Recycler
 import java.util.ArrayList;
 import java.util.List;
 
 /**
  * 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 = 4;
+    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;
@@ -105,17 +105,16 @@ public class TopSitesPagerAdapter extend
         // 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);
 
-                page.setTiles(tiles);
                 final TopSitesPageAdapter adapter = new TopSitesPageAdapter(
                         context, i, tiles, 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--) {