BottomSheetState progress bug [276776071]

Fixed

Bug

Status Update

No update yet.

Description

an...@gmail.com

created issue #1

Apr 4, 2023 11:05AM

Jetpack Compose version: androidx.compose:compose-bom:2023.03.00

Jetpack Compose component used: BottomSheetState

Kotlin version: 1.8.10

I noticed a bug inside the implementation of BottomSheetState. I tried calculating the absolute fractional progress of the BottomSheetState. The desired result would be a function that returns 1f if the BottomSheet is expanded and 0 if it is Hidden, so I could use the BottomSheet in combination with a MotionLayout.

@OptIn(ExperimentalMaterialApi::class)
val BottomSheetScaffoldState.currentFraction: Float
    get() {
        return if (bottomSheetState.currentValue == BottomSheetValue.Expanded && bottomSheetState.progress == 1f) {
            1f
        } else if (bottomSheetState.currentValue == BottomSheetValue.Collapsed && bottomSheetState.progress == 1f) {
            0f
        } else if (bottomSheetState.currentValue == BottomSheetValue.Expanded) {
            1 - bottomSheetState.progress
        } else {
            bottomSheetState.progress
        }
    }

This worked fine, except that the value quickly jumps from 0 to 1 when swiping down and from 1 to 0 when swiping up, which leads to a broken animation in the MotionLayout. So I started printing some outputs.

For generating the output I used:

println("state: ${bottomSheetState.currentValue}, progress: ${bottomSheetState.progress}")

When I swipe up:

I/System.out: state: Collapsed, progress: 0.99963176
I/System.out: state: Collapsed, progress: 0.9997223
I/System.out: state: Collapsed, progress: 0.99979806
I/System.out: state: Collapsed, progress: 0.99984795
I/System.out: state: Collapsed, progress: 0.9998856
I/System.out: state: Collapsed, progress: 0.999917
I/System.out: state: Collapsed, progress: 0.9999376
I/System.out: state: Collapsed, progress: 0.99995315
I/System.out: state: Collapsed, progress: 0.9999661
I/System.out: state: Collapsed, progress: 0.99997455
I/System.out: state: Collapsed, progress: 0.9999809
I/System.out: state: Collapsed, progress: 0.99998623
I/System.out: state: Collapsed, progress: 0.9999897
I/System.out: state: Collapsed, progress: 0.99999225
I/System.out: state: Collapsed, progress: 0.9999944
I/System.out: state: Collapsed, progress: 1.0 <-Remember this one here
I/System.out: state: Expanded, progress: 1.0 <-And this one here

When I swipe down:

I/System.out: state: Expanded, progress: 0.9997586
I/System.out: state: Expanded, progress: 0.9998182
I/System.out: state: Expanded, progress: 0.99986315
I/System.out: state: Expanded, progress: 0.99990064
I/System.out: state: Expanded, progress: 0.9999253
I/System.out: state: Expanded, progress: 0.9999439
I/System.out: state: Expanded, progress: 0.9999594
I/System.out: state: Expanded, progress: 0.99996954
I/System.out: state: Expanded, progress: 0.9999772
I/System.out: state: Expanded, progress: 0.99998343
I/System.out: state: Expanded, progress: 0.99998766
I/System.out: state: Expanded, progress: 0.9999907
I/System.out: state: Expanded, progress: 0.99999326
I/System.out: state: Expanded, progress: 1.0 <- And now this value
I/System.out: state: Collapsed, progress: 1.0 <- And this one

There seems to be no possibility to differenciate between state: Collapsed, progress: 1.0 when swiping up and state: Collapsed, progress: 1.0 while swiping down, which then leads to the broken animation.

In my opinion, the second last rows of the outputs are not correct and should already return the new state.

If this is intended behaviour of course, I would be happy for some feedback on how to get the absolute progress fraction of the Bottom Sheet

Kind regards

Comments

jo...@google.com <jo...@google.com> Apr 4, 2023 11:30AM

Reassigned to jo...@google.com.

ka...@yahooinc.com <ka...@yahooinc.com> #2Apr 27, 2023 09:25AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

Message last modified on Apr 27, 2023 09:26AM

