accompanist icon indicating copy to clipboard operation
accompanist copied to clipboard

[Idea] Add ability to fully expanded BottomSheetNavigator by default

Open jgavazzisp opened this issue 2 years ago • 44 comments

Is your feature request related to a problem? Please describe. I'm trying to migrate some bottom sheets from "ScafoldBottomSheet" to the new Navigation Compose Material, however, I cannot find a way to start the sheet as fully expand — similar to the behaviour of ScafoldBottomSheet —, so it always starts as HalfExpanded which is not ideal for some use cases.

Describe the solution you'd like The solution is to lock the expansion to Expanded and skip HalfExpanded when the size of the sheet content is over half of the screen height.

Describe alternatives you've considered The class "SheetContentHost" contains a method called "ModalBottomSheetState.internalShow()" which could control if the initial expansion is the default behaviour (half expanded if the sheet is over half of the screen height) or it should lock to the full height of the content.

The "rememberBottomSheetNavigator" could receive the extra parameter controlling the expansion and pass it down to the SheetContentHost when appropriate.

Thanks

jgavazzisp avatar Aug 17 '21 07:08 jgavazzisp

@jossiwolf

chrisbanes avatar Aug 17 '21 14:08 chrisbanes

@jgavazzisp Thanks for the request! This is currently not supported by Compose Material's ModalBottomSheetLayout either and I would not like Navigation Material's ModalBottomSheetLayout to diverge from the actual implementation.

See https://issuetracker.google.com/issues/186669820 for a feature request for Compose Material's implementation; as soon as it's available there we will add it here as well.

Fwiw, Navigation Material provides support for modal bottom sheets. Compose Material's BottomSheetScaffold provides an implementation for standard bottom sheets which have different usages/behaviors.

Standard bottom sheets remain on-screen when a user interacts with the main UI region or the sheet itself.

This would mean you'd have two destinations visible at the same time, so modal bottom sheets might not be exactly what you're looking for.

jossiwolf avatar Aug 17 '21 16:08 jossiwolf

hi @jossiwolf thanks for your reply. I will my eyes on the 186669820 in the meantime.

jgavazzisp avatar Aug 18 '21 06:08 jgavazzisp

Looking at material docs It looks like this should be supported. https://material.io/components/sheets-bottom#expanding-bottom-sheet

There are times where it's applicable to animate BottomSheet to full screen and let user swipe it down.

I thought I was doing something wrong, when I created view with Modifier.fillMaxHeight(0.4f) and everything was ok, but after adding 0.9f fraction only half was visible:)

miszmaniac avatar Sep 06 '21 11:09 miszmaniac

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Oct 07 '21 03:10 github-actions[bot]

Remove stale.

tordjon avatar Oct 07 '21 12:10 tordjon

Temporary workaround:

@OptIn(ExperimentalMaterialApi::class, ExperimentalMaterialNavigationApi::class, InternalCoroutinesApi::class)
@Composable
fun rememberBottomSheetNavigator(
    animationSpec: AnimationSpec<Float> = SwipeableDefaults.AnimationSpec,
    skipHalfExpanded: Boolean = true,
): BottomSheetNavigator {
    val sheetState = rememberModalBottomSheetState(
        ModalBottomSheetValue.Hidden,
        animationSpec
    )

    if (skipHalfExpanded) {
        LaunchedEffect(sheetState) {
            snapshotFlow { sheetState.isAnimationRunning }
                .collect {
                    with(sheetState) {
                        val isOpening = currentValue == ModalBottomSheetValue.Hidden && targetValue == ModalBottomSheetValue.HalfExpanded
                        val isClosing = currentValue == ModalBottomSheetValue.Expanded && targetValue == ModalBottomSheetValue.HalfExpanded
                        when {
                            isOpening -> animateTo(ModalBottomSheetValue.Expanded)
                            isClosing -> animateTo(ModalBottomSheetValue.Hidden)
                        }
                    }
                }
        }
    }

    return remember(sheetState) {
        BottomSheetNavigator(sheetState = sheetState)
    }
}

alashow avatar Oct 08 '21 00:10 alashow

