AnchoredDraggable doesn't snap to closest anchor when released with 0 velocity [318707189]

Fixed

Bug

Status Update

No update yet.

Description

ro...@t-online.de

created issue #1

Jan 5, 2024 08:18AM

Jetpack Compose version: 1.6.0-beta03 Jetpack Compose component used: AnchoredDraggable

Steps to Reproduce:

Create an AchoredDraggable with at least 3 anchors
Set the positionalThreshold to { it * 0.8f } and space the anchors evenly, or set it to { it * 0.5f } and space the first two anchors relatively close
Now, when swiping from the first anchor a little bit further than the second anchor, the positional threshold to the third anchor will not have been reached, so the AnchoredDraggble doesn't settle on the third anchor. Expected behavior: as the positionalThreshold to the second anchor is already reached ant this is closest, it should snap to the second anchor. Actual Behavior: it snaps back to the first anchor.

If this is intended, I personally find it unintuitive and there should at least be an option to change the behavior.

Comments

as...@google.com <as...@google.com> Jan 5, 2024 08:18AM

Reassigned to jo...@google.com.

jo...@google.com <jo...@google.com> #2Jan 5, 2024 11:00AM

Reassigned to sh...@google.com.

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

sh...@google.com <sh...@google.com> #3Jan 8, 2024 09:30AM

Reassigned to jo...@google.com.

Thanks for reporting! I was able to reproduce this issue and the behavior is not the intended one.

Taking a look at the code, it seems that when calling for computeTarget inside the settle method, we pass the currentValue and currently it is the anchor we started the drag with. This value doesn't get updated when we pass another anchor in the same drag. There is a fix possible that we can do inside computeTarget method, but I think we can investigate updating the currentValue before passing it to computeTarget method.

Jossi, do you mind taking a look if we can do that?

jo...@google.com <jo...@google.com> #4Jan 18, 2024 03:11PM

This requires some deeper considerations w.r.t. how AnchoredDraggable works, specifically if we want to update currentValue mid-drag. We will investigate this, but it is not a priority at the moment. Thank you for your patience!

jo...@google.com <jo...@google.com> Mar 4, 2024 04:26PM

Accepted by jo...@google.com.

ap...@google.com <ap...@google.com> #5Mar 5, 2024 12:30PM

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

commit ea1c7b82abfbd75b1396b1113852253e7b7dc444
Author: Jossi Wolf <jossiwolf@google.com>
Date: Sun Jan 28 12:29:03 2024

Changes AnchoredDraggable to operate based on small sliding window

Previously, AnchoredDraggable would operate based on `currentValue`, which could be several anchor points away from the current position of the offset. This made exposing APIs reflecting offset fractions tricky. The new behavior is aligned with Pager and subsequently allows us to expose a `currentValueOffsetFraction`.

Relnote: AnchoredDraggable's currentValue will now update when passing through an anchor point. Use settledValue to receive the previous currentValue semantics, only updating when settling at an anchor. The progress is now exposed as a function (requiring a starting and end point) instead of a property.
Test: Existing & refactored tests
Bug: 318707189, 298271489, 294991954
Change-Id: Ibe6e88f172b099e8f1f841722946471e4641f999

M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/AnchoredDraggableDemo.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/AnchoredDraggableSample.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/anchoredDraggable/AnchoredDraggableGestureTest.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/anchoredDraggable/AnchoredDraggableStateTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/AnchoredDraggable.kt

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

ap...@google.com <ap...@google.com> #6Mar 8, 2024 01:20PM

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

commit 5c108158e6cf2e03dcd4523e9fcf4230514e8a58
Author: Jossi Wolf <jossiwolf@google.com>
Date: Tue Mar 05 17:34:38 2024

Revert "Revert "Changes AnchoredDraggable to operate based on sm..."

Revert submission 2988649-revert-2930845-DMIZVAQUBO

Reason for revert: Re-landing since this doesn't include the API changes

Reverted changes: /q/submissionid:2988649-revert-2930845-DMIZVAQUBO

Relnote: AnchoredDraggable's currentValue will now update when passing through an anchor point. Use settledValue to receive the previous currentValue semantics, only updating when settling at an anchor. The progress is now exposed as a function (requiring a starting and end point) instead of a property.
Test: Existing & refactored tests
Bug: 318707189, 298271489, 294991954

Change-Id: I2ebd3677fd98f0d14f5d365a901cd1b36b0ce9e2

M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/AnchoredDraggableDemo.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/AnchoredDraggableSample.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/anchoredDraggable/AnchoredDraggableGestureTest.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/anchoredDraggable/AnchoredDraggableStateTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/AnchoredDraggable.kt

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

jo...@google.com <jo...@google.com> Mar 21, 2024 04:06PM

Marked as fixed.