[Deleted User] <[Deleted User]> #3May 16, 2023 12:48AM

Hello, is there a release version with this issue fixed?

jo...@google.com <jo...@google.com> #4May 16, 2023 07:01AM

Hi, the there is no release containing a fix yet. We will update the issue when the fix has been merged and when it has been released.

Message last modified on May 16, 2023 07:02AM

th...@neugelb.com <th...@neugelb.com> #5May 22, 2023 12:17AM

Comment has been deleted.

Message last modified on May 22, 2023 11:21AM

ap...@google.com <ap...@google.com> #6May 24, 2023 10:57AM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 762bbc3cea2a9bcfb6fae34e71c6bb8da917f43c
Author: Jossi Wolf <jossiwolf@google.com>
Date: Mon May 15 11:50:25 2023

[M2] Expose AnchoredDraggable components' progress

Fixes: 271169225
Fixes: 276375124
Fixes: 276776071
Fixes: 270066861
Relnote: BottomSheetState, ModalBottomSheetState and BottomDrawerState now expose a progress property indicating the progress between the current (settled) anchor and the closest anchor in the swipe direction.
Test: AnchoredDraggable tests

Change-Id: I1b31727b4f56df890e336bea9f4de526733aaacc

M compose/material/material/api/current.txt
M compose/material/material/api/restricted_current.txt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/AnchoredDraggable.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/BottomSheetScaffold.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/Drawer.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/ModalBottomSheet.kt

https://android-review.googlesource.com/2588925

mi...@gmail.com <mi...@gmail.com> #7Jun 13, 2023 04:04PM

Hello, that commit doesn't resolve the issue because that is exposing only the progress between states. Also, the state doesn't change immediately when it hits 1, as we can see above. Before it was easy to determine because the target state was exposed. Is there any reason why the library can expose the targetState from SwipeableV2State class?

ri...@gmail.com <ri...@gmail.com> #8Jun 16, 2023 07:24PM

Has this fix been included in any release? I'm on 1.5.0-beta02 and I don't see the targetValue being exposed in BottomSheetScaffold

jo...@google.com <jo...@google.com> #9Jun 19, 2023 12:50PM

#7, correct, this commit exposes the the fraction of the current offset in the distance of the current transition. You can access the currentValue and targetValue though to determine whether the states.

Also, the state doesn't change immediately when it hits 1, as we can see above. What do you mean?

#8, this fix will be included in the next release, happening soon.

pr...@google.com <pr...@google.com> #10Jun 21, 2023 08:57PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.material:material:1.6.0-alpha01
androidx.compose.material:material-android:1.6.0-alpha01

ju...@google.com <ju...@google.com> #11Jun 28, 2023 11:52PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.material:material:1.5.0-beta03
androidx.compose.material:material-android:1.5.0-beta03

na...@google.com <na...@google.com> #12Jul 26, 2023 05:21PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.wear.compose:compose-material-core:1.2.0-rc01
androidx.wear.compose:compose-ui-tooling:1.2.0-rc01

Issue 276776071

Description

Issue summary

Comments

jo...@google.com <jo...@google.com> Apr 4, 2023 11:30AM

ka...@yahooinc.com <ka...@yahooinc.com> #2Apr 27, 2023 09:25AM

[Deleted User] <[Deleted User]> #3May 16, 2023 12:48AM

jo...@google.com <jo...@google.com> #4May 16, 2023 07:01AM

th...@neugelb.com <th...@neugelb.com> #5May 22, 2023 12:17AM

ap...@google.com <ap...@google.com> #6May 24, 2023 10:57AM

mi...@gmail.com <mi...@gmail.com> #7Jun 13, 2023 04:04PM

ri...@gmail.com <ri...@gmail.com> #8Jun 16, 2023 07:24PM

jo...@google.com <jo...@google.com> #9Jun 19, 2023 12:50PM

pr...@google.com <pr...@google.com> #10Jun 21, 2023 08:57PM

ju...@google.com <ju...@google.com> #11Jun 28, 2023 11:52PM

na...@google.com <na...@google.com> #12Jul 26, 2023 05:21PM

Add comment

Issue metadata