google / accompanist

A collection of extension libraries for Jetpack Compose
https://google.github.io/accompanist
Apache License 2.0
7.46k stars 599 forks source link

[Idea] Add parameter to disableclose by swiping down on BottomSheetNavigator by default #1142

Closed TanoAirthings closed 8 months ago

TanoAirthings commented 2 years ago

Is your feature request related to a problem? Please describe. My purpose is to avoid the bottomSheet to be closed by dragging it down. Inside our BottomSheet we have a scrollable list. When the user scrolls back to the top, the component will close. We want to avoid this behaviour.

Describe the solution you'd like I'd like to dismiss the bottom sheet by just tapping outside. For this purpose I'd like to be able to use a composable function (or a parameter to pass to the constructor) to switch on or off the draggability of the bottom sheet.

Describe alternatives you've considered We tried to use a BottomSheetScaffold but we don't know where in the code should be placed. We also tried to take a look at the parameters that modifies the internal behaviour of the ModalBottomSheetLayout but the internal scaffold seems not to have the sheetGesturesEnabled or isSwipable or isDraggable boolean we were looking for.

We found a sheetGesturesEnabled inside the BottomSheetScaffold object, but, as I said, we didn't find a proper way to implement it.

Thanks for you time.

github-actions[bot] commented 2 years ago

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.

TanoAirthings commented 2 years ago

@jossiwolf We're still waiting for info

agent10 commented 2 years ago

I have the same issue. Swiping down is too sensitive and closes bottom sheet almost instantly if there is scrollable content inside. But I would prefer to have exposed param per-destination for that, not the global one.

github-actions[bot] commented 2 years ago

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.

agent10 commented 2 years ago

@jossiwolf It is still the issue.

TanoAirthings commented 2 years ago

@jbw0033 @jossiwolf is it possible to have any kind of reply on this?

MarcLFJ commented 2 years ago

We need this feature as well. 👍

lilemma commented 2 years ago

I think it's an important feature to be able to prevent a user from dragging down or dismissing a bottom sheet. It's possible to disable the dragging/dismissing in SWIFT for iOS, and the feature should also exist for Jetpack Compose.

We need the functionality for the project I am working on as well.

riegersan commented 2 years ago

Same for me

riegersan commented 2 years ago

My Team and I have found a workaround.

We have set a Box around our bottom sheet and added this to the modifier.

