Fix fragment transitions in new state manager
In aosp/1679887 we attempted to fix doing shared element transitions on
ViewGroups with the transitionGroup="false" attribute set. That change
caused a regression in enter and exit transitions, and could also cause
errors if the entering fragment attempted to transition more views than
the exiting fragment.
This change makes it so that fragments no longer looks at all of the
views in the hierarchy of sharedelement views. We depend on transition
names to determine if a view is a shared element, and if a shared
element is a viewgroup with transitionGroup="true", the entering/exiting
transition should ignore that viewgroup and transition its unnamed
children.
RelNote: "Fixed regression in shared element transtions caused by
aosp/1679887. Setting transitionGroup=\"true\" will now work correctly and
shared elements will no longer throw `IndexOutOfBoundsException`s."
Test: FragmentSharedElementTransitionTest
Bug: 188679569
Bug: 188969304
Change-Id: I164845639cae1f64cf54e4e89b2a24d7eb649beb
diff --git a/fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentSharedElementTransitionTest.kt b/fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentSharedElementTransitionTest.kt
index b3491c7..c0f6bfa 100644
--- a/fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentSharedElementTransitionTest.kt
+++ b/fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentSharedElementTransitionTest.kt
@@ -64,7 +64,7 @@
}
@Test
- fun testNestedSharedElementViewNonMatching() {
+ fun testNestedSharedElementViewsMoreOutViews() {
with(ActivityScenario.launch(FragmentTestActivity::class.java)) {
val fragment = TransitionFragment(R.layout.scene5)
withActivity {
@@ -80,6 +80,11 @@
val redSquare = withActivity { findViewById(R.id.redSquare) }
val startBlueBounds = containerBlueSquare.boundsOnScreen
+ fragment.enterTransition.verifyAndClearTransition {
+ enteringViews += listOf(greenSquare, redSquare)
+ }
+ verifyNoOtherTransitions(fragment)
+
val fragment2 = TransitionFragment(R.layout.scene4)
withActivity {
@@ -93,16 +98,102 @@
val blueSquare = withActivity { findViewById(R.id.blueSquare) }
- fragment.enterTransition.verifyAndClearTransition {
- enteringViews += listOf(containerBlueSquare, greenSquare, redSquare)
+ fragment.exitTransition.verifyAndClearTransition {
+ exitingViews += listOf(greenSquare, redSquare)
+ epicenter = startBlueBounds
}
fragment2.sharedElementEnter.verifyAndClearTransition {
enteringViews += listOf(blueSquare)
- exitingViews += listOf(containerBlueSquare, greenSquare)
+ exitingViews += listOf(containerBlueSquare)
epicenter = startBlueBounds
}
verifyNoOtherTransitions(fragment)
verifyNoOtherTransitions(fragment2)
}
}
+
+ @Test
+ fun testNestedSharedElementViewsMoreInViews() {
+ with(ActivityScenario.launch(FragmentTestActivity::class.java)) {
+ val fragment = TransitionFragment(R.layout.scene4)
+ withActivity {
+ supportFragmentManager
+ .beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.content, fragment)
+ .commit()
+ }
+
+ val blueSquare = withActivity { findViewById(R.id.blueSquare) }
+ val startBlueBounds = blueSquare.boundsOnScreen
+
+ fragment.enterTransition.verifyAndClearTransition {
+ enteringViews += listOf(blueSquare)
+ }
+ verifyNoOtherTransitions(fragment)
+
+ val fragment2 = TransitionFragment(R.layout.scene6)
+
+ withActivity {
+ supportFragmentManager
+ .beginTransaction()
+ .setReorderingAllowed(true)
+ .addSharedElement(blueSquare, "blueSquare")
+ .replace(R.id.content, fragment2)
+ .commit()
+ }
+
+ val containerBlueSquare = withActivity { findViewById(R.id.containerBlueSquare) }
+
+ fragment2.sharedElementEnter.verifyAndClearTransition {
+ exitingViews += listOf(blueSquare)
+ enteringViews += listOf(containerBlueSquare)
+ epicenter = startBlueBounds
+ }
+ verifyNoOtherTransitions(fragment)
+ verifyNoOtherTransitions(fragment2)
+ }
+ }
+
+ @Test
+ fun testNestedTransitionGroupTrue() {
+ with(ActivityScenario.launch(FragmentTestActivity::class.java)) {
+ val fragment = TransitionFragment(R.layout.scene7)
+ withActivity {
+ supportFragmentManager
+ .beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.content, fragment)
+ .commit()
+ }
+
+ val containerBlueSquare = withActivity { findViewById(R.id.containerBlueSquare) }
+
+ fragment.enterTransition.verifyAndClearTransition {
+ enteringViews += listOf(containerBlueSquare)
+ }
+ verifyNoOtherTransitions(fragment)
+
+ val fragment2 = TransitionFragment(R.layout.scene4)
+
+ withActivity {
+ supportFragmentManager
+ .beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.content, fragment2)
+ .commit()
+ }
+
+ val blueSquare = withActivity { findViewById(R.id.blueSquare) }
+
+ fragment.exitTransition.verifyAndClearTransition {
+ exitingViews += listOf(containerBlueSquare)
+ }
+ fragment2.enterTransition.verifyAndClearTransition {
+ enteringViews += listOf(blueSquare)
+ }
+ verifyNoOtherTransitions(fragment)
+ verifyNoOtherTransitions(fragment2)
+ }
+ }
}
diff --git a/fragment/fragment/src/androidTest/res/layout/scene6.xml b/fragment/fragment/src/androidTest/res/layout/scene6.xml
new file mode 100644
index 0000000..552ee6f
--- /dev/null
+++ b/fragment/fragment/src/androidTest/res/layout/scene6.xml
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="utf-8"?><!--
+ Copyright 2021 The Android Open Source Project
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ -->
+
+<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
+ android:id="@+id/sharedElementContainer"
+ android:layout_width="match_parent"
+ android:layout_height="match_parent">
+ <FrameLayout android:id="@+id/containerBlueSquare"
+ android:transitionName="blueSquare"
+ android:layout_width="match_parent"
+ android:layout_height="match_parent"
+ android:background="#008">
+ <View android:id="@+id/greenSquare"
+ android:transitionName="greenSquare"
+ android:layout_width="100dp"
+ android:layout_height="100dp"
+ android:background="#080"/>
+ <View android:id="@+id/redSquare"
+ android:layout_width="100dp"
+ android:layout_height="100dp"
+ android:background="#800"
+ android:layout_gravity="end"/>
+ </FrameLayout>
+</FrameLayout>
\ No newline at end of file
diff --git a/fragment/fragment/src/androidTest/res/layout/scene7.xml b/fragment/fragment/src/androidTest/res/layout/scene7.xml
new file mode 100644
index 0000000..76ad6ee
--- /dev/null
+++ b/fragment/fragment/src/androidTest/res/layout/scene7.xml
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="utf-8"?><!--
+ Copyright 2021 The Android Open Source Project
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ -->
+
+<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
+ android:id="@+id/sharedElementContainer"
+ android:layout_width="match_parent"
+ android:layout_height="match_parent">
+ <FrameLayout android:id="@+id/containerBlueSquare"
+ android:transitionGroup="true"
+ android:layout_width="match_parent"
+ android:layout_height="match_parent"
+ android:background="#008">
+ <View android:id="@+id/greenSquare"
+ android:transitionName="greenSquare"
+ android:layout_width="100dp"
+ android:layout_height="100dp"
+ android:background="#080"/>
+ <View android:id="@+id/redSquare"
+ android:layout_width="100dp"
+ android:layout_height="100dp"
+ android:background="#800"
+ android:layout_gravity="end"/>
+ </FrameLayout>
+</FrameLayout>
\ No newline at end of file
diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/DefaultSpecialEffectsController.java b/fragment/fragment/src/main/java/androidx/fragment/app/DefaultSpecialEffectsController.java
index 5b2884d..20cf2c6 100644
--- a/fragment/fragment/src/main/java/androidx/fragment/app/DefaultSpecialEffectsController.java
+++ b/fragment/fragment/src/main/java/androidx/fragment/app/DefaultSpecialEffectsController.java
@@ -33,6 +33,7 @@
import androidx.core.util.Preconditions;
import androidx.core.view.OneShotPreDrawListener;
import androidx.core.view.ViewCompat;
+import androidx.core.view.ViewGroupCompat;
import java.util.ArrayList;
import java.util.Collection;
@@ -459,11 +460,7 @@
}
});
- // Capture all views from the firstOut Fragment under the shared element views
- for (View sharedElementView : firstOutViews.values()) {
- captureTransitioningViews(sharedElementFirstOutViews,
- sharedElementView);
- }
+ sharedElementFirstOutViews.addAll(firstOutViews.values());
// Compute the epicenter of the firstOut transition
if (!exitingNames.isEmpty()) {
@@ -473,11 +470,7 @@
firstOutEpicenterView);
}
- // Capture all views from the lastIn Fragment under the shared element views
- for (View sharedElementView : lastInViews.values()) {
- captureTransitioningViews(sharedElementLastInViews,
- sharedElementView);
- }
+ sharedElementLastInViews.addAll(lastInViews.values());
// Compute the epicenter of the lastIn transition
if (!enteringNames.isEmpty()) {
@@ -700,16 +693,18 @@
*/
void captureTransitioningViews(ArrayList<View> transitioningViews, View view) {
if (view instanceof ViewGroup) {
- if (!transitioningViews.contains(view)
- && ViewCompat.getTransitionName(view) != null) {
- transitioningViews.add(view);
- }
ViewGroup viewGroup = (ViewGroup) view;
- int count = viewGroup.getChildCount();
- for (int i = 0; i < count; i++) {
- View child = viewGroup.getChildAt(i);
- if (child.getVisibility() == View.VISIBLE) {
- captureTransitioningViews(transitioningViews, child);
+ if (ViewGroupCompat.isTransitionGroup(viewGroup)) {
+ if (!transitioningViews.contains(view)) {
+ transitioningViews.add(viewGroup);
+ }
+ } else {
+ int count = viewGroup.getChildCount();
+ for (int i = 0; i < count; i++) {
+ View child = viewGroup.getChildAt(i);
+ if (child.getVisibility() == View.VISIBLE) {
+ captureTransitioningViews(transitioningViews, child);
+ }
}
}
} else {