Reopening this for folks to be able to track progress - this still depends on the upstream issue (https://issuetracker.google.com/issues/186669820) being fixed.

jossiwolf avatar Nov 08 '21 14:11 jossiwolf

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Dec 09 '21 03:12 github-actions[bot]

Now that this is implemented in Material we can start to look at this from the Accompanist perspective. @jgavazzisp, I understand your initial proposal defines skipHalfExpanded as a Navigator-level flag. A possible API shape for that could be

@Composable
fun MyApp() {
    val sheetNavigator = rememberBottomSheetNavigator(skipHalfExpanded = true)
    ...
}

What are everybody's use cases for this API? For navigation, it feels like a destination-level flag would make more sense:

@Composable
fun MyApp() {
    val bottomSheetNavigator = rememberBottomSheetNavigator()
    val navController = rememberNavController(bottomSheetNavigator)
    ModalBottomSheetLayout(bottomSheetNavigator) {
        NavHost(navController, Destinations.Home) {
           composable(route = "home") {
               ...
           }
           bottomSheet(route = "sheet", skipHalfExpanded = true) {
               Text("This is a cool bottom sheet!")
           }
        }
    }
}

To vote for a Navigator-level API, react with 🚀. To vote for a destination-level API, react with 👀.

Disclaimer: Before a bigger refactoring that is still ongoing and will take a while, it might only be possible to have a Navigator-level flag.

jossiwolf avatar Dec 15 '21 12:12 jossiwolf

@alashow not working in my case if view height is near half so animateTo(ModalBottomSheetValue.Expanded) just don't do required work :(

Monabr avatar Jan 01 '22 18:01 Monabr

I found out what was the problem in my case. After first entering in compose i trying to set the title in my view based on what i will get from database. I was using collectAsState(null) so from start there is no title and so the height of full compose is below the half of screen so it animates to less than half of screen. But during the animation i get my item from database, make recomposition, set my title (add Text()) so increase overall height of compose. But in this case it's animate only to half of the screen.

The solution in my case - add default title to have stable height of compose.

Hope it will helps for someone.

Monabr avatar Jan 01 '22 18:01 Monabr

@jossiwolf looks like navigator level api will be available first before the destination level api.

Question with that though. Do you know if we could possible use two of these navigators?

~Pseudo code

    val bottomSheetNavigator = rememberBottomSheetNavigator()
    val bottomSheetNavigatorHalf = rememberBottomSheetNavigator(skipHalfExpanded = true)
    val navController = rememberNavController(bottomSheetNavigator, bottomSheetNavigatorHalf)
    ModalBottomSheetLayout(bottomSheetNavigator) {
      ModalBottomSheetLayout(bottomSheetNavigatorHalf) {

and therefore we could have both options as we want on each destination in this workaround?

ColtonIdle avatar Jan 25 '22 19:01 ColtonIdle

@ColtonIdle No, that won't work. We're still deciding on whether to release this API with the upcoming release, but once we release any API for this, we will provide guidance and update this issue :)

jossiwolf avatar Jan 25 '22 20:01 jossiwolf

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Feb 25 '22 03:02 github-actions[bot]

What's the current state on that? Is there sth planned? In the meanwhile we are supposed to apply the workaround metionied here https://github.com/google/accompanist/issues/657#issuecomment-938249186 ?

chriswiesner avatar Mar 10 '22 16:03 chriswiesner

Hi, guys. I have a problem with the solution alashow provided. I have a button, click it and then bottom sheet appears, it works just fine. When I click this button twice and very fast, then every time after that the bottom sheet appears exactly half expanded. Have you faced this problem ?

koetjape avatar Mar 24 '22 14:03 koetjape

@chriswiesner We have some more work to do upstream before being able to tackle this issue, sadly :( For now, work on this issue is on hold. I will update when I have any updates!

@koetjape I think this falls into the category issues of known issues with bottom sheets - consider it noted and tracked. Unfortunately, I don't have a workaround for you right now.

jossiwolf avatar Mar 24 '22 15:03 jossiwolf

Hi guys ! Also interested in this feature :)

captainhaddockfr35 avatar May 13 '22 14:05 captainhaddockfr35

I accidentally found an ugly workaround to make the bottom sheet fully expand. If the content of the bottom sheet change from very small( h < halfExpanded) to full height ( h > halfExpanded), then the bottom sheet will take the full height and it will skip the halfExpanded state. It is even working with an initial height of 0.dp. Here is the snippet:

    val bottomSheetHeight = remember { mutableStateOf(0.dp) }

    LaunchedEffect(true) {
        delay(500L)
        bottomSheetHeight.value = 1000.dp
    }

    Box(modifier = Modifier.fillMaxWidth().height(bottomSheetHeight.value).background(Color.Blue))

PiotrPrus avatar May 25 '22 08:05 PiotrPrus

One more way but without explicit height values like in the solution above. This solution doesn't need an explicit height and you could use wrapContentHeight() modifier:

@Composable
fun YourBottomSheetContent() {
    var expanded by remember { mutableStateOf(false) }

    val heightModifier = if (!expanded) {
        Modifier.height(0.dp)
    } else {
        Modifier.wrapContentHeight()
    }

    LaunchedEffect(true) {
        delay(200L)
        expanded = true
    }
    
    Column(
        modifier = Modifier
            .fillMaxWidth()
            .then(heightModifier)
            ...
    ) {
        // ...
    }
}

But this solution is dirty and we are waiting for the official solution.

TemMax avatar Jun 02 '22 00:06 TemMax

Currently, I'm using @alashow 's workaround which is not ideal. Any ETA on having a destination-level implementation of this? This would be an awesome feature

Luis-Abreu avatar Jun 18 '22 16:06 Luis-Abreu

@Luis-Abreu a better solution now — just copy sources of ModalBottomSheetLayout and modify it like you want ;)

TemMax avatar Jun 18 '22 16:06 TemMax

@Luis-Abreu and @TemMax is the following solution not working?

image image image

I haven't run in any issues since it was made available (Compose 1.2.0-beta03 and accompanist 0.24.11-rc).

jgavazzisp avatar Jun 22 '22 22:06 jgavazzisp

@jgavazzisp I was still running accompanist 0.23.1. I've updated and what you posted works 👍 hopefully we get a destination level implementation soon 🙏

Luis-Abreu avatar Jun 24 '22 10:06 Luis-Abreu

@jgavazzisp maybe works. But it works on the RC version. Still waiting for a stable release.

TemMax avatar Jun 24 '22 10:06 TemMax

For anyone else coming here looking for a fix, this is yet another "temporary" solution that implements the Navigator-level API solution (just remember to set skipHalfExpanded to true):

@ExperimentalMaterialNavigationApi
@OptIn(ExperimentalMaterialApi::class)
@Composable
fun rememberBottomSheetNavigator(
    animationSpec: AnimationSpec<Float> = SwipeableDefaults.AnimationSpec,
    skipHalfExpanded: Boolean = false,
): BottomSheetNavigator {
    val sheetState = rememberModalBottomSheetState(
        ModalBottomSheetValue.Hidden,
        animationSpec,
        skipHalfExpanded,
    )
    return remember(sheetState) {
        BottomSheetNavigator(sheetState = sheetState)
    }
}

It seems to work well and it brings minimal changes to the current implementation of rememberBottomSheetNavigator.

leinardi avatar Jul 13 '22 14:07 leinardi

@leinardi Do you know how to use this solution if you have only one global ModalBottomSheetLayout wrapping the whole app? I'd like to control the skipHalfExpanded flag per each bottom sheet.

marcin-adamczewski avatar Aug 09 '22 06:08 marcin-adamczewski

Mmm, not really, for that approach a Destination-level API would fit better. All my bottom sheets have the same behavior.

leinardi avatar Aug 09 '22 07:08 leinardi

@leinardi's solution works for the time being if you have uniform behavior throughout all sheets in your app. To share a bit more context: We would need to have a property on the BottomSheetDestination and then our flow would look like this:

  1. Navigation action -> BottomSheetNavigator's back stack is updated
  2. We see that there's a new back stack entry and look at it's properties a) Did skipHalfExpanded change? b) The destination's content
  3. If skipHalfExpanded changed, we first need to recreate the ModalBottomSheetState and only compose the sheet content after .

Ideally, ModalBottomSheetLayout would officially support mutating skipHalfExpanded in a safe way. As it stands, recreating the state could mean we lose some information that we would want to preserve. Changing skipHalfExpanded also means updating the Swipeable anchors. We need to make some changes around Swipeable to support this well.

Tl;dr: This needs to be fixed upstream but is currently blocked by some other work. Follow upstream here. We're sorry for the delay but there is no way to speed it up.

jossiwolf avatar Sep 23 '22 16:09 jossiwolf