Bug 1116415 - 9. Move grid layout tab updates out of the adapter. r?sebastian draft
authorTom Klein <twointofive@gmail.com>
Fri, 16 Sep 2016 17:44:48 -0500
changeset 424675 b0c0e289670eb91854aa8cb7cba3c28245ee8efd
parent 424674 52aec98d72b99567c23531b2c5c99c5ffb143c9c
child 424676 0c267676cb0824d916f398155b0d5b7dec6f346c
push id32215
push userbmo:twointofive@gmail.com
push dateThu, 13 Oct 2016 05:22:21 +0000
reviewerssebastian
bugs1116415
milestone52.0a1
Bug 1116415 - 9. Move grid layout tab updates out of the adapter. r?sebastian The adapter/animation machinery in the grid case doesn't quite handle the case where we queue up multiple changes and a scroll in one layout pass, so move the tab update operations out of the adapter. MozReview-Commit-ID: DnEKK2QosEy
mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
@@ -1,16 +1,17 @@
 /* -*- 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.Tab;
 import org.mozilla.gecko.widget.SpacingDecoration;
 
 import android.content.Context;
 import android.content.res.Resources;
 import android.support.v7.widget.DefaultItemAnimator;
 import android.support.v7.widget.GridLayoutManager;
 import android.support.v7.widget.helper.ItemTouchHelper;
 import android.util.AttributeSet;
@@ -80,9 +81,31 @@ public class TabsGridLayout extends Tabs
         final int firstVisibleIndex = layoutManager.findFirstVisibleItemPosition();
         // 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);
     }
+
+    @Override
+    protected void updateViewForTab(final Tab tab) {
+        // We frequently queue up multiple changes when we remove or add back a tab (animations,
+        // title changes, selection updates), and while it would be preferable to send all of those
+        // changes through adapter.notifyItemChanged(), that currently leads to slight
+        // layout/animation imperfections in the grid case, so for now we're doing the updates
+        // outside the adapter machinery.
+        post(new Runnable() {
+            @Override
+            public void run() {
+                final int position = ((TabsLayoutAdapter) getAdapter()).getPositionForTab(tab);
+                final ViewHolder viewHolder = findViewHolderForAdapterPosition(position);
+                if (viewHolder == null) {
+                    return;
+                }
+
+                final TabsLayoutItemView itemView = (TabsLayoutItemView) viewHolder.itemView;
+                itemView.assignValues(tab);
+            }
+        });
+    }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
@@ -116,29 +116,33 @@ public abstract class TabsLayout extends
                 break;
 
             case SELECTED:
             case UNSELECTED:
             case THUMBNAIL:
             case TITLE:
             case RECORDING_CHANGE:
             case AUDIO_PLAYING_CHANGE:
-                tabsAdapter.notifyTabChanged(tab);
+                updateViewForTab(tab);
                 break;
         }
     }
 
     // Addition of a tab at selected positions (dependent on LayoutManager) will result in a tab
     // being added out of view - return true if index is such a position.
     abstract protected boolean addAtIndexRequiresScroll(int index);
 
     protected int getSelectedAdapterPosition() {
         return tabsAdapter.getPositionForTab(Tabs.getInstance().getSelectedTab());
     }
 
+    protected void updateViewForTab(Tab tab) {
+        tabsAdapter.notifyTabChanged(tab);
+    }
+
     @Override
     public void onItemClicked(RecyclerView recyclerView, int position, View v) {
         final TabsLayoutItemView item = (TabsLayoutItemView) v;
         final int tabId = item.getTabId();
         final Tabs tabs = Tabs.getInstance();
         tabs.selectTab(tabId);
         autoHidePanel();
         tabs.notifyListeners(tabs.getTab(tabId), Tabs.TabEvents.OPENED_FROM_TABS_TRAY);