Make fading transitions work better

Previously, a Fade transition would only affect a view if its
parent hierarchy was not also affected between the start/end states.
This caused problems for views which were removed from their parents
between scenes when their parents' visibility also changed between those
scenes. The effect would be that the transition would fade the parent...
but the child would no longer be in that parent, so the user would just see the
child view blink out.

This fix ensure that views are faded appropriately by fading them
regardless the parent hierarchy; if a view is removed from its
parent, fade it out.

Additionally, if that view has not been removed from its parent, but
its parent is no longer parented *and* scene being
transitioned from is based on a layout resource file (and thus
the views are considered temporary after transitioning), then it is
removed from its parent to be faded out in the overlay.

Also, renamed TextChange to ChangeText to be more consistent with
other transition class names.

Change-Id: I4e0e7dfc9e9d95c7a4ca586534b6d204c4f3bae0

9 files changed, 126 insertions, 95 deletions

diff --git a/core/java/android/transition/TextChange.java b/core/java/android/transition/ChangeText.java
index cf190a1..b1be70f 100644
--- a/core/java/android/transition/TextChange.java
+++ b/core/java/android/transition/ChangeText.java
@@ -37,7 +37,7 @@ import java.util.Map;
  *
  * @hide
  */
-public class TextChange extends Transition {
+public class ChangeText extends Transition {
 
     private static final String LOG_TAG = "TextChange";
 
@@ -103,7 +103,7 @@ public class TextChange extends Transition {
      * transition is run.
      * @return this textChange object.
      */
-    public TextChange setChangeBehavior(int changeBehavior) {
+    public ChangeText setChangeBehavior(int changeBehavior) {
         if (changeBehavior >= CHANGE_BEHAVIOR_KEEP && changeBehavior <= CHANGE_BEHAVIOR_OUT_IN) {
             mChangeBehavior = changeBehavior;
         }
@@ -179,9 +179,13 @@ public class TextChange extends Transition {
             startSelectionStart = startSelectionEnd = endSelectionStart = endSelectionEnd = -1;
         }
         if (!startText.equals(endText)) {
-            view.setText(startText);
-            if (view instanceof EditText) {
-                setSelection(((EditText) view), startSelectionStart, startSelectionEnd);
+            final int startColor = (Integer) startVals.get(PROPNAME_TEXT_COLOR);
+            final int endColor = (Integer) endVals.get(PROPNAME_TEXT_COLOR);
+            if (mChangeBehavior != CHANGE_BEHAVIOR_IN) {
+                view.setText(startText);
+                if (view instanceof EditText) {
+                    setSelection(((EditText) view), startSelectionStart, startSelectionEnd);
+                }
             }
             Animator anim;
             if (mChangeBehavior == CHANGE_BEHAVIOR_KEEP) {
@@ -200,8 +204,6 @@ public class TextChange extends Transition {
                 });
             } else {
                 // Fade out start text
-                final int startColor = (Integer) startVals.get(PROPNAME_TEXT_COLOR);
-                final int endColor = (Integer) endVals.get(PROPNAME_TEXT_COLOR);
                 ValueAnimator outAnim = null, inAnim = null;
                 if (mChangeBehavior == CHANGE_BEHAVIOR_OUT_IN ||
                         mChangeBehavior == CHANGE_BEHAVIOR_OUT) {
@@ -210,8 +212,8 @@ public class TextChange extends Transition {
                         @Override
                         public void onAnimationUpdate(ValueAnimator animation) {
                             int currAlpha = (Integer) animation.getAnimatedValue();
-                            view.setTextColor(currAlpha << 24 | Color.red(startColor) << 16 |
-                                    Color.green(startColor) << 8 | Color.red(startColor));
+                            view.setTextColor(currAlpha << 24 | startColor & 0xff0000 |
+                                    startColor & 0xff00 | startColor & 0xff);
                         }
                     });
                     outAnim.addListener(new AnimatorListenerAdapter() {
@@ -225,6 +227,8 @@ public class TextChange extends Transition {
                                             endSelectionEnd);
                                 }
                             }
+                            // restore opaque alpha and correct end color
+                            view.setTextColor(endColor);
                         }
                     });
                 }
@@ -239,6 +243,13 @@ public class TextChange extends Transition {
                                     Color.green(endColor) << 8 | Color.red(endColor));
                         }
                     });
