AnimatedContent sets incorrect target animation on interrupt [238662479]

Fixed

Bug

Status Update

No update yet.

Description

jb...@google.com

created issue #1

Jul 11, 2022 08:05PM

Jetpack Compose version: 1.2.0-rc03
Jetpack Compose component used: AnimatedContent
Android Studio Build: 2022.1.1. Canary 7
Kotlin version: 1.7.1

When using AnimatedContent to animate between two contents, if you interrupt during the animation and attempt to return to the previous state, the target gets the wrong animation.

Given:

var count by remember { mutableStateOf(0) }
AnimatedContent(targetState = count, transitionSpec = {
  if (targetState == 0) {
    fadeIn(tween(2000)) with slideOut(
      tween(2000),
      targetOffset = { fullSize -> IntOffset(0, fullSize.height / 2) }) + fadeOut( 
      tween(2000))
  } else {
    fadeIn(tween(2000)) with fadeOut(tween(2000))
  }
}) { state ->
  if (state == 0) {
    Box(modifier = Modifier
      .fillMaxSize()
      .background(Color.Green)
      .clickable { count++ }
    )
  } else if (state == 1) {
    Box(modifier = Modifier
      .fillMaxSize()
      .background(Color.Red)
    )
    BackHandler { count-- }
  }
}

When starting from the green screen, starting animation to the red screen and interrupting that animation to return to green the green animation incorrectly does a slideIn() transition after the interrupt.

Expected:
The Green screen should always be fading. When we go from green to red it should be fading out. When returning back to green, because it is the targetState it should get the entering animation, which is a fadeIn() and the Red screen should be fading out.

Observed:
On interruption, the Green screen gets the exit transition, which in this case is the slideOut() and reverses it so it ends up doing a slideIn() and the Red screen fades out.

Since the targetState is always the final state of the AnimatedContent, it should always receive whatever enter transition in passed into the transitionSpec.

Comments

ti...@google.com <ti...@google.com> #2Jul 11, 2022 10:18PM

Reassigned to ti...@google.com.

I'd like to add that this automatic removal of the NavGraph at the top of the stack is also making it hard for me to implement proper lateral top-level navigation.
A practical case where this kind of navigation happens is with a Material Design bottom navigation bar, where I have a NavHost above the bottom bar.

Given this navigation scheme:
A - B (top-level)
B1 (first sub-level)

When starting the Activity, the back stack is
0. A
1. NavGraph

If I wanted to navigate to B laterally, I would do:
navController.popBackStack(R.id.navGraph, false)
navController.navigate(R.id.B)

But the resulting back stack is:
0. B

Now, navigating between top-level destinations actually works (though popBackStack returns false); the problem arises again if I navigate to B1:
0. B1
1. B

When navigating from B1 to a top-level destination like A, the stack becomes:
0. A
1. B1
2. B

That is, A is not a top-level destination in the back stack: if I press back from A, I intend to exit the app, but I'm taken back to B1.

It would be nice to have a fixed reference to the bottom of the stack. Right now I'm working around this with a horrible ` while (navController.popBackStack()) continue`.

ti...@gmail.com <ti...@gmail.com> #3Aug 11, 2022 06:09PM

Is this going to be fixed?

I have the same issue with a setup as follows:

Navigation graph.

<navigation xmlns:android="

http://schemas.android.com/apk/res/android"
xmlns:app="

http://schemas.android.com/apk/res-auto"
xmlns:tools="

http://schemas.android.com/tools"
app:startDestination="@+id/startupGraph">

<navigation
android:id="@+id/startupGraph"
app:startDestination="@id/launcher_home">
<fragment
android:id="@+id/launcher_home"
android:name="com.example.android.codelabs.navigation.MainFragment"
android:label="@string/home"
tools:layout="@layout/main_fragment" />
</navigation>

<navigation
android:id="@+id/homeGraph"
app:startDestination="@id/flow_step_one">
<fragment
android:id="@+id/flow_step_one"
android:name="com.example.android.codelabs.navigation.FlowStepOneFragment"
tools:layout="@layout/flow_step_one_fragment" />
<fragment
android:id="@+id/flow_step_two"
android:name="com.example.android.codelabs.navigation.FlowStepTwoFragment"
tools:layout="@layout/flow_step_two_fragment" />
</navigation>

</navigation>

MainFragment.

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
view.findViewById<Button>(R.id.navigate_dest_bt).setOnClickListener{
Navigation.findNavController(it)
.navigate(R.id.homeGraph, null, NavOptions.Builder()
.setPopUpTo(R.id.startupGraph, false)
.build())
}
}

FlowStepOneFragment.

view.findViewById<View>(R.id.next_button).setOnClickListener {
Navigation.findNavController(it)
.navigate(R.id.flow_step_two, null, NavOptions.Builder()
.setPopUpTo(R.id.flow_step_one, true)
.build())
}
}
}

Causes the following error:

E/AndroidRuntime: FATAL EXCEPTION: main
Process: com.example.android.codelabs.navigation, PID: 12579
java.lang.IllegalArgumentException: Navigator androidx.navigation.fragment.FragmentNavigator@d70ec6f reported navigation to unknown destination id com.example.android.codelabs.navigation:id/flow_step_two

Message last modified on Aug 11, 2022 06:12PM

compose-navigation-animation-bug.mp4

7 MB

Download

ap...@google.com <ap...@google.com> #4Sep 30, 2022 06:41PM

The original issue was fixed as a result of

https://android-review.googlesource.com/833717 and a number of other internal changes and that will be available in alpha08.

Discovered another issue when doing this specific set of navigation actions, so leaving this open until that part is also fixed.

ti...@google.com <ti...@google.com> Sep 30, 2022 06:43PM

Marked as fixed.

na...@google.com <na...@google.com> #5Oct 24, 2022 01:37PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit d3507cb9dab82264070c72a37dbde1dfb43ef578
Author: Ian Lake <ilake@google.com>
Date: Tue Dec 04 14:24:32 2018

Split pop and dispatch logic

When using popUpTo, navigate() would
internally call popBackStack(), resulting
in additional dispatch calls to
OnDestinationChangedListener before the
navigate() operation itself was done.

By separating the logic, we can ensure that
we don't prematurely remove NavGraph destinations
that should still exist after the navigate()
operation and that developers only see a
single final OnDestinationChangedListener callback.

Test: new test
BUG: 113611083
Change-Id: Ib63e4156521259a945ffecf679ccb404ac192017

M navigation/runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
M navigation/runtime/src/androidTest/res/navigation/nav_nested_start_destination.xml
M navigation/runtime/src/main/java/androidx/navigation/NavController.java

https://android-review.googlesource.com/840996

https://goto.google.com/android-sha1/d3507cb9dab82264070c72a37dbde1dfb43ef578

[Deleted User] <[Deleted User]> #6Nov 25, 2022 11:35AM

Why is this not released as a hotfix so stable users can get the bug fix right away?

pe...@gmail.com <pe...@gmail.com> #7Nov 27, 2022 10:58AM

He is mine just saying his time is my time