Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider passing AnimatedContentScope to the composables #137

Open
StylianosGakis opened this issue Apr 9, 2024 · 2 comments
Open

Consider passing AnimatedContentScope to the composables #137

StylianosGakis opened this issue Apr 9, 2024 · 2 comments

Comments

@StylianosGakis
Copy link

StylianosGakis commented Apr 9, 2024

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 #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
Copy link
Collaborator

hrach commented Apr 9, 2024

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: android/compose-samples#1314

@StylianosGakis
Copy link
Author

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 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants