-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Isn't stable if the parent component recomposes during a drag #276
Comments
yap had the same issue |
I've learned more information about this through testing. It appears that if the caller of |
Ok, I've written a cache to work around this issue by caching the items while dragging. Example usage: val reorderCache = rememberReorderableLazyListCache(items = myItems)
val reorderState = rememberReorderableLazyListState(
onMove = { from, to ->
// It's safe to directly modify the items in the cache since they only get replaced when
// a drag starts.
reorderCache.items.value = reorderCache.items.value.toMutableList().apply {
if (to.index < size && from.index < size) {
add(to.index, removeAt(from.index))
} else {
Timber.w("Tried to reorder outside of bounds. size: ${reorderCache.items.value.size}, from: $from, to: $to")
}
}
},
onDragEnd = { startIndex, endIndex ->
onReorder(reorderCache.items.value.map { it.id })
}
)
// Keep items updated.
reorderCache.onCompose(reorderState, proteinColumns.value)
LazyRow(
state = reorderState.listState,
modifier = modifier.reorderable(reorderState)
) {
items(reorderCache.items.value, { it.id }) { item ->
ReorderableItem(
reorderableState = reorderState,
key = item.id,
) { isDragging ->
}
}
} And the actual implementation: /**
* Caches the list of items while dragging. This is necessary in some cases because ReorderableLazyList
* isn't stable if the list changes while a drag is in progress. Example usage:
*
* ```
* val reorderCache = rememberReorderableLazyListCache(items = myItems)
* val reorderState = rememberReorderableLazyListState(
* onMove = { from, to ->
* // It's safe to directly modify the items in the cache since they only get replaced when
* // a drag starts.
* reorderCache.items.value = reorderCache.items.value.toMutableList().apply {
* if (to.index < size && from.index < size) {
* add(to.index, removeAt(from.index))
* } else {
* Timber.w("Tried to reorder outside of bounds. size: ${reorderCache.items.value.size}, from: $from, to: $to")
* }
* }
* },
* onDragEnd = { startIndex, endIndex ->
* onReorder(reorderCache.items.value.map { it.id })
* }
* )
* // Keep items updated.
* reorderCache.onCompose(reorderState, proteinColumns.value)
*
* LazyRow(
* state = reorderState.listState,
* modifier = modifier.reorderable(reorderState)
* ) {
* items(reorderCache.items.value, { it.id }) { item ->
* ReorderableItem(
* reorderableState = reorderState,
* key = item.id,
* ) { isDragging ->
*
* }
* }
* }
* ```
*/
@Composable
fun <T> rememberReorderableLazyListCache(
items: List<T>,
): ReorderableLazyListCache<T> {
val scope = rememberCoroutineScope()
val cache = remember { ReorderableLazyListCache<T>(items, scope) }
return cache
}
@Stable
class ReorderableLazyListCache <T>(
private val initialItems: List<T>,
private val scope: CoroutineScope,
) {
private var state: ReorderableLazyListState? = null
private var isDragging = false
private var isDraggingWatchJob: Job? = null
private var currentItems = initialItems
/**
* Will cache the current items when a drag begins.
*/
val items = mutableStateOf(initialItems)
/**
* Must be called each time the parent Composable runs to keep the items up-to-date.
*/
fun onCompose(newState: ReorderableLazyListState, newItems: List<T>) {
currentItems = newItems
if (!isDragging) {
// It works without this, but it seems safer to not fire observers here since we don't need to,
// since if the parent composable does a transform on the list, it might technically be a different
// list each time, which might trigger an infinite loop.
Snapshot.withoutReadObservation {
items.value = currentItems
}
}
val oldState = state
state = newState
if (oldState != newState) {
/**
* Launch a job to observe the draggingItemKey without invalidating our parent scope.
* When we start dragging, we'll cache the current items while the drag runs.
*/
isDraggingWatchJob?.cancel()
isDraggingWatchJob = scope.launch {
snapshotFlow {
newState.draggingItemKey
}.collect { draggingItemKey ->
isDragging = draggingItemKey != null
// When we start dragging, update "items" with our current state, then
// ignore updates until we stop dragging. Notice that we're not updating them
// when a drag ends, which prevents a flash of undragged items and allows our
// parent to save and then recompose.
if (isDragging) {
Snapshot.withoutReadObservation {
items.value = currentItems
}
}
}
}
}
}
} |
I'm trying to keep track of the sort order in a separate list, so that if it gets recomposed with new state during a drag, we won't lose the sort order. My app needs to subscribe to a server that emits new state periodically. However, ComposeReorderable appears to require that the same mutableList be passed to LazyColumn's
items
as the list you update inonMove
.Steps to reproduce:
onMove
, I'm generating it in aremember
block based on the sort order list.I've also tested this by updating both a mutableList directly in addition to my derived list. I've verified that both lists have exactly the same contents at every recomposition, so I'm guessing ComposeReorderable might be relying on some internal functionality in LazyList that runs in a different order if you update the list directly.
The text was updated successfully, but these errors were encountered: