From 37f0e22911dab4941696c53c0520f88d105f910c Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 1 Jul 2026 13:25:06 +0200 Subject: [PATCH] fix(views): correct snapping + animation for reorderable view lists Rework ReorderableColumn: the first cut measured each row, but grouped cards have position-dependent padding, so slot heights varied and rows snapped to the wrong places with no animation. Now rows are a fixed uniform pitch, the target slot is the whole-pitches dragged, neighbours slide aside with a spring, and the held row settles onto its slot before the order commits. Add a lift (scale + shadow) on the held row and cancel an in-flight settle if a new drag pre-empts it. GroupedRow gains gapBelow so the reorderable list can own uniform inter-row spacing. On-device verified on Pixel 10. Co-Authored-By: Claude Opus 4.8 --- .../calendula/ui/common/GroupedList.kt | 10 +- .../calendula/ui/common/ReorderableColumn.kt | 151 ++++++++++++------ .../calendula/ui/settings/SettingsScreen.kt | 5 +- 3 files changed, 114 insertions(+), 52 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/GroupedList.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/GroupedList.kt index c183aab..dd39503 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/GroupedList.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/GroupedList.kt @@ -142,6 +142,9 @@ fun GroupedRow( dimmed: Boolean = false, container: Color? = null, minHeight: Dp = 72.dp, + // The 2.dp separation between rows in a run. Suppressed by the reorderable + // list, which owns uniform spacing itself so every slot has the same pitch. + gapBelow: Boolean = true, leading: @Composable (() -> Unit)? = null, trailing: @Composable (() -> Unit)? = null, onClick: (() -> Unit)? = null, @@ -160,9 +163,10 @@ fun GroupedRow( topStart = small, topEnd = small, bottomStart = full, bottomEnd = full, ) } - val gap = when (position) { - Position.Top, Position.Middle -> Modifier.padding(bottom = 2.dp) - Position.Bottom, Position.Alone -> Modifier + val gap = when { + !gapBelow -> Modifier + position == Position.Top || position == Position.Middle -> Modifier.padding(bottom = 2.dp) + else -> Modifier } val itemColors = if (selected) { ListItemDefaults.colors( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/ReorderableColumn.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/ReorderableColumn.kt index 369ee3a..d6a1f5e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/ReorderableColumn.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/ReorderableColumn.kt @@ -1,19 +1,38 @@ package de.jeanlucmakiola.calendula.ui.common +import androidx.compose.animation.core.Animatable +import androidx.compose.animation.core.Spring +import androidx.compose.animation.core.animateFloatAsState +import androidx.compose.animation.core.snap +import androidx.compose.animation.core.spring +import androidx.compose.animation.core.tween import androidx.compose.foundation.gestures.detectDragGestures +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateMapOf +import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.input.pointer.pointerInput -import androidx.compose.ui.layout.onSizeChanged +import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp import androidx.compose.ui.zIndex +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import kotlin.math.roundToInt + +/** Uniform row height for [ReorderableColumn]; a fixed pitch keeps drag maths exact. */ +val ReorderableRowHeight: Dp = 64.dp +private val RowGap: Dp = 2.dp /** * A vertical list whose rows can be dragged into a new order by their handle. @@ -21,15 +40,15 @@ import androidx.compose.ui.zIndex * Built for the short, fixed grouped-card lists in Settings (#24) — no external * dependency and no [androidx.compose.foundation.lazy.LazyColumn] (the settings * screens are a single [androidx.compose.foundation.verticalScroll] column, which - * can't nest a scrolling list). [rowContent] receives the [Position] for the - * row's current place in the order (so it can reuse [GroupedRow]'s card shaping) - * and a `dragHandle` [Modifier] to attach to whatever element should start a - * drag. While a row is held it floats above the others, which reflow live; the - * final order is committed once via [onReorder] on release, so the persistence - * layer sees one write per gesture, not one per crossed row. + * can't nest a scrolling list). Rows are a fixed [ReorderableRowHeight] with a + * uniform gap, so a row's target slot is simply how many whole pitches it has + * been dragged. The held row follows the finger while the others slide out of + * its way (animated); on release the held row settles into its slot, then the + * order is committed with a single [onReorder] call. * - * Reordering is measurement-driven: each row reports its own height, so rows of - * unequal height still swap at the right point. + * [rowContent] receives the [Position] for the row's place in the order (to reuse + * [GroupedRow]'s card shaping — pass `gapBelow = false` there, this owns spacing) + * and a `dragHandle` [Modifier] to attach to the element that starts a drag. */ @Composable fun ReorderableColumn( @@ -39,64 +58,100 @@ fun ReorderableColumn( modifier: Modifier = Modifier, rowContent: @Composable (item: T, position: Position, dragHandle: Modifier, isDragging: Boolean) -> Unit, ) { - // Local working copy so rows reflow instantly during a drag; re-seeded when - // the incoming list changes (including the echo of our own committed order). + val pitchPx = with(LocalDensity.current) { (ReorderableRowHeight + RowGap).toPx() } + val scope = rememberCoroutineScope() + + // Local working copy; re-seeded when the incoming list changes (including the + // echo of our own committed order). var order by remember(items) { mutableStateOf(items) } var draggedKey by remember { mutableStateOf(null) } - // Translation of the held row from its settled slot; a swap folds one slot - // height back into this so the row stays under the finger across the swap. - var dragOffsetY by remember { mutableStateOf(0f) } - val heights = remember { mutableStateMapOf() } + // Live translation of the held row from its slot (px); also drives the + // release settle, animated back onto a slot before the order is committed. + var dragOffset by remember { mutableFloatStateOf(0f) } + // The running release/cancel settle, cancelled if a new drag pre-empts it. + var settleJob by remember { mutableStateOf(null) } - Column(modifier) { + val draggedIndex = draggedKey?.let { key -> order.indexOfFirst { keyOf(it) == key }.takeIf { it >= 0 } } + // Whole slots dragged → the slot the held row currently hovers over. + val targetIndex = draggedIndex?.let { + (it + (dragOffset / pitchPx).roundToInt()).coerceIn(0, order.lastIndex) + } + + Column(modifier, verticalArrangement = Arrangement.spacedBy(RowGap)) { order.forEachIndexed { index, item -> val key = keyOf(item) - val isDragging = key == draggedKey + val isDragged = key == draggedKey + + // Slide neighbours by one pitch to open the gap the held row will drop + // into. Snap (not animate) once idle, so committing the new order — which + // moves each row's slot — doesn't visibly fight a lingering animation. + val shift = when { + draggedIndex == null || targetIndex == null || isDragged -> 0f + index in (draggedIndex + 1)..targetIndex -> -pitchPx + index in targetIndex until draggedIndex -> pitchPx + else -> 0f + } + val animatedShift by animateFloatAsState( + targetValue = shift, + animationSpec = if (draggedKey != null) spring(stiffness = Spring.StiffnessMediumLow) else snap(), + label = "reorderShift", + ) + val dragHandle = Modifier.pointerInput(key) { detectDragGestures( onDragStart = { + settleJob?.cancel() draggedKey = key - dragOffsetY = 0f + dragOffset = 0f + }, + onDrag = { change, amount -> + change.consume() + dragOffset += amount.y }, onDragEnd = { - draggedKey = null - dragOffsetY = 0f - onReorder(order) + val from = order.indexOfFirst { keyOf(it) == key } + if (from < 0) return@detectDragGestures + val to = (from + (dragOffset / pitchPx).roundToInt()).coerceIn(0, order.lastIndex) + settleJob = scope.launch { + // Settle the held row onto its target slot, then commit — + // resetting offset and slot in the same frame, so nothing jumps. + Animatable(dragOffset).animateTo((to - from) * pitchPx, tween(160)) { + dragOffset = value + } + if (to != from) { + order = order.toMutableList().apply { add(to, removeAt(from)) } + onReorder(order) + } + draggedKey = null + dragOffset = 0f + } }, onDragCancel = { - draggedKey = null - dragOffsetY = 0f - }, - onDrag = { change, dragAmount -> - change.consume() - dragOffsetY += dragAmount.y - val cur = order.indexOfFirst { keyOf(it) == key } - if (cur < 0) return@detectDragGestures - // Swap once the row has travelled past half of the - // neighbour it's heading toward, in the drag direction. - if (dragAmount.y > 0 && cur < order.lastIndex) { - val nextHeight = heights[keyOf(order[cur + 1])] ?: 0 - if (nextHeight > 0 && dragOffsetY > nextHeight / 2f) { - order = order.toMutableList().apply { add(cur + 1, removeAt(cur)) } - dragOffsetY -= nextHeight - } - } else if (dragAmount.y < 0 && cur > 0) { - val prevHeight = heights[keyOf(order[cur - 1])] ?: 0 - if (prevHeight > 0 && dragOffsetY < -prevHeight / 2f) { - order = order.toMutableList().apply { add(cur - 1, removeAt(cur)) } - dragOffsetY += prevHeight - } + settleJob = scope.launch { + Animatable(dragOffset).animateTo(0f, tween(160)) { dragOffset = value } + draggedKey = null + dragOffset = 0f } }, ) } + Box( Modifier - .onSizeChanged { heights[key] = it.height } - .zIndex(if (isDragging) 1f else 0f) - .graphicsLayer { translationY = if (isDragging) dragOffsetY else 0f }, + .height(ReorderableRowHeight) + .zIndex(if (isDragged) 1f else 0f) + .graphicsLayer { + translationY = if (isDragged) dragOffset else animatedShift + if (isDragged) { + scaleX = 1.02f + scaleY = 1.02f + shadowElevation = 8.dp.toPx() + shape = RoundedCornerShape(20.dp) + clip = false + } + }, ) { - rowContent(item, positionOf(index, order.size), dragHandle, isDragging) + rowContent(item, positionOf(index, order.size), dragHandle, isDragged) } } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsScreen.kt index 3c3c97a..8b17a74 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsScreen.kt @@ -107,6 +107,7 @@ import de.jeanlucmakiola.calendula.ui.common.GroupedRow import de.jeanlucmakiola.calendula.ui.common.IMPLEMENTED_VIEWS import de.jeanlucmakiola.calendula.ui.common.QuickSwitchConfig import de.jeanlucmakiola.calendula.ui.common.ReorderableColumn +import de.jeanlucmakiola.calendula.ui.common.ReorderableRowHeight import de.jeanlucmakiola.calendula.ui.common.icon import de.jeanlucmakiola.calendula.ui.common.labelRes import de.jeanlucmakiola.calendula.ui.common.CalendarColorChip @@ -768,7 +769,9 @@ private fun ViewRow( title = stringResource(view.labelRes), position = position, dimmed = dimmed, - minHeight = 64.dp, + minHeight = ReorderableRowHeight, + // The reorderable column owns the inter-row spacing (uniform pitch). + gapBelow = false, container = if (isDragging) MaterialTheme.colorScheme.secondaryContainer else null, leading = { Icon(