Bug 1116415 - 14. Work around setSupportsChangeAnimations(false) flicker. draft
authorTom Klein <twointofive@gmail.com>
Sun, 25 Sep 2016 00:29:21 -0500
changeset 418606 58b0d9ce886ec91359460c9e9ed96d7a9818e9b1
parent 418604 873094f0808d2cf3f14bc6c9ab5f5b86dfce3fa0
child 532385 242401429d75c7c6431f4ff4d5de538a0acf7a31
push id30722
push userbmo:twointofive@gmail.com
push dateWed, 28 Sep 2016 19:15:32 +0000
bugs1116415
milestone52.0a1
Bug 1116415 - 14. Work around setSupportsChangeAnimations(false) flicker. MozReview-Commit-ID: 7kg09GF9z3g
mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayoutAnimator.java
mobile/android/base/moz.build
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
@@ -5,17 +5,16 @@
 
 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.DefaultItemAnimator;
 import android.support.v7.widget.GridLayoutManager;
 import android.support.v7.widget.helper.ItemTouchHelper;
 import android.util.AttributeSet;
 
 public class TabsGridLayout extends TabsLayout {
 
     public TabsGridLayout(Context context, AttributeSet attrs) {
         super(context, attrs, R.layout.tabs_layout_item_view);
@@ -47,23 +46,17 @@ public class TabsGridLayout extends Tabs
             @Override
             protected float alphaForItemSwipeDx(float dX, int distanceToAlphaMin) {
                 return 1f - 2f * Math.abs(dX) / distanceToAlphaMin;
             }
         };
         final ItemTouchHelper touchHelper = new ItemTouchHelper(callback);
         touchHelper.attachToRecyclerView(this);
 
-        final DefaultItemAnimator animator = new DefaultItemAnimator();
-        // On close we only animate the moves, not the remove.
-        animator.setRemoveDuration(0);
-        // A fade in/out each time the title/thumbnail/etc. gets updated isn't helpful, so disable
-        // the change animation.
-        animator.setSupportsChangeAnimations(false);
-        setItemAnimator(animator);
+        setItemAnimator(new TabsGridLayoutAnimator());
 
         addItemDecoration(new GridSpacingDecoration(itemWidth, verticalItemPadding));
     }
 
     @Override
     public void closeAll() {
         autoHidePanel();
 
@@ -90,18 +83,17 @@ public class TabsGridLayout extends Tabs
         }
 
         final int lastVisible = layoutManager.findLastCompletelyVisibleItemPosition();
         // TODO: When a new item appears in the first position and there's room to scroll,
         // recyclerview scrolls during the animation so that only the bottom of the first row winds
         // up in view.  We want to end at the top of the scroll in this case, so we call
         // scrollToPosition, but the result isn't quite perfect - sometimes it's correct, sometimes
         // it appears as though the new first item is animating in from the top of the screen.
-        // [The condition here is maybe not quite right, plus this sort of unwanted scrolling
-        // occurs in other positions (and on removes) as well, which we're not addressing.]
+        // [The condition here is maybe not quite right and/or general.]
         if (selected == 0 && lastVisible < getAdapter().getItemCount() - 1) {
             scrollToPosition(0);
             return;
         }
 
         // If you close the last tab and it's on a row by itself, then undoing the tab close will
         // open the tab on a new hidden row which RecyclerView doesn't scroll to for you.
         if (lastVisible != NO_POSITION && selected > lastVisible) {
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayoutAnimator.java
@@ -0,0 +1,47 @@
+/* -*- 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 android.support.v7.widget.DefaultItemAnimator;
+import android.support.v7.widget.RecyclerView;
+
+public class TabsGridLayoutAnimator extends DefaultItemAnimator {
+
+    @Override
+    public boolean animateChange(RecyclerView.ViewHolder oldHolder, RecyclerView.ViewHolder newHolder,
+                                 int fromX, int fromY, int toX, int toY) {
+        // We'd like to turn off change animations because a fade in/out every time a tab title or
+        // thumbnail gets updated is distracting; unfortunately setSupportsChangeAnimations(false)
+        // leads to intermittent flickering on adds (when undoing a tab close).
+        // When we undo a tab close we wind up queueing up in short succession a notifyItemAdded for
+        // the new tab and a notifyItemChanged to select the new tab, so the change animation for
+        // the select winds up interacting with the add animations in that case and seems to be
+        // necessary to get the correct dual animation.  On the other hand if we receive a
+        // notifyItemChanged for a title/thumb/etc. update while we're not otherwise animating then
+        // we really don't want a fade/in out, so in that case we cancel the change animation.
+        // This is a hack workaround which depends on the change animation not being the first one
+        // queued on an add, and should be revisited once we move to a more recent version
+        // of the support library.
+        if (fromX == toX && fromY == toY && !isRunning()) {
+            if (oldHolder != null) {
+                dispatchChangeFinished(oldHolder, true);
+            }
+            if (newHolder != null) {
+                dispatchChangeFinished(newHolder, false);
+            }
+            return false;
+        }
+
+        return super.animateChange(oldHolder, newHolder, fromX, fromY, toX, toY);
+    }
+
+    @Override
+    public boolean animateRemove(final RecyclerView.ViewHolder holder) {
+        // Skip the remove animation.
+        dispatchRemoveFinished(holder);
+        return false;
+    }
+}
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -659,16 +659,17 @@ gbjar.sources += ['java/org/mozilla/geck
     'tabs/PrivateTabsPanel.java',
     'tabs/TabCurve.java',
     'tabs/TabHistoryController.java',
     'tabs/TabHistoryFragment.java',
     'tabs/TabHistoryItemRow.java',
     'tabs/TabHistoryPage.java',
     'tabs/TabPanelBackButton.java',
     'tabs/TabsGridLayout.java',
+    'tabs/TabsGridLayoutAnimator.java',
     'tabs/TabsLayout.java',
     'tabs/TabsLayoutAdapter.java',
     'tabs/TabsLayoutItemView.java',
     'tabs/TabsListLayout.java',
     'tabs/TabsListLayoutAnimator.java',
     'tabs/TabsPanel.java',
     'tabs/TabsPanelThumbnailView.java',
     'tabs/TabsTouchHelperCallback.java',