navigation-compose-typed icon indicating copy to clipboard operation
navigation-compose-typed copied to clipboard

Consider passing `AnimatedContentScope` to the composables

Open StylianosGakis opened this issue 1 year ago • 2 comments

With the news about shared element transitions coming to compose, naturally a lot of people are going to want to be able to use them in their navigation screens.

As I was trying to get this https://x.com/GakisStylianos/status/1776691881243553937 to work, part of that PR was me having to add this function

private inline fun <reified T : Destination> NavGraphBuilder.animatedComposable(
  deepLinks: List<NavDeepLink> = emptyList(),
  noinline enterTransition: (@JvmSuppressWildcards AnimatedContentTransitionScope<NavBackStackEntry>.() -> EnterTransition?)? = null,
  noinline exitTransition: (@JvmSuppressWildcards AnimatedContentTransitionScope<NavBackStackEntry>.() -> ExitTransition?)? = null,
  noinline popEnterTransition: (@JvmSuppressWildcards AnimatedContentTransitionScope<NavBackStackEntry>.() -> EnterTransition?)? = enterTransition,
  noinline popExitTransition: (@JvmSuppressWildcards AnimatedContentTransitionScope<NavBackStackEntry>.() -> ExitTransition?)? = exitTransition,
  noinline content: @Composable AnimatedContentScope.(NavBackStackEntry, T) -> Unit,
) {
  val serializer = serializer<T>()
  registerDestinationType(T::class, serializer)
  composable(
    route = createRoutePattern(serializer),
    arguments = createNavArguments(serializer),
    enterTransition = enterTransition,
    exitTransition = exitTransition,
    popEnterTransition = popEnterTransition,
    popExitTransition = popExitTransition,
    deepLinks = deepLinks,
  ) { navBackStackEntry ->
    val t = decodeArguments(serializer, navBackStackEntry)
    content(navBackStackEntry, t)
  }
}

To basically turn the

noinline content: @Composable T.(NavBackStackEntry) -> Unit,

from the library into this

noinline content: @Composable AnimatedContentScope.(NavBackStackEntry, T) -> Unit,

To get access to AnimatedContentScope. I suspect this will be a common thing that people will want from their nav library, what are your thoughts? Would you want to add this here?

p.s. I know this exists as well https://github.com/kiwicom/navigation-compose-typed/issues/135 and perhaps there won't be much more work done on this library itself, but if you decide to keep it around it's worth considering.

StylianosGakis avatar Apr 09 '24 08:04 StylianosGakis

What I don't like at all is that the "two args" lambda forces you every time to name them (or put _ there).

This would be nicely solved by context receivers, but with its replacement to context parameters, it will be less optimal. So I'm thinking about something like:

fun <T : Destination> ...composable(...
   noinline content: @Composable DestinationScope<T>.() -> Unit,
)

interface DestinationScope<T : Destination> : AnimatedContentScope {
   val args: T
   val backStackEntry: NavBackStackEntry
}

But let's see how it develops, because there are other struggles for shared transitions having another scope, as can be seen here: https://github.com/android/compose-samples/pull/1314

hrach avatar Apr 09 '24 09:04 hrach

What I don't like at all is that the "two args" lambda forces you every time to name them (or put _ there).

Yes I agree that this is suboptimal, I didn't love that either.

My main goal would be that you somehow need to get a hold to that AnimatedContentScope, and then if you want to provide that through a composition local or not can be a per-case decision. The library can as you show here provide it, (be it with an interface or not), and then each caller can do what they want with it.

I would not suspect the provide is as a local inside androidx.navigation themselves, so in any case kiwi navigation will need to at least expose it to the user.

The other alternative is of course to just do what I did, which is to call the more low level function and do it ourselves in user land. So in any case this is not a blocker, just a good to have 😊

StylianosGakis avatar Apr 09 '24 19:04 StylianosGakis