+                    inAnim.addListener(new AnimatorListenerAdapter() {
+                        @Override
+                        public void onAnimationCancel(Animator animation) {
+                            // restore opaque alpha and correct end color
+                            view.setTextColor(endColor);
+                        }
+                    });
                 }
                 if (outAnim != null && inAnim != null) {
                     anim = new AnimatorSet();
@@ -251,21 +262,32 @@ public class TextChange extends Transition {
                 }
             }
             TransitionListener transitionListener = new TransitionListenerAdapter() {
-                boolean mCanceled = false;
+                int mPausedColor = 0;
 
                 @Override
                 public void onTransitionPause(Transition transition) {
-                    view.setText(endText);
-                    if (view instanceof EditText) {
-                        setSelection(((EditText) view), endSelectionStart, endSelectionEnd);
+                    if (mChangeBehavior != CHANGE_BEHAVIOR_IN) {
+                        view.setText(endText);
+                        if (view instanceof EditText) {
+                            setSelection(((EditText) view), endSelectionStart, endSelectionEnd);
+                        }
+                    }
+                    if (mChangeBehavior > CHANGE_BEHAVIOR_KEEP) {
+                        mPausedColor = view.getCurrentTextColor();
+                        view.setTextColor(endColor);
                     }
                 }
 
                 @Override
                 public void onTransitionResume(Transition transition) {
-                    view.setText(startText);
-                    if (view instanceof EditText) {
-                        setSelection(((EditText) view), startSelectionStart, startSelectionEnd);
+                    if (mChangeBehavior != CHANGE_BEHAVIOR_IN) {
+                        view.setText(startText);
+                        if (view instanceof EditText) {
+                            setSelection(((EditText) view), startSelectionStart, startSelectionEnd);
+                        }
+                    }
+                    if (mChangeBehavior > CHANGE_BEHAVIOR_KEEP) {
+                        view.setTextColor(mPausedColor);
                     }
                 }
             };
diff --git a/core/java/android/transition/Fade.java b/core/java/android/transition/Fade.java
index 5f948bd..8edb1ff 100644
--- a/core/java/android/transition/Fade.java
+++ b/core/java/android/transition/Fade.java
@@ -30,6 +30,24 @@ import android.view.ViewGroup;
  * {@link View#setVisibility(int)} state of the view as well as whether it
  * is parented in the current view hierarchy.
  *
+ * <p>The ability of this transition to fade out a particular view, and the
+ * way that that fading operation takes place, is based on
+ * the situation of the view in the view hierarchy. For example, if a view was
+ * simply removed from its parent, then the view will be added into a {@link
+ * android.view.ViewGroupOverlay} while fading. If a visible view is
+ * changed to be {@link View#GONE} or {@link View#INVISIBLE}, then the
+ * visibility will be changed to {@link View#VISIBLE} for the duration of
+ * the animation. However, if a view is in a hierarchy which is also altering
+ * its visibility, the situation can be more complicated. In general, if a
+ * view that is no longer in the hierarchy in the end scene still has a
+ * parent (so its parent hierarchy was removed, but it was not removed from
+ * its parent), then it will be left alone to avoid side-effects from
+ * improperly removing it from its parent. The only exception to this is if
+ * the previous {@link Scene} was
+ * {@link Scene#getSceneForLayout(android.view.ViewGroup, int, android.content.Context)
+ * created from a layout resource file}, then it is considered safe to un-parent
+ * the starting scene view in order to fade it out.</p>
+ *
  * <p>A Fade transition can be described in a resource file by using the
  * tag <code>fade</code>, along with the standard
  * attributes of {@link android.R.styleable#Fade} and
@@ -167,7 +185,7 @@ public class Fade extends Visibility {
         if ((mFadingMode & OUT) != OUT) {
             return null;
         }
-        View view;
+        View view = null;
         View startView = (startValues != null) ? startValues.view : null;
         View endView = (endValues != null) ? endValues.view : null;
         if (DBG) {
@@ -177,9 +195,28 @@ public class Fade extends Visibility {
         View overlayView = null;
         View viewToKeep = null;
         if (endView == null || endView.getParent() == null) {
-            // view was removed: add the start view to the Overlay
-            view = startView;
-            overlayView = view;
+            if (endView != null) {
+                // endView was removed from its parent - add it to the overlay
+                view = overlayView = endView;
+            } else if (startView != null) {
+                // endView does not exist. Use startView only under certain
+                // conditions, because placing a view in an overlay necessitates
+                // it being removed from its current parent
+                if (startView.getParent() == null) {
+                    // no parent - safe to use
+                    view = overlayView = startView;
+                } else if (startView.getParent() instanceof View &&
+                        startView.getParent().getParent() == null) {
+                    View startParent = (View) startView.getParent();
+                    int id = startParent.getId();
+                    if (id != View.NO_ID && sceneRoot.findViewById(id) != null && mCanRemoveViews) {
+                        // no parent, but its parent is unparented  but the parent
+                        // hierarchy has been replaced by a new hierarchy with the same id
+                        // and it is safe to un-parent startView
+                        view = overlayView = startView;
+                    }
+                }
+            }
         } else {
             // visibility change
             if (endVisibility == View.INVISIBLE) {
diff --git a/core/java/android/transition/Scene.java b/core/java/android/transition/Scene.java
index f81eeef..d798abe 100644
--- a/core/java/android/transition/Scene.java
+++ b/core/java/android/transition/Scene.java
@@ -157,11 +157,11 @@ public final class Scene {
     public void enter() {
 
         // Apply layout change, if any
-        if (mLayoutId >= 0 || mLayout != null) {
+        if (mLayoutId > 0 || mLayout != null) {
             // empty out parent container before adding to it
             getSceneRoot().removeAllViews();
 
-            if (mLayoutId >= 0) {
+            if (mLayoutId > 0) {
                 LayoutInflater.from(mContext).inflate(mLayoutId, mSceneRoot);
             } else {
                 mSceneRoot.addView(mLayout);
@@ -242,4 +242,19 @@ public final class Scene {
         mExitAction = action;
     }
 
+
+    /**
+     * Returns whether this Scene was created by a layout resource file, determined
+     * by the layoutId passed into
+     * {@link #getSceneForLayout(android.view.ViewGroup, int, android.content.Context)}.
+     * This is called by TransitionManager to determine whether it is safe for views from
+     * this scene to be removed from their parents when the scene is exited, which is
+     * used by {@link Fade} to fade these views out (the views must be removed from
+     * their parent in order to add them to the overlay for fading purposes). If a
+     * Scene is not based on a resource file, then the impact of removing views
+     * arbitrarily is unknown and should be avoided.
+     */
+    boolean isCreatedFromLayoutResource() {
+        return (mLayoutId > 0);
+    }
 }
\ No newline at end of file
diff --git a/core/java/android/transition/Transition.java b/core/java/android/transition/Transition.java
index a552fd4..dcf668b 100644
--- a/core/java/android/transition/Transition.java
+++ b/core/java/android/transition/Transition.java
@@ -118,6 +118,14 @@ public abstract class Transition implements Cloneable {
     // Scene Root is set at createAnimator() time in the cloned Transition
     ViewGroup mSceneRoot = null;
 
+    // Whether removing views from their parent is possible. This is only for views
+    // in the start scene, which are no longer in the view hierarchy. This property
+    // is determined by whether the previous Scene was created from a layout
+    // resource, and thus the views from the exited scene are going away anyway
+    // and can be removed as necessary to achieve a particular effect, such as
+    // removing them from parents to add them to overlays.
+    boolean mCanRemoveViews = false;
+
     // Track all animators in use in case the transition gets canceled and needs to
     // cancel running animators
     private ArrayList<Animator> mCurrentAnimators = new ArrayList<Animator>();
@@ -1445,6 +1453,10 @@ public abstract class Transition implements Cloneable {
         return this;
     }
 
+    void setCanRemoveViews(boolean canRemoveViews) {
+        mCanRemoveViews = canRemoveViews;
+    }
+
     @Override
     public String toString() {
         return toString("");
diff --git a/core/java/android/transition/TransitionManager.java b/core/java/android/transition/TransitionManager.java
index 44ca4e5..9be91d0 100644
--- a/core/java/android/transition/TransitionManager.java
+++ b/core/java/android/transition/TransitionManager.java
@@ -178,6 +178,11 @@ public class TransitionManager {
         Transition transitionClone = transition.clone();
         transitionClone.setSceneRoot(sceneRoot);
 
+        Scene oldScene = Scene.getCurrentScene(sceneRoot);
+        if (oldScene != null && oldScene.isCreatedFromLayoutResource()) {
+            transitionClone.setCanRemoveViews(true);
+        }
+
         sceneChangeSetup(sceneRoot, transitionClone);
 
         scene.enter();
diff --git a/core/java/android/transition/TransitionSet.java b/core/java/android/transition/TransitionSet.java
index 6fdd309..79cd8b6 100644
--- a/core/java/android/transition/TransitionSet.java
+++ b/core/java/android/transition/TransitionSet.java
@@ -340,6 +340,15 @@ public class TransitionSet extends Transition {
     }
 
     @Override
+    void setCanRemoveViews(boolean canRemoveViews) {
+        super.setCanRemoveViews(canRemoveViews);
+        int numTransitions = mTransitions.size();
+        for (int i = 0; i < numTransitions; ++i) {
+            mTransitions.get(i).setCanRemoveViews(canRemoveViews);
+        }
+    }
+
+    @Override
     String toString(String indent) {
         String result = super.toString(indent);
         for (int i = 0; i < mTransitions.size(); ++i) {
diff --git a/core/java/android/transition/Visibility.java b/core/java/android/transition/Visibility.java
index f49821f..44f92cd 100644
--- a/core/java/android/transition/Visibility.java
+++ b/core/java/android/transition/Visibility.java
@@ -30,22 +30,6 @@ import android.view.ViewGroup;
  * changes occur. Subclasses should implement one or both of the methods
  * {@link #onAppear(ViewGroup, TransitionValues, int, TransitionValues, int)},
  * {@link #onDisappear(ViewGroup, TransitionValues, int, TransitionValues, int)},
- *
- * <p>Note that a view's visibility change is determined by both whether the view
- * itself is changing and whether its parent hierarchy's visibility is changing.
- * That is, a view that appears in the end scene will only trigger a call to
- * {@link #onAppear(android.view.ViewGroup, TransitionValues, int, TransitionValues, int)
- * appear()} if its parent hierarchy was stable between the start and end scenes.
- * This is done to avoid causing a visibility transition on every node in a hierarchy
- * when only the top-most node is the one that should be transitioned in/out.
- * Stability is determined by either the parent hierarchy views being the same
- * between scenes or, if scenes are inflated from layout resource files and thus
- * have result in different view instances, if the views represented by
- * the ids of those parents are stable. This means that visibility determination
- * is more effective with inflated view hierarchies if ids are used.
- * The exception to this is when the visibility subclass transition is
- * targeted at specific views, in which case the visibility of parent views
- * is ignored.</p>
  */
 public abstract class Visibility extends Transition {
 
@@ -111,51 +95,6 @@ public abstract class Visibility extends Transition {
         return visibility == View.VISIBLE && parent != null;
     }
 
-    /**
-     * Tests whether the hierarchy, up to the scene root, changes visibility between
-     * start and end scenes. This is done to ensure that a view that changes visibility
-     * is only animated if that view's parent was stable between scenes; we should not
-     * fade an entire hierarchy, but rather just the top-most node in the hierarchy that
-     * changed visibility. Note that both the start and end parents are passed in
-     * because the instances may differ for the same view due to layout inflation
-     * between scenes.
-     *
-     * @param sceneRoot The root of the scene hierarchy
-     * @param startView The container view in the start scene
-     * @param endView The container view in the end scene
-     * @return true if the parent hierarchy experienced a visibility change, false
-     * otherwise
-     */
-    private boolean isHierarchyVisibilityChanging(ViewGroup sceneRoot, ViewGroup startView,
-            ViewGroup endView) {
-
-        if (startView == sceneRoot || endView == sceneRoot) {
-            return false;
-        }
-        TransitionValues startValues = startView != null ?
-                getTransitionValues(startView, true) : getTransitionValues(endView, true);
-        TransitionValues endValues = endView != null ?
-                getTransitionValues(endView, false) : getTransitionValues(startView, false);
-
-        if (startValues == null || endValues == null) {
-            return true;
-        }
-        Integer visibility = (Integer) startValues.values.get(PROPNAME_VISIBILITY);
-        int startVisibility = (visibility != null) ? visibility : -1;
-        ViewGroup startParent = (ViewGroup) startValues.values.get(PROPNAME_PARENT);
-        visibility = (Integer) endValues.values.get(PROPNAME_VISIBILITY);
-        int endVisibility = (visibility != null) ? visibility : -1;
-        ViewGroup endParent = (ViewGroup) endValues.values.get(PROPNAME_PARENT);
-        if (startVisibility != endVisibility || startParent != endParent) {
-            return true;
-        }
-
-        if (startParent != null || endParent != null) {
-            return isHierarchyVisibilityChanging(sceneRoot, startParent, endParent);
-        }
-        return false;
-    }
-
     private VisibilityInfo getVisibilityChangeInfo(TransitionValues startValues,
             TransitionValues endValues) {
         final VisibilityInfo visInfo = new VisibilityInfo();
@@ -225,9 +164,7 @@ public abstract class Visibility extends Transition {
                 int endId = endView != null ? endView.getId() : -1;
                 isTarget = isValidTarget(startView, startId) || isValidTarget(endView, endId);
             }
-            if (isTarget || ((visInfo.startParent != null || visInfo.endParent != null) &&
-                    !isHierarchyVisibilityChanging(sceneRoot,
-                            visInfo.startParent, visInfo.endParent))) {
+            if (isTarget || ((visInfo.startParent != null || visInfo.endParent != null))) {
                 if (visInfo.fadeIn) {
                     return onAppear(sceneRoot, startValues, visInfo.startVisibility,
                             endValues, visInfo.endVisibility);
diff --git a/core/java/com/android/internal/transition/ActionBarTransition.java b/core/java/com/android/internal/transition/ActionBarTransition.java
index 8beae8c..c1065e7 100644
--- a/core/java/com/android/internal/transition/ActionBarTransition.java
+++ b/core/java/com/android/internal/transition/ActionBarTransition.java
@@ -19,7 +19,7 @@ package com.android.internal.transition;
 
 import android.transition.ChangeBounds;
 import android.transition.Fade;
-import android.transition.TextChange;
+import android.transition.ChangeText;
 import android.transition.Transition;
 import android.transition.TransitionManager;
 import android.transition.TransitionSet;
@@ -35,8 +35,8 @@ public class ActionBarTransition {
 
     static {
         if (TRANSITIONS_ENABLED) {
-            final TextChange tc = new TextChange();
-            tc.setChangeBehavior(TextChange.CHANGE_BEHAVIOR_OUT_IN);
+            final ChangeText tc = new ChangeText();
+            tc.setChangeBehavior(ChangeText.CHANGE_BEHAVIOR_OUT_IN);
             final TransitionSet inner = new TransitionSet();
             inner.addTransition(tc).addTransition(new ChangeBounds());
             final TransitionSet tg = new TransitionSet();
diff --git a/core/java/com/android/internal/widget/ActionBarView.java b/core/java/com/android/internal/widget/ActionBarView.java
index a6566d5..b5d74e8 100644
--- a/core/java/com/android/internal/widget/ActionBarView.java
+++ b/core/java/com/android/internal/widget/ActionBarView.java
@@ -29,12 +29,6 @@ import android.os.Parcel;
 import android.os.Parcelable;
 import android.text.Layout;
 import android.text.TextUtils;
-import android.transition.ChangeBounds;
-import android.transition.Fade;
-import android.transition.TextChange;
-import android.transition.Transition;
-import android.transition.TransitionManager;
-import android.transition.TransitionSet;
 import android.util.AttributeSet;
 import android.view.CollapsibleActionView;
 import android.view.Gravity;