kiwicom / navigation-compose-typed

Type-safe arguments for Jetpack Navigation Compose using Kotlinx.Serialization
MIT License
220 stars 9 forks source link

Consider passing `AnimatedContentScope` to the composables #137

Open StylianosGakis opened 6 months ago

StylianosGakis commented 6 months ago

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.

hrach commented 6 months ago

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

StylianosGakis commented 6 months ago

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 😊