Lifecycle aware call wrappers for navigation [317230685]

Fixed

Feature Request

Status Update

No update yet.

Description

bj...@google.com

created issue #1

Dec 20, 2023 05:54PM

When using Compose navigation, calls to navigate should be guarded by lifecycle state to prevent invoking navigation methods while composable is transitioning as an effect of navigation. Given navigation destination:

composable(...) { backStackEntry ->
  Screen1(
    ...,
    navigateToScreen2 = { navController.navigate("screen2") },
    ...,
    navigateBack = { navController.popBackStack() },
  )
}

navigation calls should be guarded by backStackEntry lifecycle state to prevent invoking navigation from composable that's under transition:

composable(...) { backStackEntry ->
  Screen1(
    ...,
    navigateToScreen2 = {
      if (backStackEntry.lifecycle.currentState == Lifecycle.State.RESUMED) {
        navController.navigate("screen2")
      }
  },

It would be really useful to have a set of lifecycle aware function wrappers for guarding execution of calls, for example:

fun NavBackStackEntry.whenResumed(onClick: () -> Unit): () -> Unit

or standalone composable version which will use LocalLifecycleOwner to access lifecycle state:

@Composable
fun whenResumed(onClick: () -> Unit): () -> Unit

Comments

il...@google.com <il...@google.com> Jan 23, 2024 09:23PM

Assigned to mg...@google.com.

mg...@google.com <mg...@google.com> Jan 31, 2024 05:17PM

Accepted by mg...@google.com.

ap...@google.com <ap...@google.com> #2Feb 8, 2024 11:39AM

Jeremy, is this still an issue? I think the problem was that you had two transitions targeting the same View for the same action (e.g. two Slide() transitions).

il...@google.com <il...@google.com> Feb 13, 2024 06:10PM

Marked as fixed.

pr...@google.com <pr...@google.com> #3Feb 21, 2024 07:32PM

I have a similar issue with plain AnimatorSet:

set.start()
set.pause()
set.setCurrentPlayTime(100)
set.setCurrentPlayTime(0)
set.setCurrentPlayTime(100)
set.resume()

doesn't play animation in resume().

he...@reaktor.com <he...@reaktor.com> #4Apr 30, 2024 12:28PM

Should clarify that if I filter out setCurrentPlayTime(0) (or replace it with setCurrentPlayTime(1)) it works well.

Also even with setCurrentPlayTime(0), onAnimationEnd is notified with correct delay (as if the animation has played).

il...@google.com <il...@google.com> #5Apr 30, 2024 09:44PM

@ #4:

I think that is intended for Animator. If you set the currentPlayTime to 0 or the total duration the animator completes. We do some special checks in fragments to handle it.

ha...@gmail.com <ha...@gmail.com> #6Sep 5, 2024 12:12PM

Did some investigation on the Fragment side and it seems like the merged transition is targeting correctly.

Exiting Transition: Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
>>>>> ExitingViews <<<<<
View: android.widget.LinearLayout{f9add3d}
Entering Transition: Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})
>>>>> EnteringViews <<<<<
View: android.widget.LinearLayout{b7f24bc}
Final merged transition: TransitionSet@7bc1c45: 
      TransitionSet@e133f9a: 
        Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
      Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})
merged transition passed to controlDelayedTransition: TransitionSet@7bc1c45: 
      TransitionSet@e133f9a: 
        Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
      Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})

Still digging.

Message last modified on Sep 5, 2024 12:13PM