Bug 1310081 - 3. Add ItemDecoration to create fixed spacing items. r?sebastian draft
authorTom Klein <twointofive@gmail.com>
Wed, 21 Sep 2016 22:51:27 -0500
changeset 442504 8a690d13ea118dc51bb990be5d5cda785e27abcc
parent 441611 f9e68ff4caf16235a81a934681c6d652e3394533
child 537803 1dc9cf98207e5cc7199ac7369c520905ccfa3cf8
push id36707
push userbmo:twointofive@gmail.com
push dateTue, 22 Nov 2016 15:11:11 +0000
reviewerssebastian
bugs1310081
milestone53.0a1
Bug 1310081 - 3. Add ItemDecoration to create fixed spacing items. r?sebastian We switch to thinking of the tabs grid layout as being determined by specifying the spacing between the items, and then allowing the items themselves to expand to fill whatever room that leaves available, but we also allow the spacing to be adjusted to match the span counts of the previous GridLayout implementation (which is a good thing). MozReview-Commit-ID: L3fgjacMu2d
mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
mobile/android/base/java/org/mozilla/gecko/widget/GridSpacingDecoration.java
mobile/android/base/moz.build
mobile/android/base/resources/layout/tabs_layout_item_view.xml
mobile/android/base/resources/values/dimens.xml
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
@@ -1,45 +1,55 @@
 /* -*- 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.tabs;
 
 import org.mozilla.gecko.R;
+import org.mozilla.gecko.widget.GridSpacingDecoration;
 
 import android.content.Context;
 import android.content.res.Resources;
 import android.support.v7.widget.GridLayoutManager;
 import android.support.v7.widget.helper.ItemTouchHelper;
 import android.util.AttributeSet;
+import android.util.Log;
 
 public class TabsGridLayout extends TabsLayout {
-    private final int desiredColumnWidth;
+    private static final String LOGTAG = "Gecko" + TabsGridLayout.class.getSimpleName();
+
+    private GridSpacingDecoration spacingDecoration;
+    private final int desiredItemWidth;
+    private final int desiredHorizontalItemSpacing;
+    private final int minHorizontalItemSpacing;
+    private final int verticalItemPadding;
 
     public TabsGridLayout(Context context, AttributeSet attrs) {
         super(context, attrs, R.layout.tabs_layout_item_view);
 
         final Resources resources = context.getResources();
 
+        // Actual span count is updated in onSizeChanged.
         setLayoutManager(new GridLayoutManager(context, 1));
 
-        desiredColumnWidth = resources.getDimensionPixelSize(R.dimen.tab_panel_item_width);
+        desiredItemWidth = resources.getDimensionPixelSize(R.dimen.tab_panel_item_width);
+        desiredHorizontalItemSpacing = resources.getDimensionPixelSize(R.dimen.tab_panel_grid_ideal_item_hspacing);
+        minHorizontalItemSpacing = resources.getDimensionPixelOffset(R.dimen.tab_panel_grid_min_item_hspacing);
+        verticalItemPadding = resources.getDimensionPixelSize(R.dimen.tab_panel_grid_item_vpadding);
         final int viewPaddingHorizontal = resources.getDimensionPixelSize(R.dimen.tab_panel_grid_hpadding);
         final int viewPaddingVertical = resources.getDimensionPixelSize(R.dimen.tab_panel_grid_vpadding);
 
         setPadding(viewPaddingHorizontal, viewPaddingVertical, viewPaddingHorizontal, viewPaddingVertical);
         setClipToPadding(false);
         setScrollBarStyle(SCROLLBARS_OUTSIDE_OVERLAY);
 
         setItemAnimator(new TabsGridLayoutAnimator());
 
-        // TODO Add ItemDecoration.
-
         // A TouchHelper handler for swipe to close.
         final TabsTouchHelperCallback callback = new TabsTouchHelperCallback(this) {
             @Override
             protected float alphaForItemSwipeDx(float dX, int distanceToAlphaMin) {
                 return 1f - 2f * Math.abs(dX) / distanceToAlphaMin;
             }
         };
         final ItemTouchHelper touchHelper = new ItemTouchHelper(callback);
@@ -61,28 +71,64 @@ public class TabsGridLayout extends Tabs
         // When you add an item at the first visible position to a GridLayoutManager and there's
         // room to scroll, RecyclerView scrolls the new position to anywhere from near the bottom of
         // its row to completely offscreen (for unknown reasons), so we need to scroll to fix that.
         // We also scroll when the item being added is the only item on the final row.
         return index == firstVisibleIndex ||
                 (index == getAdapter().getItemCount() - 1 && index % spanCount == 0);
     }
 
+    private void updateSpacingDecoration(int horizontalItemSpacing) {
+        if (spacingDecoration != null) {
+            removeItemDecoration(spacingDecoration);
+        }
+        spacingDecoration = new GridSpacingDecoration(horizontalItemSpacing, verticalItemPadding);
+        addItemDecoration(spacingDecoration);
+        updateSelectedPosition();
+    }
+
     @Override
     public void onSizeChanged(int w, int h, int oldw, int oldh) {
         super.onSizeChanged(w, h, oldw, oldh);
 
         if (w == oldw) {
             return;
         }
 
-        // TODO This is temporary - we need to take into account item padding and we'll also try to
-        // match the previous GridLayout span count.
+        final GridLayoutManager layoutManager = (GridLayoutManager) getLayoutManager();
+
         final int nonPaddingWidth = w - getPaddingLeft() - getPaddingRight();
-        // Adjust span based on space available (what GridView does when you say numColumns="auto_fit").
-        final int spanCount = Math.max(1, nonPaddingWidth / desiredColumnWidth);
-        final GridLayoutManager layoutManager = (GridLayoutManager) getLayoutManager();
-        if (spanCount == layoutManager.getSpanCount()) {
-            return;
+        // We lay out the tabs so that the outer two tab edges are butted up against the
+        // RecyclerView padding, and then all other tab edges get their own padding, so
+        // nonPaddingWidth in terms of tab width w and tab spacing s for n tabs is
+        //   n * w + (n - 1) * s
+        // Solving for n gives the formulas below.
+        final int idealSpacingSpanCount = Math.max(1,
+                (nonPaddingWidth + desiredHorizontalItemSpacing) / (desiredItemWidth + desiredHorizontalItemSpacing));
+        final int maxSpanCount = Math.max(1,
+                (nonPaddingWidth + minHorizontalItemSpacing) / (desiredItemWidth + minHorizontalItemSpacing));
+
+        // General caution note: span count can change here at a point where some ItemDecorations
+        // have been computed and some have not, and Android doesn't recompute ItemDecorations after
+        // a setSpanCount call, so we need to always remove and then add back our spacingDecoration
+        // (whose computations depend on spanCount) in order to get a full layout recompute.
+        if (idealSpacingSpanCount == maxSpanCount) {
+            layoutManager.setSpanCount(idealSpacingSpanCount);
+            updateSpacingDecoration(desiredHorizontalItemSpacing);
+        } else {
+            // We're gaining a column by decreasing the item spacing - this actually turns out to be
+            // necessary to fit three columns in landscape mode on many phones.  It also allows us
+            // to match the span counts produced by the previous GridLayout implementation.
+            layoutManager.setSpanCount(maxSpanCount);
+
+            // Increase the spacing as much as we can without giving up our increased span count.
+            for (int spacing = minHorizontalItemSpacing + 1; spacing <= desiredHorizontalItemSpacing; spacing++) {
+                if (maxSpanCount * desiredItemWidth + (maxSpanCount - 1) * spacing > nonPaddingWidth) {
+                    updateSpacingDecoration(spacing - 1);
+                    return;
+                }
+            }
+            // We should never get here if our calculations above were correct.
+            Log.e(LOGTAG, "Span count calculation error");
+            updateSpacingDecoration(minHorizontalItemSpacing);
         }
-        layoutManager.setSpanCount(spanCount);
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
@@ -145,17 +145,17 @@ public abstract class TabsLayout extends
             return;
         }
 
         autoHidePanel();
         Tabs.getInstance().notifyListeners(tab, Tabs.TabEvents.OPENED_FROM_TABS_TRAY);
     }
 
     /** Updates the selected position in the list so that it will be scrolled to the right place. */
