google / accompanist

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

[Navigation Material] ModalBottomSheetState offset #1582

Closed fedor7peaks closed 1 year ago

fedor7peaks commented 1 year ago

For some of the requirements it's necessary to place an sticky button on the ModalBottomSheet, however the ability to get the offset has been removed, and it seems there is no way to get the offset. Using onGloballyPositioned is not helping as it doesn't fire off all the time and the sticky button jumps. This is the required parameter:

@Deprecated(
        message = "BottomSheetNavigatorSheetState#offset has been removed",
        level = DeprecationLevel.ERROR
    )
    val offset: State<Float>
        get() = error("BottomSheetNavigatorSheetState#offset has been removed")

If has been deprecated but no other solution is offered. This is quite important feature, so I think it should be added back.

oOJohn6Oo commented 1 year ago

I need this value too. I think the difference between ModalBottomSheetState and BottomSheetState is that ModalBottomSheetState has another Hidden state, other functions are similar.

But why remove the offset from ModalBottomSheetState but keep it in BottomSheetState ?

enum class BottomSheetValue {
    /**
     * The bottom sheet is visible, but only showing its peek height.
     */
    Collapsed,
    /**
     * The bottom sheet is visible at its maximum height.
     */
    Expanded
}

enum class ModalBottomSheetValue {
    /**
     * The bottom sheet is not visible.
     */
    Hidden,

    /**
     * The bottom sheet is visible at full height.
     */
    Expanded,

    /**
     * The bottom sheet is partially visible at 50% of the screen height. This state is only
     * enabled if the height of the bottom sheet is more than 50% of the screen height.
     */
    HalfExpanded
}
fedor7peaks commented 1 year ago

@oOJohn6Oo Nice catch! This is actually what prevents me to upgrade current project to newer Compose version, I hope there will be some updates on this.

fedor7peaks commented 1 year ago

@oOJohn6Oo @jossiwolf I'm not exactly sure but in the newer version of Material ModalBottomSheetState, we can have same functionality using anchoredDraggableState? However, it's not possible to test this suggestion as it's internal in ModalBottomSheetState. I've opened the bug ticket in IssueTracker to make it accessible: https://issuetracker.google.com/issues/284325735

fedor7peaks commented 1 year ago

@oOJohn6Oo @jossiwolf Seems since there is no update from Google, I've made a very ugly hack to make offset work, since I cannot keep outdated version in my project:

@Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
@OptIn(ExperimentalMaterialNavigationApi::class, ExperimentalMaterialApi::class)
fun BottomSheetNavigatorSheetState.getOffset() = sheetState.requireOffset()

Hope this helps people, who urgently need this for their project.

fedor7peaks commented 1 year ago

Looks like this has been fixed in latest release of Compose Material 1.6.0-alpha01: https://developer.android.com/jetpack/androidx/releases/compose-material#1.6.0-alpha01 Any plans on updating it in Accompanist library? There is only need to expose progress as per my understanding:

    /**
     * The fraction of the progress, within [0f..1f] bounds, or 1f if the [AnchoredDraggableState]
     * is in a settled state.
     */
    @get:FloatRange(from = 0.0, to = 1.0)
    @ExperimentalMaterialApi
    val progress: Float
        get() = anchoredDraggableState.progress
joharei commented 1 year ago

@fedor7peaks while it certainly helps that they expose the progress, it's a shame that they didn't also expose the offset.

My use case is that I want to morph the sheet into a full screen page when the sheet is nearing the top of the screen, taking into account insets. I don't think I can do this with the progress alone. For now I'm accessing the internal anchoredDraggableState with suppressions as per your previous comment.

oOJohn6Oo commented 1 year ago

@fedor7peaks Good to hear that.

@joharei I'm currently using reflection to get the offset like this, works fine for me.

private fun ModalBottomSheetState.safeRequireOffset(): Float {
    return try {
        val tempField = requireOffsetField ?: return Float.MAX_VALUE
        return tempField.invoke(this) as Float
    } catch (e: Exception) {
        Float.MAX_VALUE
    }
}

private val requireOffsetField by lazy {
    try {
        ModalBottomSheetState::class.java.getDeclaredMethod("requireOffset\$material_release").apply {
            isAccessible = true
        }
    } catch (e: Exception) {
        null
    }
}
fedor7peaks commented 1 year ago

