Status Update
No update yet.
Comments
au...@google.com
It would be even better if it was a required parameter so developers have to choose explicitly.
If thats not feasable, adding a lint check that warns about trying to use onDragStart without any consuming of events.
ma...@google.com
Aurimas, could you clarify what do you want to be a "required parameter" in this case? Do you mean that we should require boolean to be returned from the onDrag to explicitly consume or pass along the delta?
cc-ing Louis for the lint check feasibility
lp...@google.com
It's maybe possible to write a lint check here, but I don't think there is a way to avoid false positives, as the consumption could happen in a separate function, and I think it can be quite damaging to the developer experience when we have false positives for things like this. I agree that ideally the API would guide you down the right path, rather than you needing to know about this separate thing you have to do.
au...@google.com
Aurimas, could you clarify what do you want to be a "required parameter" in this case? Do you mean that we should require boolean to be returned from the onDrag to explicitly consume or pass along the delta?
something along those lines. Either return on onDrag. Or the onDrag lambda having OnShouldConsumeCallback as another parameter, or detectDragGestures has another parameter of should consume callback.
I am not sure what the right suggestion is given I have limited context on the API design, but as a user I see it as a big foot-gun.
ma...@google.com
Makes sense, thanks for the context. I'll take a loot into how we can improve this besides docs.
ap...@google.com
Branch: androidx-main
commit b1b6e7ab35e639fa74efeecabb31e496a130cc3e
Author: Matvei Malkov <malkov@google.com>
Date: Wed May 19 19:01:23 2021
Make detectDragGestures, detectVerticalGestures and detectHorizontaGestures to consume position change automatically.
This will allow this functions to fulfill the ordering contract when the start should be called before first onDrag. Also this removed the implicit contract where we people have to consume explicitly, making this high level functions easier to use.
Fixes: 185096350
Fixes: 187320697
Test: added new
Change-Id: I42fc4a6529f73db228ae671097d10a0cda0d834b
Relnote: now detectDragGesures, detectVerticalGestures and detectHozirontalGestures will consume the position change automatically, no need to call change.consumePositionChange in the onDrag callbacks
M compose/animation/animation/integration-tests/animation-demos/src/main/java/androidx/compose/animation/demos/SpringChainDemo.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/SuspendingGesturesDemo.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/DragGestureDetectorSamples.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/DragGestureDetector.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/LongPressTextDragObserver.kt
M compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/gestures/DragGestureDetectorTest.kt
M compose/integration-tests/docs-snippets/src/main/java/androidx/compose/integration/docs/gestures/Gestures.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/gestures/LongPressDragGestureDetectorDemo.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/gestures/PopupDragDemo.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/gestures/TouchSlopDragGestureDetectorDemo.kt
ma...@google.com
Made is so we always consume, the explicit consumption is no longer required and the usage of those functions should be easier now.
Thanks again Aurimas for the report!
au...@google.com
This is awesome, thanks for making this change as it will be a lot easier to do the right thing!
pa...@gmail.com
This change should have be optional! I have a usecase where i want to be able to detectdraggestures and at the same time lets other composables be focusable or detect if theyre being hovered over. this is currently impossible and we have to use large workarounds that affect performance to achieve this same behaviour. I domnt know if there is not another way of implementing this usecase without creating a lot of code that could have been imnplemented if this issue hadnt been "fixed"
Description
We need to explain how to consume changes in those functions, why it's important and what will happen if you don't do it.