-    private void updateSelectedPosition() {
+    protected void updateSelectedPosition() {
         final int selected = getSelectedAdapterPosition();
         if (selected != NO_POSITION) {
             scrollToPosition(selected);
         }
     }
 
     private void refreshTabsData() {
         // Store a different copy of the tabs, so that we don't have to worry about
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/widget/GridSpacingDecoration.java
@@ -0,0 +1,48 @@
+/* -*- 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.widget;
+
+import android.graphics.Rect;
+import android.support.v7.widget.GridLayoutManager;
+import android.support.v7.widget.RecyclerView;
+import android.view.View;
+
+/**
+ * An ItemDecoration for a GridLayoutManager that provides fixed spacing (but not fixed padding)
+ * to create fixed sized items, with no spacing on the outer edges of the outer items.
+ * <p>
+ * So, for example, if there are 2 columns and the spacing is s, then the first column gets a right
+ * padding of s/2 and the second column gets a left paddding of s/2.  If there are three columns
+ * then the first column gets a right padding of 2s/3, the second column gets left and right
+ * paddings of s/3, and the third column gets a left padding of 2s/3.
+ * </p>
+ */
+public class GridSpacingDecoration extends RecyclerView.ItemDecoration {
+    private final int horizontalSpacing;
+    private final int verticalPadding;
+
+    public GridSpacingDecoration(int horizontalSpacing, int verticalPadding) {
+        this.horizontalSpacing = horizontalSpacing;
+        this.verticalPadding = verticalPadding;
+    }
+
+    @Override
+    public void getItemOffsets(Rect outRect, View view, RecyclerView parent, RecyclerView.State state) {
+        final int position = parent.getChildAdapterPosition(view);
+        if (position == RecyclerView.NO_POSITION) {
+            return;
+        }
+
+        final GridLayoutManager layoutManager = (GridLayoutManager) parent.getLayoutManager();
+        final int spanCount = layoutManager.getSpanCount();
+        final int column = position % spanCount;
+
+        final int columnLeftOffset = (int) (((float) column / (float) spanCount) * horizontalSpacing);
+        final int columnRightOffset = (int) (((float) (spanCount - (column + 1)) / (float) spanCount) * horizontalSpacing);
+
+        outRect.set(columnLeftOffset, verticalPadding, columnRightOffset, verticalPadding);
+    }
+}
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -765,16 +765,17 @@ gbjar.sources += ['java/org/mozilla/geck
     'widget/FadedMultiColorTextView.java',
     'widget/FadedSingleColorTextView.java',
     'widget/FadedTextView.java',
     'widget/FaviconView.java',
     'widget/FilledCardView.java',
     'widget/FlowLayout.java',
     'widget/GeckoActionProvider.java',
     'widget/GeckoPopupMenu.java',
+    'widget/GridSpacingDecoration.java',
     'widget/HistoryDividerItemDecoration.java',
     'widget/IconTabWidget.java',
     'widget/LoginDoorHanger.java',
     'widget/RecyclerViewClickSupport.java',
     'widget/ResizablePathDrawable.java',
     'widget/RoundedCornerLayout.java',
     'widget/SiteLogins.java',
     'widget/SquaredImageView.java',
--- a/mobile/android/base/resources/layout/tabs_layout_item_view.xml
+++ b/mobile/android/base/resources/layout/tabs_layout_item_view.xml
@@ -2,25 +2,27 @@
 <!-- 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/. -->
 
 <org.mozilla.gecko.tabs.TabsLayoutItemView xmlns:android="http://schemas.android.com/apk/res/android"
                                            xmlns:gecko="http://schemas.android.com/apk/res-auto"
                                            style="@style/TabsItem"
                                            android:id="@+id/info"
-                                           android:layout_width="fill_parent"
+                                           android:layout_width="match_parent"
                                            android:layout_height="wrap_content"
                                            android:gravity="center"
                                            android:orientation="vertical">
 
-    <LinearLayout android:layout_width="@dimen/tab_thumbnail_width"
+    <LinearLayout android:layout_width="match_parent"
                   android:layout_height="wrap_content"
                   android:orientation="horizontal"
                   android:duplicateParentState="true"
+                  android:paddingLeft="@dimen/tab_highlight_stroke_width"
+                  android:paddingRight="@dimen/tab_highlight_stroke_width"
                   android:paddingBottom="@dimen/tab_highlight_stroke_width">
 
        <org.mozilla.gecko.widget.FadedSingleColorTextView
                android:id="@+id/title"
                android:layout_width="0dip"
                android:layout_height="wrap_content"
                android:layout_weight="1.0"
                style="@style/TabLayoutItemTextAppearance"
@@ -53,17 +55,17 @@
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
             android:padding="@dimen/tab_highlight_stroke_width"
             android:background="@drawable/tab_thumbnail"
             android:duplicateParentState="true"
             android:clipToPadding="false">
 
         <org.mozilla.gecko.tabs.TabsPanelThumbnailView android:id="@+id/thumbnail"
-                                                       android:layout_width="@dimen/tab_thumbnail_width"
-                                                       android:layout_height="@dimen/tab_thumbnail_height"
+                                                       android:layout_width="match_parent"
+                                                       android:layout_height="wrap_content"
                                                        android:elevation="2dp"
                                                        android:outlineProvider="bounds"
                                                 />
 
     </org.mozilla.gecko.widget.TabThumbnailWrapper>
 
 </org.mozilla.gecko.tabs.TabsLayoutItemView>
--- a/mobile/android/base/resources/values/dimens.xml
+++ b/mobile/android/base/resources/values/dimens.xml
@@ -142,17 +142,18 @@
     <dimen name="validation_message_height">50dp</dimen>
     <dimen name="validation_message_margin_top">6dp</dimen>
 
     <dimen name="tab_thumbnail_width">121dp</dimen>
     <dimen name="tab_thumbnail_height">90dp</dimen>
     <dimen name="tab_panel_item_width">129dp</dimen>
     <dimen name="tab_panel_grid_hpadding">20dp</dimen>
     <dimen name="tab_panel_grid_vpadding">19dp</dimen>
-    <dimen name="tab_panel_grid_item_hpadding">1dp</dimen>
+    <dimen name="tab_panel_grid_ideal_item_hspacing">20dp</dimen>
+    <dimen name="tab_panel_grid_min_item_hspacing">2dp</dimen>
     <dimen name="tab_panel_grid_item_vpadding">10dp</dimen>
 
     <dimen name="tab_highlight_stroke_width">4dp</dimen>
 
     <!-- PageActionButtons dimensions -->
     <dimen name="page_action_button_width">32dp</dimen>
 
     <!-- Banner -->