Status Update
No update yet.
Comments
ma...@google.com
Thanks Nick. We had this discussions about setters from the very beginning in the AnimatedFloat design, then in ScrollState and now here.
Having a setter seems very good, but every time the only problem was with ambiguity: what should setter do? Snap or animate?
Every person had their own opinion and it always was one of the two 50/50, haha
curious to know your opinion on this!
ni...@google.com
I'm on #TeamAnimate.
ni...@google.com
To expand on this, I feel like swipeable is intended for use cases with continuous changes so animating with the 'obvious' API feels right here. The only issue I see is that value feels like a property, so passing extra params like an animation spec feels odd.
That said, I dislike value being a snap as I feel like it makes it harder to create a smooth experience.
ap...@google.com
Branch: androidx-master-dev
commit 3d00acd443e0e5dd2cc33eb314a65f78f21cec61
Author: Calin Tataru <calintat@google.com>
Date: Mon Aug 10 17:59:53 2020
Modifier.swipeable polishing
This CL does the following things:
* Expand the documentation in SwipeableState.
* Make the setter of SwipeableState.value private.
* Rename some of the properties in SwipeableState, and slightly change
the definition of swipeTarget (now called targetValue) so that it uses
the target value of the ongoing animation if isAnimationRunning = true.
* Add a rememberSwipeableState function to help reduce boilerplate.
* Add Savers for [Drawer/BottomDrawer/Dismiss/Swipeable]State, so that
we can use rememberSavedInstanceState in the rememberFooState functions.
Bug: 163129614
Bug: 163132293
Test: Ran SwipeableTest and other tests depending on swipeable
Relnote: "Renamed some properties in SwipeableState: swipeTarget ->
targetValue, swipeProgress -> progress, swipeDirection -> direction.
Added a rememberSwipeableState function for creating SwipeableStates."
Change-Id: I2fc9c3af465b579a18359ae0aa0853c2d2b02abe
M ui/ui-material/api/current.txt
M ui/ui-material/api/public_plus_experimental_current.txt
M ui/ui-material/api/restricted_current.txt
M ui/ui-material/samples/src/main/java/androidx/compose/material/samples/SwipeToDismissSamples.kt
M ui/ui-material/samples/src/main/java/androidx/compose/material/samples/SwipeableSamples.kt
M ui/ui-material/src/androidAndroidTest/kotlin/androidx/compose/material/SwipeableTest.kt
M ui/ui-material/src/commonMain/kotlin/androidx/compose/material/Drawer.kt
M ui/ui-material/src/commonMain/kotlin/androidx/compose/material/SwipeToDismiss.kt
M ui/ui-material/src/commonMain/kotlin/androidx/compose/material/Swipeable.kt
M ui/ui-material/src/commonMain/kotlin/androidx/compose/material/Switch.kt
ca...@google.com
I have made the setter of value private, so developers will have to use either snapTo or animateTo to change the value. This is the safest option and it's consistent with other classes like AnimatedValue.
ca...@google.com
BTW the reason marking the setter as internal didn't work is that internal fields are visisble in subclasses and hence any modules that see those subclasses, which is interesting. Thanks again Nick for finding this!
Description
SwipeableState.valueproperty is currently writeable but should likely be private to encourage consumers to use the[animate|snap]Tomethods. See (internal)