Naming suggestion for {X}Screen to {X}Route

[mustafayigitt]

I have a naming suggestion for navigation.

As I see, Nia is following this flow: NavHost > Feature Navigation graph/route > Feature Screen Composable Example: NiaNavHost > forYouScreen > ForYouRoute

But I think it should be like this: NiaNavHost > forYouRoute > ForYouScreen

We actually tell it to open the target feature route; not the screen. The target screen will be shown by the route.

// Current
fun NavGraphBuilder.forYouScreen(onTopicClick: (String) -> Unit) {
    composable(
        route = forYouNavigationRoute,
        deepLinks = listOf(
            navDeepLink { uriPattern = DEEP_LINK_URI_PATTERN },
        ),
        arguments = listOf(
            navArgument(LINKED_NEWS_RESOURCE_ID) { type = NavType.StringType },
        ),
    ) {
        ForYouRoute(onTopicClick)
    }
}

// Should be
fun NavGraphBuilder.forYouRoute(onTopicClick: (String) -> Unit) {
    composable(
        route = forYouNavigationRoute,
        deepLinks = listOf(
            navDeepLink { uriPattern = DEEP_LINK_URI_PATTERN },
        ),
        arguments = listOf(
            navArgument(LINKED_NEWS_RESOURCE_ID) { type = NavType.StringType },
        ),
    ) {
        ForYouScreen(onTopicClick)
    }
}

https://github.com/android/nowinandroid/blob/b989d3a243d09e40fadaba1264ea20d7df89e362/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt#L37C21-L37C33

[lelelongwang]

I agree with your suggestion, but I think it currently looks like this: NiaNavHost > forYouScreen > ForYouRoute > ForYouScreen

Referring to your thinking, is it better to change it to the following? NiaNavHost > forYouRoute > ForYouRoute > ForYouScreen

That's it:

fun NavGraphBuilder.forYouRoute(onTopicClick: (String) -> Unit) {
    composable(
        route = forYouNavigationRoute,
        deepLinks = listOf(
            navDeepLink { uriPattern = DEEP_LINK_URI_PATTERN },
        ),
        arguments = listOf(
            navArgument(LINKED_NEWS_RESOURCE_ID) { type = NavType.StringType },
        ),
    ) {
        ForYouRoute(onTopicClick)
    }
}

[mustafayigitt]

It is an extension function. What if we declared directly inside the NavHost init block, should we use it like this? I don't think so. Actually, we're telling which content to show when calling this route when using the composable function. Now the content is ForYouScreen in this case. That's the main reason for my suggestion.