Hello, I have a design question. We have an app wh...

# compose

[Lisandro Di Meo]

Hello, I have a design question. We have an app where first the following pattern seem to work. But now, we have a lot of screens, not all of them uses scaffold stuff, and we are moving states from everywhere

Scaffold(/*...*/){
   NavGraph(/*...*/)
}

Shouldn't it be better if we face it with a different approach, something like:

NavGraph(/*...*/){
   composable(A){ Scaffold(...) }
   composable(B){ ModalBottomSheet(...) }
   ...
}

Also by using the previous approach, we shouldn't have to move the scaffold state from one place to other, and each screen will know its own traits.

[dorche]

What do you use the Scaffold for exactly - which slots do you use?

[Lisandro Di Meo]

ModalBottomSheetLayout(
    sheetState = sheetState,
    sheetShape = RoundedCornerShape(topStart = Dimen16dp, topEnd = Dimen16dp),
    sheetContent = {
        ModalBottomSheet(
            coroutine = coroutineScope,
            sheetState = sheetState,
            isExpanded = true,
            header = bottomSheetState.value?.header,
            body = bottomSheetState.value?.body,
            footer = bottomSheetState.value?.footer
        )
    }
) {

    Scaffold(
        scaffoldState = scaffoldState,
        snackbarHost = {
            SnackbarHost(hostState = snackbarHostState) { data ->
                Snackbar(actionColor = Color(0xFFBB86FC), snackbarData = data)
            }
        },
        topBar = {
            if (appBarState.value?.enabled == true) {
                TopAppBar(appBarViewModel)
            }
        },
        drawerContent = if (drawer.drawerState.value == null) {
            null
        } else {
            { drawer.drawerState.value?.let { drawer(drawerState = it) } }
        },
        bottomBar = {
            BottomNavigation(
                navHostController = navController,
                viewModel = appBarViewModel,
                dashboardNavigationItems
            )
        },
        floatingActionButton = {
            if (barsState.value!!.isVisible) {
                FloatingActionButton(
                    shape = CircleShape,
                    contentColor = Color.White,
                    onClick = {
                        navController.navigate(...) {
                            popUpTo(...) {
                                saveState = true
                            }
                            launchSingleTop = true
                            restoreState = true
                        }
                    },
                    backgroundColor = Mirage
                ) {
                    Icon(painterResource(id = DashboardNavigationItem.QR.iconId), "QR")
                }
            }
        },
        floatingActionButtonPosition = FabPosition.Center,
        isFloatingActionButtonDocked = true,
        content = {
            Column(modifier = Modifier.padding(it)) {
                Navigation(
                    navController,
                    snackbarHelper,
                    appBarViewModel,
                    accountCreationViewModel,
                    newPasswordViewModel,
                    dashboardViewModel,
                    accessMenuViewModel,
                    bottomSheetViewModel,
                    qrViewModel,
                    drawer,
                    analytics,
                    onLoginSucceed = {
                        expirationPolicy.attachToLifecycle()
                    },
                    onLogOutSucceed = {
                        expirationPolicy.detachFromLifecycle()
                    },
                    onFinish = onFinish
                )
            }
        }
    )
}

[Lisandro Di Meo]

Sorry for the long chunk of code, but the point is that I think this approach isn't very scalable

[Lisandro Di Meo]

Ignore the viewmodels, since them can be moved out of the Navigation easily

[dorche]

Right okay you're using quite a bit - snackbar & fab means you can't split this into 2 nested Scaffolds really (one at top level and one at screen level)..
So in this case why do you think repeating the scaffold is any more maintainable? Is it hiding and showing specific slots that bugs you?

[Lisandro Di Meo]

What I fell like an smell is maintaining states like sheetState all over the app, while I believe it could be better to only use it on the set of screens that actually need a BottomSheet?
For instance, if a sheet need to change its contents, i need to emit a state change that updates the sheetcontent

[dorche]

In this case I agree with you - does that change have an impact on your UI while doing a nav animation?

Getting rid of the bottomSheet slot there and defining it at screen level will have impact on UI too

[Lisandro Di Meo]

Yeah, deff during recomposition a resize effect happens 😕

[dorche]

So could you benefit from a codegen nav library that will let you define properties for your now strongly typed destinations?
This will let you do things like

val showFab = when destination {
  ScreenA -> true
  ScreenB -> false
}

So in some basic scenarios you won't need to tell it to hide/show all the time. Hard to say if that's useful without knowing your app.


What do you use the bottomSheet for? Could you move it into a nested Scaffold defined per screen?

[Lisandro Di Meo]

(sorry for the late reply) I was wondering to doing something like that. Since the Sheet content is variable through the app, I could do:

val showSheet = when destination ...

And each destination that has to show the bottom sheet, should emit its desired content like a callback

composable(A) {
Column(...){
// ScreenA
}
sheetContent = {
SheetContentForA()
}
}

[dorche]

That still won’t be particularly nice if you have a lot of Screen->Sheet interactions. If that’s the case I’d still explore if having this would better fit your case:

Scaffold(
// dont set sheet here
){
   NavGraph(/*...*/){
     composable(A){
       ScreenA()
     }
     composable(B){ ScreenB() }
     ...
  }
}

@Composable
fun ScerenA() {
  Scaffold(
    // sheet here so now it's part of the "screen/destination"
  ) {
  }
}

[Lisandro Di Meo]

mm shouldn’t using a nested scaffold conflict with things like the topbar? or even having multiple drawers 🤔

[dorche]

You can just not use the slots for these, only provide Sheet at the 2nd level

[Lisandro Di Meo]

great 👍