.pointerInput(Unit) { awaitPointerEventScope { // we should wait for all new pointer events while (true) { awaitPointerEvent(pass = PointerEventPass.Initial) .changes .forEach(PointerInputChange::consumePositionChange) } } },

To prevent a closing of the sheet when the user presses above the sheet we set the hight to full screen hight.

I know, it's a ugly workaround but the best we have found currently.

lilemma commented 2 years ago

My Team and I have found a workaround.

We have set a Box around our bottom sheet and added this to the modifier.

.pointerInput(Unit) { awaitPointerEventScope { // we should wait for all new pointer events while (true) { awaitPointerEvent(pass = PointerEventPass.Initial) .changes .forEach(PointerInputChange::consumePositionChange) } } },

To prevent a closing of the sheet when the user presses above the sheet we set the hight to full screen hight.

I know, it's a ugly workaround but the best we have found currently.

I can't get your code to work unfortunately.

Where would you put that code, if you're using the NavGraphBuilder.bottomSheet? A box around it is not possible, since the NavGraphBuilder is not a @Composable function.

jossiwolf commented 2 years ago

Please see https://issuetracker.google.com/issues/186669832 for the upstream issue. Unfortunately, this is blocked until we have it upstream. We will be working on this in the near future, but there are some other dependencies we need to resolve first. @riegersan's workaround should work for touch input, but doesn't consider a11y (that'd be a lot to ask for a workaround!). Sorry about the delay everyone, I too wish we could speed it up :)

trod-123 commented 1 year ago

My Team and I have found a workaround.

We have set a Box around our bottom sheet and added this to the modifier.

.pointerInput(Unit) { awaitPointerEventScope { // we should wait for all new pointer events while (true) { awaitPointerEvent(pass = PointerEventPass.Initial) .changes .forEach(PointerInputChange::consumePositionChange) } } },

To prevent a closing of the sheet when the user presses above the sheet we set the hight to full screen hight.

I know, it's a ugly workaround but the best we have found currently.

Just a heads up, this workaround disables all interactive content hosted in your sheet as well. So for example, if your bottom sheet has buttons, then a user won't be able to tap them.

Monabr commented 1 year ago

Same for me. Really need it. Is there any ideas for temporary solution for this problem?

Monabr commented 1 year ago

I found this code. Maybe we could modify this code so it enables inner scrolling but still disables bottom sheet scrolling?

fun Modifier.verticalScrollDisabled() =
    pointerInput(Unit) {
        awaitPointerEventScope {
            while (true) {
                awaitPointerEvent(pass = PointerEventPass.Initial).changes.forEach {
                    val offset = it.positionChange()
                    if (abs(offset.y) > 0f) {
                        it.consume()
                    }
                }
            }
        }
    }
Monabr commented 1 year ago

@jossiwolf this repo https://github.com/SmartToolFactory/Jetpack-Compose-Tutorials in screen 2-10-1 https://github.com/SmartToolFactory/Jetpack-Compose-Tutorials#2-10-1-bottomsheet does not have this problem. After content started scrolled and scrolled back to the top - bottom sheet does not collapse.

reminmax commented 1 year ago

Hi everyone. Here is my solution (use confirmStateChange callback). I hope this will be useful to someone.

 val scaffoldState = rememberBottomSheetScaffoldState(
        bottomSheetState = rememberBottomSheetState(
            initialValue = BottomSheetValue.Collapsed,
            confirmStateChange = {
                // Prevent collapsing by swipe down gesture
                it != BottomSheetValue.Collapsed
            }
        ),
        snackbarHostState = remember { SnackbarHostState() }
    )
lilemma commented 1 year ago

Hello @jossiwolf ,

I hope you're doing well. I wanted to bring up a concern about the bottom sheets functionality. From my perspective, it feels like the prioritization of this feature is lagging behind, and it's important to have it available. I'm wondering if you're overloaded with work or if there are other factors that are preventing progress on this issue.

As a developer relying on accompanist, it's frustrating to see multiple issues and requested features for bottom sheets with no progress or indication of when they will be looked into. I believe that many others feel the same way.

With that in mind, I would like to include some other contributors in this comment so that we can amplify our voices and work together to ensure that this feature receives the attention it deserves. Thank you for your time. @chrisbanes @bentrengrove @JoseAlcerreca @alexvanyo @manuelvicnt @nickbutcher @andkulikov @fornewid @riggaroo @svenjacobs @sampengilly @jbw0033 @CaptnBlubber

bentrengrove commented 1 year ago

As Jossi mentioned above, we are waiting on a proper API for this to be added to Compose upstream. Work on Accompanist is done on a best effort basis and work on the main Compose libraries comes first. We happily accept contributions if someone else can implement this.

Mass tagging contributors as above is not appropriate and could lead to being banned from commenting in the future.

jossiwolf commented 1 year ago

In addition to Ben's comment, we are not only happy to accept contributions for Accompanist but also accept contributions to Compose as it is a part of the Android Open Source Project.

There are three work items needed for this issue:

  1. Add the parameter upstream
  2. Figure out per-destination flags like skipHalfExpanded and gesturesEnabled, which needs careful architecting because of recomposition timing
  3. Implement the gesturesEnabled flag

Feel free to ping us directly on channels like KotlinLang if you are interested in contributing this upstream and here!

chrisbanes commented 1 year ago

I don't agree with the mass tagging, but the 'best effort' stuff is a bit of a cop-out. Either the library is supported or not. If it's not supported, it should be deprecated as such.

bentrengrove commented 1 year ago

Best effort may not have been the right term here. Accompanist is actively supported, in this case we have just had to prioritise other work first for bottom sheets in the main Compose libraries. This issue is blocked until we can get that work done upstream.

DaveSeverns commented 1 year ago

Hi everyone. Here is my solution (use confirmStateChange callback). I hope this will be useful to someone.

 val scaffoldState = rememberBottomSheetScaffoldState(
        bottomSheetState = rememberBottomSheetState(
            initialValue = BottomSheetValue.Collapsed,
            confirmStateChange = {
                // Prevent collapsing by swipe down gesture
                it != BottomSheetValue.Collapsed
            }
        ),
        snackbarHostState = remember { SnackbarHostState() }
    )

@reminmax Just to confirm, with this setting the sheet will still "slide" down but then return to the top position? Thats what I see, it works but a little more janky than the intention

reminmax commented 1 year ago

Hi everyone. Here is my solution (use confirmStateChange callback). I hope this will be useful to someone.

 val scaffoldState = rememberBottomSheetScaffoldState(
        bottomSheetState = rememberBottomSheetState(
            initialValue = BottomSheetValue.Collapsed,
            confirmStateChange = {
                // Prevent collapsing by swipe down gesture
                it != BottomSheetValue.Collapsed
            }
        ),
        snackbarHostState = remember { SnackbarHostState() }
    )

@reminmax Just to confirm, with this setting the sheet will still "slide" down but then return to the top position? Thats what I see, it works but a little more janky than the intention

Yes. The sheet will still "slide" down but then return to the top position. In other words, this prevents closing the BottomSheet.

UKMIITB commented 1 year ago

@jossiwolf The upstream issue is marked as fixed https://github.com/google/accompanist/issues/1142

Is it possible to relook into having it for bottom sheet navigator now

UKMIITB commented 1 year ago

@jossiwolf Any update here

sidbamba commented 1 year ago

@jossiwolf any updates?

DikenMaharjan commented 1 year ago

any updates?

StringMeUp commented 12 months ago

Updates?

DoraswamyVamsi commented 11 months ago

They apparently solved the issue, but I didn't understand the solution. Anyone care to explain? https://issuetracker.google.com/issues/215403277#comment11

ianhanniballake commented 8 months ago

With the release of Compose Material 1.7.0-alpha04, the Material team has added a new artifact: androidx.compose.material:material-navigation, which fully replaces Accompanist Navigation Material.

As such, we are closing all issues here on Accompanist and will be fully deprecating Accompanist Navigation Material in an upcoming release.

This was not fixed as part of the migration, so if you're still interested in this, please file an issue and the Material team will take a look.

Include the link here so that others can star it!