@joharei @oOJohn6Oo True. I tried to use progress, but there is just too much calculations needed sadly, I was able to make smooth corner transitions from HalfExpanded to Expanded states:

  val cornerMultiplier = remember(sheetState.currentValue, sheetState.targetValue, sheetState.progress) {
        when(sheetState.currentValue to sheetState.targetValue) {
            HalfExpanded to Expanded -> (1f - sheetState.progress)
            Expanded to HalfExpanded -> sheetState.progress
            Expanded to Expanded -> 0f
            else -> 1f
        }
    }

However, I didn't even start calculations to transform it to offset - it doesn't make sense to do all the calculations just to get it, since they can expose it directly. This is the solution I use as of now:

@Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
@OptIn(ExperimentalMaterialNavigationApi::class, ExperimentalMaterialApi::class)
fun BottomSheetNavigatorSheetState.requireOffset() = try {
    sheetState.requireOffset()
} catch (e: IllegalStateException) {
    0f
}

I'm wondering will there ever any update on this, because all the ways seem not ideal.

joharei commented 1 year ago
Here is my full solution ```kt @ExperimentalMaterialNavigationApi @OptIn(ExperimentalMaterialApi::class) @Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE") @Composable fun ModalBottomSheetLayout( bottomSheetNavigator: BottomSheetNavigator, modifier: Modifier = Modifier, sheetShape: CornerBasedShape = MaterialTheme.shapes.large.copy( bottomStart = ZeroCornerSize, bottomEnd = ZeroCornerSize, ), content: @Composable () -> Unit, ) { val density = LocalDensity.current val configuration = LocalConfiguration.current val statusBarHeight = WindowInsets.statusBars.getTop(density) val offset = bottomSheetNavigator.navigatorSheetState.sheetState.anchoredDraggableState.offset val spacerHeight = remember(statusBarHeight, offset) { if (offset > statusBarHeight * 2) 0f else maxOf(0f, statusBarHeight - offset / 2) } val screenWidth = with(density) { configuration.screenWidthDp.dp.roundToPx() } var sheetWidth by remember { mutableIntStateOf(0) } val fullWidth = abs(screenWidth - sheetWidth) < 2 val progress = spacerHeight / statusBarHeight val updatedSheetShape = if (fullWidth) { fun CornerSize.calculateCornerSize(): CornerSize { return CornerSize( ((1f - progress) * toPx(Size(sheetWidth.toFloat(), sheetWidth.toFloat()), density)) .takeIf { it.isFinite() } ?: return this, ) } sheetShape.copy( topStart = sheetShape.topStart.calculateCornerSize(), topEnd = sheetShape.topEnd.calculateCornerSize(), ) } else { sheetShape } if (fullWidth) { val controller = rememberSystemUiController() val passedThreshold = progress > 0.75 DisposableEffect(controller, passedThreshold) { controller.setStatusBarColor(color = Color.Transparent, darkIcons = passedThreshold) onDispose { controller.setStatusBarColor(color = Color.Transparent, darkIcons = false) } } } MaterialModalBottomSheetLayout( sheetState = bottomSheetNavigator.sheetState, sheetContent = { Spacer( Modifier .height(with(density) { spacerHeight.toDp() }) .fillMaxWidth() .onSizeChanged { sheetWidth = it.width }, ) bottomSheetNavigator.sheetContent(this) }, modifier = modifier, sheetShape = updatedSheetShape, content = content, ) } ```

I wanted to use derivedStateOf in there, but it did not work with the offset. I suspect the state observability is getting lost perhaps because of the internal property.

There are probably other performance problems here as well, but it looks great!

github-actions[bot] commented 1 year 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.

fedor7peaks commented 1 year ago

Not stale.

ianhanniballake commented 1 year ago

We'll track progress in the earlier filed issue #1472

rcht007 commented 7 months ago

@Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
var offset = Float.POSITIVE_INFINITY try { offset = sheetState.requireOffset() } catch (e: IllegalStateException) { log.d("MODAL BOTTOM SHEET", "${e.printStackTrace()}") } // offset is 1568.9 val outside = it.y < offset this works for me