From 4ec6c49eaaa3508bf4c169c133bdae6863966e9b Mon Sep 17 00:00:00 2001 From: Markel15 Date: Fri, 16 Oct 2026 04:03:20 +0200 Subject: [PATCH 01/11] refactor: phase 1 of having a grid instead of a Carousel for ideas and checklists. Purely additive: no UI or behavior changes yet, SectionedFlowView still renders the carousels and the existing reorder methods stay in place. The key here is that I can't simply change the actual carousel with a LazyVerticalStaggeredGrid because that composable doesn't know their height. For that I create an item to put everything with the layout I have in mind. These items also help with the reordering because add a scope within the items would be reordered. - New sealed interface FlowItem: every rendered item (section headers, reminder banner, task rows, checklist cards and idea cards) is one variant, so the index reported by `reorderable` maps 1:1 onto items[index]. That removes the hardcoded `headerOffset` arithmetic SectionedFlowView needs today. - FlowUiState.Success.items: flat, render-ready list declared in the class body so `copy()` always recomputes it. Headers are emitted only for non-empty sections. - FlowItem.stableKey(): unique key per variant; folds the per-task delete version into the task key so an undo recreates the composable with a fresh SwipeToDismissBoxState instead of restoring the stale EndToStart saved state. - FlowUiState.Success.withItems(): projects a reordered item list back onto the three typed lists. - FlowItemTest: 12 unit tests covering item construction, key uniqueness and the withItems round-trip. --- .../markel/flowstate/feature/flow/FlowItem.kt | 91 +++++++ .../flowstate/feature/flow/FlowUiState.kt | 43 +++- .../flowstate/feature/flow/FlowItemTest.kt | 238 ++++++++++++++++++ 3 files changed, 370 insertions(+), 2 deletions(-) create mode 100644 feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt create mode 100644 feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt new file mode 100644 index 00000000..d6e3fa12 --- /dev/null +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt @@ -0,0 +1,91 @@ +package com.markel.flowstate.feature.flow + +import com.markel.flowstate.core.domain.CheckList +import com.markel.flowstate.core.domain.Idea +import com.markel.flowstate.core.domain.Task + +/** + * Flat item model for the Flow screen. + * + * Every item the screen renders — including section headers and the reminder + * banner — is one of these. That makes the index reported by the `reorderable` + * library (`from.index` / `to.index`) map 1:1 onto `items[index]`, which removes + * the hardcoded `headerOffset` arithmetic that `SectionedFlowView` needs while + * headers are emitted as independent `item { }` blocks. + * + * Reordering is scoped to a single [Section]: a move is only valid when the + * dragged item and the target item share the same non-null [section]. Items whose + * [section] is `null` (headers, banner) are fixed chrome and never move. + * + * Adding a future item type (drawings, recordings, …) means adding a variant + * here plus its card composable — the grid, the reorder state and the span + * logic stay untouched. That is what keeps a future single combined list cheap. + */ +sealed interface FlowItem { + + /** The reorderable scope an item belongs to. */ + enum class Section { TASKS, CHECKLISTS, IDEAS } + + /** Section this item may be reordered within, or `null` for fixed chrome. */ + val section: Section? get() = null + + /** + * `true` when the item must span **every** column of the grid + * (`StaggeredGridItemSpan.FullLine`) instead of occupying a single one. + * + * Note this is not "fill the available width" — every item does that. It is + * "take all N columns instead of 1". Only the two card types opt out. + */ + val spanAllColumns: Boolean get() = true + + // ── Tasks ───────────────────────────────────────────────────────────────── + + data object TasksHeader : FlowItem + + data object ReminderBanner : FlowItem + + data class TaskRow(val task: Task) : FlowItem { + override val section: Section? get() = Section.TASKS + } + + // ── CheckLists ──────────────────────────────────────────────────────────── + + data object CheckListsHeader : FlowItem + + data class CheckListCard(val checkList: CheckList) : FlowItem { + override val section: Section? get() = Section.CHECKLISTS + override val spanAllColumns: Boolean get() = false + } + + // ── Ideas ───────────────────────────────────────────────────────────────── + + data object IdeasHeader : FlowItem + + data class IdeaCard(val idea: Idea) : FlowItem { + override val section: Section? get() = Section.IDEAS + override val spanAllColumns: Boolean get() = false + } +} + +/** + * Stable key for an item, used by the grid's `itemsIndexed`. + * + * Must be unique across **all** [FlowItem] variants: a collision between, say, a + * task and an idea would make Compose reuse the wrong composable and the wrong + * `SwipeToDismissBoxState`. + * + * [taskDeleteVersions] is folded into the task key on purpose. Bumping it after + * an undo gives the item a brand-new key, so Compose creates a fresh composable + * with a fresh `SwipeToDismissBoxState` instead of restoring the stale + * `EndToStart` saved state that would immediately re-trigger the deletion. + */ +internal fun FlowItem.stableKey(taskDeleteVersions: Map = emptyMap()): Any = + when (this) { + FlowItem.TasksHeader -> "tasks_header" + FlowItem.ReminderBanner -> "reminder_banner" + is FlowItem.TaskRow -> Pair(task.id, taskDeleteVersions[task.id] ?: 0) + FlowItem.CheckListsHeader -> "checklists_header" + is FlowItem.CheckListCard -> "cl_${checkList.id}" + FlowItem.IdeasHeader -> "ideas_header" + is FlowItem.IdeaCard -> "idea_${idea.id}" + } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt index 95d84eee..1ba9b1a7 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt @@ -15,5 +15,44 @@ sealed interface FlowUiState { val selectedCategoryId: Int? = Category.GENERAL_ID, val categoriesEnabled: Boolean = false, val pendingTaskCounts: Map = emptyMap() - ) : FlowUiState -} \ No newline at end of file + ) : FlowUiState { + + /** + * Flat, render-ready list consumed by `SectionedFlowView`. + */ + val items: List = buildList { + if (tasks.isNotEmpty()) { + add(FlowItem.TasksHeader) + add(FlowItem.ReminderBanner) + tasks.forEach { add(FlowItem.TaskRow(it)) } + } + if (checkLists.isNotEmpty()) { + add(FlowItem.CheckListsHeader) + checkLists.forEach { add(FlowItem.CheckListCard(it)) } + } + if (ideas.isNotEmpty()) { + add(FlowItem.IdeasHeader) + ideas.forEach { add(FlowItem.IdeaCard(it)) } + } + } + } +} + +/** + * Rebuilds a [FlowUiState.Success] from an already-reordered [items] list. + * + * Used by the reorder callback: a drag hands back a shuffled `List`, + * and this projects it back onto the three typed lists. `filterIsInstance` + * preserves the relative order of each section and the assignment order + * (tasks → checkLists → ideas) matches how [FlowUiState.Success.items] is built, + * so the resulting `items` mirrors the list that was passed in. + * + * Chrome items (headers, banner) are dropped — they are re-emitted by `items` in + * their canonical position. As a side effect a cross-section move (which the + * caller rejects anyway) degrades safely instead of corrupting a typed list. + */ +internal fun FlowUiState.Success.withItems(items: List): FlowUiState.Success = copy( + tasks = items.filterIsInstance().map { it.task }, + checkLists = items.filterIsInstance().map { it.checkList }, + ideas = items.filterIsInstance().map { it.idea }, +) diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt new file mode 100644 index 00000000..5aa0f030 --- /dev/null +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt @@ -0,0 +1,238 @@ +package com.markel.flowstate.feature.flow + +import com.markel.flowstate.core.domain.CheckList +import com.markel.flowstate.core.domain.Idea +import com.markel.flowstate.core.domain.Task +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for the flat item model that backs the Flow screen + * ([FlowItem], [FlowUiState.Success.items], [stableKey] and [withItems]). + * + * These cover pure mapping logic only — no coroutines, no mocks, no Android + * dependencies — so they run on the local JVM. + */ +class FlowItemTest { + + // ── Fixtures ────────────────────────────────────────────────────────────── + + private fun task(id: Int) = Task(id = id, title = "Task $id", isDone = false) + + /** + * `createdAt` is pinned on purpose: it defaults to `System.currentTimeMillis()`, + * so two separate calls to `idea(3)` would produce two *unequal* ideas and + * `items.indexOf(FlowItem.IdeaCard(idea(3)))` would return -1. + */ + private fun idea(id: Int) = Idea( + id = id, title = "Idea $id", content = "", createdAt = 0L, color = 0L + ) + + private fun checkList(id: Int) = CheckList(id = id, title = "List $id", color = 0L) + + private fun state( + tasks: List = emptyList(), + ideas: List = emptyList(), + checkLists: List = emptyList(), + ) = FlowUiState.Success(tasks = tasks, ideas = ideas, checkLists = checkLists) + + // ── items: construcción ──────────────────────────────────────────────────── + + @Test + fun rows_withAllSections_emitsHeadersThenItems_inOrder() { + val items = state( + tasks = listOf(task(1), task(2)), + checkLists = listOf(checkList(1)), + ideas = listOf(idea(1)), + ).items + + assertEquals( + listOf( + FlowItem.TasksHeader, + FlowItem.ReminderBanner, + FlowItem.TaskRow(task(1)), + FlowItem.TaskRow(task(2)), + FlowItem.CheckListsHeader, + FlowItem.CheckListCard(checkList(1)), + FlowItem.IdeasHeader, + FlowItem.IdeaCard(idea(1)), + ), + items + ) + } + + @Test + fun rows_withEmptySections_omitsTheirHeaders() { + assertEquals(emptyList(), state().items) + + assertEquals( + listOf(FlowItem.TasksHeader, FlowItem.ReminderBanner, FlowItem.TaskRow(task(1))), + state(tasks = listOf(task(1))).items + ) + + assertEquals( + listOf(FlowItem.IdeasHeader, FlowItem.IdeaCard(idea(1))), + state(ideas = listOf(idea(1))).items + ) + } + + /** + * `items` lives in the class body, so `copy()` runs the constructor again and + * recomputes it. If it were ever promoted to a constructor parameter this + * test would fail — which is exactly why it exists. + */ + @Test + fun rows_isRecomputedAfterCopy() { + val original = state(tasks = listOf(task(1)), ideas = listOf(idea(1))) + assertEquals(5, original.items.size) + + val withoutIdeas = original.copy(ideas = emptyList()) + assertEquals(3, withoutIdeas.items.size) + + val withCheckLists = withoutIdeas.copy(checkLists = listOf(checkList(1))) + assertEquals( + listOf( + FlowItem.TasksHeader, + FlowItem.ReminderBanner, + FlowItem.TaskRow(task(1)), + FlowItem.CheckListsHeader, + FlowItem.CheckListCard(checkList(1)), + ), + withCheckLists.items + ) + } + + // ── stableKey ───────────────────────────────────────────────────────────── + + @Test + fun stableKey_isUniqueAcrossAllRowVariants() { + val items = listOf( + FlowItem.TasksHeader, + FlowItem.ReminderBanner, + FlowItem.TaskRow(task(1)), + FlowItem.CheckListsHeader, + FlowItem.CheckListCard(checkList(1)), + FlowItem.IdeasHeader, + FlowItem.IdeaCard(idea(1)), + ) + + assertEquals(items.size, items.map { it.stableKey() }.toSet().size) + } + + /** Tasks and ideas share an id space, so their keys need an explicit prefix. */ + @Test + fun stableKey_taskAndIdeaWithSameId_doNotCollide() { + assertNotEquals( + FlowItem.TaskRow(task(1)).stableKey(), + FlowItem.IdeaCard(idea(1)).stableKey() + ) + assertNotEquals( + FlowItem.CheckListCard(checkList(1)).stableKey(), + FlowItem.IdeaCard(idea(1)).stableKey() + ) + } + + @Test + fun stableKey_forTask_includesDeleteVersion() { + val item = FlowItem.TaskRow(task(7)) + + assertEquals(Pair(7, 0), item.stableKey()) + assertEquals(Pair(7, 3), item.stableKey(mapOf(7 to 3))) + // Ids distintos siguen siendo distintos con el mismo contador + assertNotEquals( + FlowItem.TaskRow(task(7)).stableKey(mapOf(7 to 1)), + FlowItem.TaskRow(task(8)).stableKey(mapOf(7 to 1)) + ) + } + + @Test + fun stableKey_forIdeasAndCheckLists_ignoresDeleteVersions() { + val versions = mapOf(1 to 5) + + assertEquals("idea_1", FlowItem.IdeaCard(idea(1)).stableKey(versions)) + assertEquals("cl_1", FlowItem.CheckListCard(checkList(1)).stableKey(versions)) + } + + // ── withItems ────────────────────────────────────────────────────────────── + + @Test + fun withRows_projectsRowsBackOntoTypedLists() { + val original = state( + tasks = listOf(task(1), task(2)), + checkLists = listOf(checkList(1)), + ideas = listOf(idea(1), idea(2)), + ) + + val rebuilt = original.withItems(original.items) + + assertEquals(original.tasks, rebuilt.tasks) + assertEquals(original.checkLists, rebuilt.checkLists) + assertEquals(original.ideas, rebuilt.ideas) + assertEquals(original.items, rebuilt.items) + } + + @Test + fun withRows_preservesANewOrderWithinASection() { + val original = state(ideas = listOf(idea(1), idea(2), idea(3))) + val items = original.items.toMutableList() + + val from = items.indexOf(FlowItem.IdeaCard(idea(3))) + val to = items.indexOf(FlowItem.IdeaCard(idea(1))) + items.add(to, items.removeAt(from)) + + val rebuilt = original.withItems(items) + + assertEquals(listOf(idea(3), idea(1), idea(2)), rebuilt.ideas) + // Las secciones que no se han tocado se conservan + assertEquals(original.tasks, rebuilt.tasks) + assertEquals(original.checkLists, rebuilt.checkLists) + } + + @Test + fun withRows_dropChromeRowsAndReEmitsThemInPlace() { + val original = state(tasks = listOf(task(1)), ideas = listOf(idea(1))) + val shuffled = listOf( + FlowItem.IdeasHeader, + FlowItem.IdeaCard(idea(1)), + FlowItem.TasksHeader, + FlowItem.ReminderBanner, + FlowItem.TaskRow(task(1)), + ) + + val rebuilt = original.withItems(shuffled) + + assertEquals(listOf(task(1)), rebuilt.tasks) + assertEquals(listOf(idea(1)), rebuilt.ideas) + assertEquals(original.items, rebuilt.items) + } + + // ── section / spanAllColumns ─────────────────────────────────────────────────── + + @Test + fun section_isNullForChrome_andSetForCards() { + assertNull(FlowItem.TasksHeader.section) + assertNull(FlowItem.ReminderBanner.section) + assertNull(FlowItem.CheckListsHeader.section) + assertNull(FlowItem.IdeasHeader.section) + + assertEquals(FlowItem.Section.TASKS, FlowItem.TaskRow(task(1)).section) + assertEquals(FlowItem.Section.CHECKLISTS, FlowItem.CheckListCard(checkList(1)).section) + assertEquals(FlowItem.Section.IDEAS, FlowItem.IdeaCard(idea(1)).section) + } + + @Test + fun fullWidth_isTrueForChromeAndTasks_falseForGridCards() { + assertTrue(FlowItem.TasksHeader.spanAllColumns) + assertTrue(FlowItem.ReminderBanner.spanAllColumns) + assertTrue(FlowItem.CheckListsHeader.spanAllColumns) + assertTrue(FlowItem.IdeasHeader.spanAllColumns) + assertTrue(FlowItem.TaskRow(task(1)).spanAllColumns) + + assertFalse(FlowItem.CheckListCard(checkList(1)).spanAllColumns) + assertFalse(FlowItem.IdeaCard(idea(1)).spanAllColumns) + } +} From 72753675f2df98c33f6142983687becf5f543ecd Mon Sep 17 00:00:00 2001 From: Markel15 Date: Fri, 16 Oct 2026 23:10:59 +0200 Subject: [PATCH 02/11] refactor: adopt unified staggered grid architecture for Flow screen Consolidate screen sections into a single LazyVerticalStaggeredGrid inside SectionedFlowView to allow unified scrolling, cross-section reordering, and responsive multi-column card layouts. Key Changes: * SectionedFlowView: - Replaced individual list containers with a single LazyVerticalStaggeredGrid. - Added full-width line spans for Tasks/Headers and single-lane spans for Cards. - Localized draggable item logic using `ReorderableCard` and `TaskRowContent`. * Stable Keys & Task Deletions: - Abstracted key generation via `stableKey(taskDeleteVersions)`. - For Tasks, combines `task.id` with its monotonic `taskDeleteVersions` counter to ensure Compose re-instantiates SwipeToDismiss state upon undo. * Scroll Modifiers: - Added a `LazyStaggeredGridState` overload for `rememberFabVisibilityState`. - Refactored internal scroll tracking to reuse a single `Flow>` pipeline across both LazyList and StaggeredGrid states. --- .../core/designsystem/ui/ScrollModifiers.kt | 57 ++- .../flowstate/feature/flow/FlowScreen.kt | 25 +- .../flowstate/feature/flow/FlowViewModel.kt | 87 ++-- .../feature/flow/components/GridCards.kt | 19 +- .../flow/components/ReorderCategorySheet.kt | 2 +- .../flow/components/ReorderableCarousel.kt | 62 --- .../flow/components/SectionedFlowView.kt | 391 ++++++++++-------- .../feature/flow/FlowViewModelTest.kt | 182 +++++--- 8 files changed, 482 insertions(+), 343 deletions(-) delete mode 100644 feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/ReorderableCarousel.kt diff --git a/core/designsystem/src/main/java/com/markel/flowstate/core/designsystem/ui/ScrollModifiers.kt b/core/designsystem/src/main/java/com/markel/flowstate/core/designsystem/ui/ScrollModifiers.kt index 1fe6f61a..6e1e04bc 100644 --- a/core/designsystem/src/main/java/com/markel/flowstate/core/designsystem/ui/ScrollModifiers.kt +++ b/core/designsystem/src/main/java/com/markel/flowstate/core/designsystem/ui/ScrollModifiers.kt @@ -1,8 +1,16 @@ package com.markel.flowstate.core.designsystem.ui import androidx.compose.foundation.lazy.LazyListState -import androidx.compose.runtime.* +import androidx.compose.foundation.lazy.staggeredgrid.LazyStaggeredGridState +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.State +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.snapshotFlow +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged import kotlin.math.abs @@ -16,6 +24,47 @@ fun rememberFabVisibilityState( lazyListState: LazyListState, forceVisible: Boolean = false, scrollThreshold: Int = 60 +): State = rememberFabVisibilityState( + scrollPosition = remember(lazyListState) { + snapshotFlow { + lazyListState.firstVisibleItemIndex to lazyListState.firstVisibleItemScrollOffset + } + }, + forceVisible = forceVisible, + scrollThreshold = scrollThreshold, +) + +/** + * Overload for the staggered grid used by the Flow screen. + * + * `LazyStaggeredGridState` exposes the exact same two fields the logic needs + * (`firstVisibleItemIndex` and `firstVisibleItemScrollOffset`), so both + * overloads share a single implementation. + */ +@Composable +fun rememberFabVisibilityState( + gridState: LazyStaggeredGridState, + forceVisible: Boolean = false, + scrollThreshold: Int = 60 +): State = rememberFabVisibilityState( + scrollPosition = remember(gridState) { + snapshotFlow { + gridState.firstVisibleItemIndex to gridState.firstVisibleItemScrollOffset + } + }, + forceVisible = forceVisible, + scrollThreshold = scrollThreshold, +) + +/** + * Shared implementation, expressed only in terms of the scroll position so that + * `LazyListState` and `LazyStaggeredGridState` can both reuse it. + */ +@Composable +private fun rememberFabVisibilityState( + scrollPosition: Flow>, + forceVisible: Boolean, + scrollThreshold: Int, ): State { val fabVisible = rememberSaveable { mutableStateOf(true) } @@ -23,14 +72,12 @@ fun rememberFabVisibilityState( fabVisible.value = true } - LaunchedEffect(lazyListState) { + LaunchedEffect(scrollPosition) { var previousIndex = 0 var previousOffset = 0 var accumulatedScroll = 0 - snapshotFlow { - lazyListState.firstVisibleItemIndex to lazyListState.firstVisibleItemScrollOffset - } + scrollPosition // Avoid processing if the state hasn't changed at all .distinctUntilChanged() .collect { (currentIndex, currentOffset) -> diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowScreen.kt index 20421dd3..38dd63cd 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowScreen.kt @@ -2,8 +2,8 @@ package com.markel.flowstate.feature.flow import android.content.res.Configuration import androidx.compose.foundation.layout.* -import androidx.compose.foundation.lazy.LazyListState -import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.foundation.lazy.staggeredgrid.LazyStaggeredGridState +import androidx.compose.foundation.lazy.staggeredgrid.rememberLazyStaggeredGridState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.* import androidx.compose.runtime.* @@ -73,12 +73,12 @@ fun FlowScreen( val currentTabId = selectedCategoryId ?: Category.GENERAL_ID // Registry of each tab's LazyListState so the FAB can read the active list state from a single variable - val tabListStates = remember { mutableStateMapOf() } + val tabGridStates = remember { mutableStateMapOf() } // ── FAB ────────── - val activeListState = tabListStates[currentTabId] ?: rememberLazyListState() + val activeGridState = tabGridStates[currentTabId] ?: rememberLazyStaggeredGridState() val fabVisible by rememberFabVisibilityState( - lazyListState = activeListState, + gridState = activeGridState, forceVisible = allEmpty ) @@ -107,27 +107,24 @@ fun FlowScreen( } saveableStateHolder.SaveableStateProvider(currentTabId) { - val tabListState = rememberLazyListState() + val tabGridState = rememberLazyStaggeredGridState() - // Publish this tab's LazyListState to the shared registry - // so the FAB (outside SaveableStateProvider) can read it. - SideEffect { tabListStates[currentTabId] = tabListState } + // Publish this tab's grid state to the shared registry so the + // FAB (outside SaveableStateProvider) can read it. + SideEffect { tabGridStates[currentTabId] = tabGridState } SectionedFlowView( uiState = flowUiState, onScrolled = { isHeaderMinimized = true }, + onItemReorder = { from, to -> flowViewModel.onItemReorder(from, to) }, onTaskClick = { onNavigateToTaskEditor(it.id) }, onTaskDelete = { task -> flowViewModel.onTaskSwiped(task) }, onTaskToggle = { taskViewModel.toggleTaskDone(it) }, - onTaskReorder = { from, to -> flowViewModel.onTaskReorder(from, to) }, onIdeaClick = { onNavigateToIdeaEditor(it.id) }, - onIdeaReorder = { from, to -> flowViewModel.onIdeaReorder(from, to) }, onCheckListClick = { onNavigateToCheckListEditor(it.id, it.categoryId) }, - onCheckListReorder = { from, to -> flowViewModel.onCheckListReorder(from, to) }, showPermissionBanner = showPermissionBanner, taskDeleteVersions = taskDeleteVersions, - categoriesEnabled = categoriesEnabled, - outerListState = tabListState, + gridState = tabGridState, ) } } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt index 9dafc759..704c1302 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt @@ -324,50 +324,57 @@ class FlowViewModel @Inject constructor( // ── Reorder ──────────────────────────────────────────────────────── - fun onTaskReorder(fromIndex: Int, toIndex: Int) { - val current = (_uiState.value as? FlowUiState.Success)?.tasks?.toMutableList() ?: return - - val item = current.removeAt(fromIndex) - current.add(toIndex, item) - - val updated = current.mapIndexed { i, t -> t.copy(position = i) } - - _uiState.value = (_uiState.value as FlowUiState.Success).copy(tasks = updated) - - viewModelScope.launch { - taskRepository.updateTasksOrder(updated) - } - } - - fun onIdeaReorder(fromIndex: Int, toIndex: Int) { - val currentState = _uiState.value as? FlowUiState.Success ?: return - val list = currentState.ideas.toMutableList() - - val item = list.removeAt(fromIndex) - list.add(toIndex, item) - - val updated = list.mapIndexed { i, idea -> idea.copy(position = i) } - - _uiState.value = currentState.copy(ideas = updated) - - viewModelScope.launch { - ideaRepository.updateIdeasOrder(updated) + /** + * Applies a drag between two rows of the flat [FlowItem] list. + * + * The move is only applied when both rows belong to the same non-null + * [FlowItem.Section]: headers and the reminder banner are fixed chrome, and + * a card may not jump between the CheckLists and Ideas sections. When the + * guard rejects the move the row simply snaps back to its original slot. + * + * Note this is called on every item crossing during a drag, so it writes to + * the database on each one — same as the carousel did. Persisting a single + * time on drop is a follow-up. + */ + fun onItemReorder(fromIndex: Int, toIndex: Int) { + val state = _uiState.value as? FlowUiState.Success ?: return + val items = state.items.toMutableList() + + val from = items.getOrNull(fromIndex) ?: return + val to = items.getOrNull(toIndex) ?: return + + val section = from.section ?: return + if (section != to.section) return + + val moved = items.removeAt(fromIndex) + items.add(toIndex, moved) + + val updated = state.withItems(items) + + // `position` is the column the DAOs sort by, so the reordered section has + // to be renumbered 0..n-1 before it is written. Without this the next + // database emission (ORDER BY position ASC) would restore the previous + // order and the card would visibly snap back to where it started. + val renumbered = when (section) { + FlowItem.Section.TASKS -> updated.copy( + tasks = updated.tasks.mapIndexed { index, task -> task.copy(position = index) } + ) + FlowItem.Section.CHECKLISTS -> updated.copy( + checkLists = updated.checkLists.mapIndexed { index, list -> list.copy(position = index) } + ) + FlowItem.Section.IDEAS -> updated.copy( + ideas = updated.ideas.mapIndexed { index, idea -> idea.copy(position = index) } + ) } - } - - fun onCheckListReorder(fromIndex: Int, toIndex: Int) { - val currentState = _uiState.value as? FlowUiState.Success ?: return - val list = currentState.checkLists.toMutableList() - - val item = list.removeAt(fromIndex) - list.add(toIndex, item) - val updated = list.mapIndexed { i, cl -> cl.copy(position = i) } - - _uiState.value = currentState.copy(checkLists = updated) + _uiState.value = renumbered viewModelScope.launch { - checkListRepository.updateCheckListsOrder(updated) + when (section) { + FlowItem.Section.TASKS -> taskRepository.updateTasksOrder(renumbered.tasks) + FlowItem.Section.CHECKLISTS -> checkListRepository.updateCheckListsOrder(renumbered.checkLists) + FlowItem.Section.IDEAS -> ideaRepository.updateIdeasOrder(renumbered.ideas) + } } } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/GridCards.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/GridCards.kt index 0c78ca4c..60a0cc91 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/GridCards.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/GridCards.kt @@ -33,6 +33,17 @@ import java.text.SimpleDateFormat import java.util.Date import java.util.Locale +/** + * Cap on the idea body before it is truncated with an ellipsis. + * + * The card sizes itself to its content, so this is what bounds the height. + * `heightIn(max = …)` would clip the text instead, which is why it is not used. + */ +private const val IDEA_CONTENT_MAX_LINES = 8 + +/** Pending items rendered inside a checklist card before the "+N more" line. */ +private const val CHECK_LIST_VISIBLE_ITEMS = 5 + // ── Idea ────────────────────────────────────────────────────────────────────── @Composable @@ -74,7 +85,7 @@ fun IdeaGridCard( text = idea.content, style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, - maxLines = 6, + maxLines = IDEA_CONTENT_MAX_LINES, overflow = TextOverflow.Ellipsis ) } @@ -122,7 +133,7 @@ fun CheckListGridCard( ) Spacer(Modifier.height(6.dp)) val pendingItems = checkList.items.filter { !it.isDone } - val visibleItems = pendingItems.take(4) + val visibleItems = pendingItems.take(CHECK_LIST_VISIBLE_ITEMS) visibleItems.forEach { item -> Row( verticalAlignment = Alignment.CenterVertically, @@ -139,10 +150,10 @@ fun CheckListGridCard( ) } } - if (pendingItems.size > 5) { + if (pendingItems.size > visibleItems.size) { Spacer(Modifier.height(3.dp)) Text( - text = "+${pendingItems.size - 4} " + stringResource(com.markel.flowstate.feature.tasks.R.string.more), + text = "+${pendingItems.size - visibleItems.size} " + stringResource(com.markel.flowstate.feature.tasks.R.string.more), style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.5f) ) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/ReorderCategorySheet.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/ReorderCategorySheet.kt index 3a9093e8..c8e37eaf 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/ReorderCategorySheet.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/ReorderCategorySheet.kt @@ -173,7 +173,7 @@ private fun ReorderableCategoryRow( } .zIndex(if (isDragging) 1f else 0f) ) { - // Drag handle (long-press to start dragging, mirrors ReorderableCarousel) + // Drag handle (long-press to start dragging, mirrors the Flow grid) Icon( imageVector = ImageVector.vectorResource(R.drawable.drag_handle_24px), contentDescription = null, diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/ReorderableCarousel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/ReorderableCarousel.kt deleted file mode 100644 index c4f6a80f..00000000 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/ReorderableCarousel.kt +++ /dev/null @@ -1,62 +0,0 @@ -package com.markel.flowstate.feature.flow.components - -import androidx.compose.animation.core.animateFloatAsState -import androidx.compose.foundation.interaction.MutableInteractionSource -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.PaddingValues -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.lazy.LazyRow -import androidx.compose.foundation.lazy.items -import androidx.compose.foundation.lazy.rememberLazyListState -import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember -import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.graphicsLayer -import androidx.compose.ui.unit.dp -import androidx.compose.ui.zIndex -import sh.calvin.reorderable.ReorderableItem -import sh.calvin.reorderable.rememberReorderableLazyListState - -@Composable -fun ReorderableCarousel( - items: List, - key: (T) -> Any, - onReorder: (from: Int, to: Int) -> Unit, - content: @Composable (T) -> Unit -) { - val rowState = rememberLazyListState() - val reorderableState = rememberReorderableLazyListState(rowState) { from, to -> - onReorder(from.index, to.index) - } - - LazyRow( - state = rowState, - contentPadding = PaddingValues(horizontal = 20.dp), - horizontalArrangement = Arrangement.spacedBy(12.dp), - modifier = Modifier.fillMaxWidth() - ) { - items(items, key = { key(it) }) { item -> - ReorderableItem(reorderableState, key = key(item)) { isDragging -> - val scale by animateFloatAsState( - targetValue = if (isDragging) 1.04f else 1f, - label = "carousel_drag_scale" - ) - Box( - modifier = Modifier - .longPressDraggableHandle( - interactionSource = remember { MutableInteractionSource() } - ) - .graphicsLayer { - scaleX = scale; scaleY = scale - alpha = if (isDragging) 0.92f else 1f - } - .zIndex(if (isDragging) 1f else 0f) - ) { - content(item) - } - } - } - } -} \ No newline at end of file diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt index 057e1a92..4a0392e2 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt @@ -6,19 +6,33 @@ import androidx.compose.animation.core.VisibilityThreshold import androidx.compose.animation.core.animateFloatAsState import androidx.compose.animation.core.spring import androidx.compose.animation.core.tween +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.interaction.MutableInteractionSource -import androidx.compose.foundation.layout.* -import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.LazyListState -import androidx.compose.foundation.lazy.LazyRow -import androidx.compose.foundation.lazy.items -import androidx.compose.foundation.lazy.itemsIndexed -import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.WindowInsets +import androidx.compose.foundation.layout.asPaddingValues +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.systemBars +import androidx.compose.foundation.lazy.staggeredgrid.LazyStaggeredGridItemScope +import androidx.compose.foundation.lazy.staggeredgrid.LazyStaggeredGridState +import androidx.compose.foundation.lazy.staggeredgrid.LazyVerticalStaggeredGrid +import androidx.compose.foundation.lazy.staggeredgrid.StaggeredGridCells +import androidx.compose.foundation.lazy.staggeredgrid.StaggeredGridItemSpan +import androidx.compose.foundation.lazy.staggeredgrid.itemsIndexed +import androidx.compose.foundation.lazy.staggeredgrid.rememberLazyStaggeredGridState import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.material3.* -import androidx.compose.runtime.* +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.RectangleShape import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight @@ -29,81 +43,85 @@ import androidx.compose.ui.zIndex import com.markel.flowstate.core.domain.CheckList import com.markel.flowstate.core.domain.Idea import com.markel.flowstate.core.domain.Task +import com.markel.flowstate.feature.flow.FlowItem import com.markel.flowstate.feature.flow.FlowUiState +import com.markel.flowstate.feature.flow.stableKey import com.markel.flowstate.feature.flow.tasks.components.AnimatableTaskItem import com.markel.flowstate.feature.flow.tasks.components.EmptyStateView import com.markel.flowstate.feature.flow.tasks.components.ReminderPermissionBanner -import sh.calvin.reorderable.ReorderableItem -import sh.calvin.reorderable.rememberReorderableLazyListState import com.markel.flowstate.feature.tasks.R -import kotlinx.coroutines.flow.StateFlow +import sh.calvin.reorderable.ReorderableCollectionItemScope +import sh.calvin.reorderable.ReorderableItem +import sh.calvin.reorderable.ReorderableLazyStaggeredGridState +import sh.calvin.reorderable.rememberReorderableLazyStaggeredGridState + +/** Minimum card width. ~390dp screens → 2 columns; tablets/landscape → 3–4. */ +private const val GRID_MIN_COLUMN_DP = 170 +private const val GRID_SPACING_DP = 10 +private const val GRID_EDGE_DP = 12 +/** Extra inset for full-width rows so tasks keep today's 20dp margins (12 + 8). */ +private const val ROW_EXTRA_INSET_DP = 8 +/** Bottom room for the FAB. */ +private const val GRID_BOTTOM_DP = 96 /** - * Main sectioned view for the Flow screen. + * Main view for the Flow screen: a single staggered grid. * * Layout: - * - Tasks → vertical reorderable list using [AnimatableTaskItem] - * - Lists → horizontal reorderable carousel using [CheckListGridCard] - * - Ideas → horizontal reorderable carousel using [IdeaGridCard] (taller cards) + * - Tasks → full-width rows using [AnimatableTaskItem] (swipe-to-dismiss) + * - CheckLists → cards, one per column, height driven by their content + * - Ideas → cards, one per column, height driven by their content * - * Future cross-section reordering: - * Replace per-section reorderable states with a unified drag state that tracks - * the source section and exposes a combined drop target across LazyList instances. + * Everything (headers included) is emitted from one [FlowItem] list, which is + * what lets the index reported by `reorderable` map 1:1 onto `items[index]`. + * Full-width rows opt in via [FlowItem.spanAllColumns]; cards take a single lane. */ +@OptIn(ExperimentalFoundationApi::class) @Composable fun SectionedFlowView( uiState: FlowUiState, onScrolled: () -> Unit, + onItemReorder: (fromIndex: Int, toIndex: Int) -> Unit, // Task callbacks onTaskClick: (Task) -> Unit, onTaskDelete: (Task) -> Unit, onTaskToggle: (Task) -> Unit, - onTaskReorder: (from: Int, to: Int) -> Unit, - // Idea callbacks + // Idea / CheckList callbacks onIdeaClick: (Idea) -> Unit, - onIdeaReorder: (from: Int, to: Int) -> Unit, - // CheckList callbacks onCheckListClick: (CheckList) -> Unit, - onCheckListReorder: (from: Int, to: Int) -> Unit, - showPermissionBanner : Boolean, + showPermissionBanner: Boolean, modifier: Modifier = Modifier, // Tracks per-task swipe-to-delete versions so that undo restores a fresh // SwipeToDismissBoxState instead of the stale EndToStart saved state. taskDeleteVersions: Map = emptyMap(), - categoriesEnabled: Boolean = false, // Hoisted from the caller (FlowScreen) so that the screen can derive - // FAB visibility from the same LazyListState that drives this list. - // See FlowScreen.kt → derivedStateOf { firstVisibleItemIndex == 0 && ... } - outerListState: LazyListState = rememberLazyListState(), + // FAB visibility from the same state that drives this grid. + gridState: LazyStaggeredGridState = rememberLazyStaggeredGridState(), ) { if (uiState !is FlowUiState.Success) return - // Empty state when all lists are empty - val allEmpty = uiState.tasks.isEmpty() - && uiState.checkLists.isEmpty() - && uiState.ideas.isEmpty() - - if (allEmpty) { + if (uiState.items.isEmpty()) { EmptyStateView(modifier = modifier.fillMaxSize()) return } - val reorderableState = rememberReorderableLazyListState(outerListState) { from, to -> - val headerOffset = 2 - val fromTaskIndex = from.index - headerOffset - val toTaskIndex = to.index - headerOffset - - if (fromTaskIndex in uiState.tasks.indices && toTaskIndex in uiState.tasks.indices) { - onTaskReorder(fromTaskIndex, toTaskIndex) - } - } + val reorderState = rememberReorderableLazyStaggeredGridState( + gridState, + scrollThresholdPadding = WindowInsets.systemBars.asPaddingValues(), + ) { from, to -> onItemReorder(from.index, to.index) } - LaunchedEffect(outerListState) { + LaunchedEffect(gridState) { snapshotFlow { - outerListState.firstVisibleItemIndex > 0 || outerListState.firstVisibleItemScrollOffset > 90 + gridState.firstVisibleItemIndex > 0 || gridState.firstVisibleItemScrollOffset > 90 }.collect { scrolled -> if (scrolled) onScrolled() } } + // Index of each task inside its own section, used to keep the rounded outer + // corners of the first / last / only task row. + val taskPositions = remember(uiState.tasks) { + uiState.tasks.mapIndexed { index, task -> task.id to index }.toMap() + } + val itemPlacementSpec = spring( dampingRatio = Spring.DampingRatioNoBouncy, stiffness = 520f, @@ -112,147 +130,180 @@ fun SectionedFlowView( val itemFadeInSpec = spring(stiffness = Spring.StiffnessMediumLow) val itemFadeOutSpec = tween(durationMillis = 130, easing = FastOutLinearInEasing) - LazyColumn( - state = outerListState, + LazyVerticalStaggeredGrid( + columns = StaggeredGridCells.Adaptive(minSize = GRID_MIN_COLUMN_DP.dp), + state = gridState, modifier = modifier.fillMaxSize(), - contentPadding = PaddingValues(bottom = 40.dp) + contentPadding = PaddingValues( + start = GRID_EDGE_DP.dp, + end = GRID_EDGE_DP.dp, + top = 8.dp, + bottom = GRID_BOTTOM_DP.dp, + ), + horizontalArrangement = Arrangement.spacedBy(GRID_SPACING_DP.dp), + verticalItemSpacing = 9.dp, ) { - // ── Tasks ────────────────────────────────────────────────────────── - if (uiState.tasks.isNotEmpty()) { - item(key = "tasks_header") { - SectionHeader( + itemsIndexed( + items = uiState.items, + key = { _, item -> item.stableKey(taskDeleteVersions) }, + span = { _, item -> + if (item.spanAllColumns) StaggeredGridItemSpan.FullLine + else StaggeredGridItemSpan.SingleLane + }, + ) { _, item -> + val animateItemModifier = Modifier.animateItem( + fadeInSpec = itemFadeInSpec, + placementSpec = itemPlacementSpec, + fadeOutSpec = itemFadeOutSpec + ) + + when (item) { + FlowItem.TasksHeader -> SectionHeader( title = stringResource(R.string.tasks_m), - modifier = Modifier - .animateItem( - fadeInSpec = itemFadeInSpec, - placementSpec = itemPlacementSpec, - fadeOutSpec = itemFadeOutSpec - ) - .padding(horizontal = 20.dp, vertical = 8.dp) + modifier = animateItemModifier + .padding(horizontal = ROW_EXTRA_INSET_DP.dp, vertical = 8.dp) + ) + + FlowItem.ReminderBanner -> ReminderPermissionBanner( + visible = showPermissionBanner, + modifier = animateItemModifier + .padding(horizontal = ROW_EXTRA_INSET_DP.dp) ) - } - item { - ReminderPermissionBanner(showPermissionBanner) - } - itemsIndexed( - items = uiState.tasks, - key = { _, task -> - // Include the delete version in the key so that after an undo the LazyColumn creates a fresh composable - Pair(task.id, taskDeleteVersions[task.id] ?: 0) - } - ) { index, task -> - val itemKey = Pair(task.id, taskDeleteVersions[task.id] ?: 0) - val itemShape = when { - uiState.tasks.size == 1 -> RoundedCornerShape(16.dp) - index == 0 -> RoundedCornerShape(topStart = 16.dp, topEnd = 16.dp, bottomStart = 4.dp, bottomEnd = 4.dp) - index == uiState.tasks.lastIndex -> RoundedCornerShape(topStart = 4.dp, topEnd = 4.dp, bottomStart = 16.dp, bottomEnd = 16.dp) - else -> RoundedCornerShape(4.dp) - } - ReorderableItem( - reorderableState, - key = itemKey, - animateItemModifier = Modifier.animateItem( - fadeInSpec = itemFadeInSpec, - placementSpec = itemPlacementSpec, - fadeOutSpec = itemFadeOutSpec - ) - ) { isDragging -> - val scale by animateFloatAsState( - targetValue = if (isDragging) 1.03f else 1f, - label = "task_drag_scale" - ) - Box( - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 20.dp, vertical = 1.dp) - .longPressDraggableHandle( - interactionSource = remember { MutableInteractionSource() } - ) - .graphicsLayer { - scaleX = scale; scaleY = scale - alpha = if (isDragging) 0.92f else 1f - } - .zIndex(if (isDragging) 1f else 0f) - ) { - AnimatableTaskItem( - task = task, - shape = itemShape, - onDelete = { onTaskDelete(task) }, - onComplete = { onTaskToggle(task) }, - onContentClick = { onTaskClick(task) } - ) - } - } - } - } - // ── CheckLists ───────────────────────────────────────────────────── - if (uiState.checkLists.isNotEmpty()) { - item(key = "checklists_header") { - SectionHeader( + FlowItem.CheckListsHeader -> SectionHeader( title = stringResource(R.string.checklists_m), - modifier = Modifier - .animateItem( - fadeInSpec = itemFadeInSpec, - placementSpec = itemPlacementSpec, - fadeOutSpec = itemFadeOutSpec - ) + modifier = animateItemModifier .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) ) - } - item(key = "checklists_carousel") { - Box(Modifier.animateItem(fadeInSpec = itemFadeInSpec, placementSpec = itemPlacementSpec, fadeOutSpec = itemFadeOutSpec).fillMaxWidth()) { - ReorderableCarousel( - items = uiState.checkLists, - key = { it.id }, - onReorder = onCheckListReorder - ) { checkList -> - CheckListGridCard( - checkList = checkList, - onClick = { onCheckListClick(checkList) }, - modifier = Modifier.width(160.dp) - ) - } - } - } - } - // ── Ideas ────────────────────────────────────────────────────────── - if (uiState.ideas.isNotEmpty()) { - item(key = "ideas_header") { - SectionHeader( + FlowItem.IdeasHeader -> SectionHeader( title = stringResource(R.string.ideas_notes_m), - modifier = Modifier - .animateItem( - fadeInSpec = itemFadeInSpec, - placementSpec = itemPlacementSpec, - fadeOutSpec = itemFadeOutSpec - ) + modifier = animateItemModifier .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) ) - } - item(key = "ideas_carousel") { - Box(Modifier.animateItem(fadeInSpec = itemFadeInSpec, placementSpec = itemPlacementSpec, fadeOutSpec = itemFadeOutSpec).fillMaxWidth()) { - ReorderableCarousel( - items = uiState.ideas, - key = { it.id }, - onReorder = onIdeaReorder - ) { idea -> - // Taller modifier to visually differentiate from checklist cards - IdeaGridCard( - idea = idea, - onClick = { onIdeaClick(idea) }, - modifier = Modifier - .width(200.dp) - .height(160.dp) - ) - } + + is FlowItem.TaskRow -> { + val taskIndex = taskPositions[item.task.id] ?: 0 + TaskRowContent( + reorderState = reorderState, + key = item.stableKey(taskDeleteVersions), + animateItemModifier = animateItemModifier, + task = item.task, + shape = taskRowShape(taskIndex, uiState.tasks.size), + onDelete = { onTaskDelete(item.task) }, + onComplete = { onTaskToggle(item.task) }, + onContentClick = { onTaskClick(item.task) }, + ) + } + + is FlowItem.CheckListCard -> ReorderableCard( + reorderState = reorderState, + key = item.stableKey(taskDeleteVersions), + animateItemModifier = animateItemModifier, + ) { + CheckListGridCard( + checkList = item.checkList, + onClick = { onCheckListClick(item.checkList) } + ) + } + + is FlowItem.IdeaCard -> ReorderableCard( + reorderState = reorderState, + key = item.stableKey(taskDeleteVersions), + animateItemModifier = animateItemModifier, + ) { + IdeaGridCard( + idea = item.idea, + onClick = { onIdeaClick(item.idea) } + ) } } } } } +/** Outer-corner rounding for the tasks block */ +private fun taskRowShape(index: Int, size: Int) = when { + size == 1 -> RoundedCornerShape(16.dp) + index == 0 -> RoundedCornerShape( + topStart = 16.dp, topEnd = 16.dp, bottomStart = 4.dp, bottomEnd = 4.dp + ) + index == size - 1 -> RoundedCornerShape( + topStart = 4.dp, topEnd = 4.dp, bottomStart = 16.dp, bottomEnd = 16.dp + ) + else -> RoundedCornerShape(4.dp) +} + +// ── Reorderable rows ────────────────────────────────────────────────────────── + +/** + * Shared shell for every draggable row: long-press anywhere on the item starts + * the drag, and the item lifts, scales and fades slightly while it is held. + */ +@Composable +private fun LazyStaggeredGridItemScope.ReorderableCard( + reorderState: ReorderableLazyStaggeredGridState, + key: Any, + animateItemModifier: Modifier, + content: @Composable ReorderableCollectionItemScope.(isDragging: Boolean) -> Unit, +) { + ReorderableItem( + state = reorderState, + key = key, + animateItemModifier = animateItemModifier, + ) { isDragging -> + val collectionScope = this + val scale by animateFloatAsState( + targetValue = if (isDragging) 1.04f else 1f, + label = "grid_drag_scale" + ) + Box( + modifier = Modifier + .longPressDraggableHandle(interactionSource = remember { MutableInteractionSource() }) + .graphicsLayer { + scaleX = scale + scaleY = scale + alpha = if (isDragging) 0.92f else 1f + } + .zIndex(if (isDragging) 1f else 0f) + ) { + collectionScope.content(isDragging) + } + } +} + +@Composable +private fun LazyStaggeredGridItemScope.TaskRowContent( + reorderState: ReorderableLazyStaggeredGridState, + key: Any, + animateItemModifier: Modifier, + task: Task, + shape: RoundedCornerShape, + onDelete: () -> Unit, + onComplete: () -> Unit, + onContentClick: () -> Unit, +) { + ReorderableCard( + reorderState = reorderState, + key = key, + animateItemModifier = animateItemModifier, + ) { + Box( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = ROW_EXTRA_INSET_DP.dp, vertical = 1.dp) + ) { + AnimatableTaskItem( + task = task, + shape = shape, + onDelete = onDelete, + onComplete = onComplete, + onContentClick = onContentClick, + ) + } + } +} + // ── Section header ──────────────────────────────────────────────────────────── @Composable diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt index c9338433..3a1aa2e4 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt @@ -1,5 +1,6 @@ package com.markel.flowstate.feature.flow +import app.cash.turbine.ReceiveTurbine import app.cash.turbine.test import com.markel.flowstate.core.data.UserPreferencesRepository import com.markel.flowstate.core.domain.Category @@ -273,10 +274,14 @@ class FlowViewModelTest { } } - // ── Reorder Logic ───────────────────────────────────────────────────────── + // ── Reorder logic ───────────────────────────────────────────────────────── + // + // onItemReorder() takes indices into the FLAT item list (headers included), + // so every test below locates the row it wants instead of hardcoding an + // offset. That is the whole point of the FlowItem model. @Test - fun onTaskReorder_updatesStateOptimistically_andCallsRepository() = runTest { + fun onItemReorder_withinTasks_reordersAndPersists() = runTest { // GIVEN val t1 = Task(id = 1, title = "T1", isDone = false, position = 0) val t2 = Task(id = 2, title = "T2", isDone = false, position = 1) @@ -285,36 +290,27 @@ class FlowViewModelTest { viewModel = createViewModel(tasks = listOf(t1, t2, t3)) viewModel.uiState.test { - val initialState = awaitItem() - val success = if (initialState is FlowUiState.Loading) awaitItem() as FlowUiState.Success else initialState as FlowUiState.Success - assertEquals(3, success.tasks.size) - - // WHEN - Move the first task (index 0) to the end (index 2) -> Expected: T2, T3, T1 - viewModel.onTaskReorder(fromIndex = 0, toIndex = 2) + val before = awaitSuccess() - // THEN - Local state is updated - val updatedState = awaitItem() as FlowUiState.Success - assertEquals("T2", updatedState.tasks[0].title) - assertEquals("T3", updatedState.tasks[1].title) - assertEquals("T1", updatedState.tasks[2].title) + // WHEN - drag the first task row down to the last task slot + val from = before.indexOfTask(1) + val to = before.items.indexOfLast { it is FlowItem.TaskRow } + viewModel.onItemReorder(fromIndex = from, toIndex = to) - // Positions were recalculated correctly (0, 1, 2) - assertEquals(0, updatedState.tasks[0].position) - assertEquals(1, updatedState.tasks[1].position) - assertEquals(2, updatedState.tasks[2].position) + // THEN + val after = awaitItem() as FlowUiState.Success + assertEquals(listOf("T2", "T3", "T1"), after.tasks.map { it.title }) + // Positions are renumbered so the DAOs (ORDER BY position) keep the order + assertEquals(listOf(0, 1, 2), after.tasks.map { it.position }) } - // AND - Repository is called coVerify { - taskRepository.updateTasksOrder(match { list -> - list[0].id == 2 && list[0].position == 0 && - list[2].id == 1 && list[2].position == 2 - }) + taskRepository.updateTasksOrder(match { list -> list.map { it.id } == listOf(2, 3, 1) }) } } @Test - fun onIdeaReorder_updatesStateOptimistically_andCallsRepository() = runTest { + fun onItemReorder_withinIdeas_reordersAndPersists() = runTest { // GIVEN val i1 = Idea(id = 1, title = "I1", position = 0, content = "Content", color = 0L) val i2 = Idea(id = 2, title = "I2", position = 1, content = "Content", color = 0L) @@ -322,27 +318,26 @@ class FlowViewModelTest { viewModel = createViewModel(ideas = listOf(i1, i2)) viewModel.uiState.test { - val initialState = awaitItem() - val startState = if (initialState is FlowUiState.Loading) awaitItem() as FlowUiState.Success else initialState as FlowUiState.Success - - // WHEN - Swap I1 and I2 - viewModel.onIdeaReorder(fromIndex = 0, toIndex = 1) + val before = awaitSuccess() + // WHEN - swap the two idea cards + viewModel.onItemReorder( + fromIndex = before.indexOfIdea(1), + toIndex = before.indexOfIdea(2), + ) // THEN - val updatedState = awaitItem() as FlowUiState.Success - assertEquals("I2", updatedState.ideas[0].title) - assertEquals("I1", updatedState.ideas[1].title) + val after = awaitItem() as FlowUiState.Success + assertEquals(listOf("I2", "I1"), after.ideas.map { it.title }) + assertEquals(listOf(0, 1), after.ideas.map { it.position }) } coVerify { - ideaRepository.updateIdeasOrder(match { list -> - list[0].id == 2 && list[1].id == 1 - }) + ideaRepository.updateIdeasOrder(match { list -> list.map { it.id } == listOf(2, 1) }) } } @Test - fun onCheckListReorder_updatesStateOptimistically_andCallsRepository() = runTest { + fun onItemReorder_withinCheckLists_reordersAndPersists() = runTest { // GIVEN val c1 = CheckList(id = 1, title = "C1", position = 0, color = 0L) val c2 = CheckList(id = 2, title = "C2", position = 1, color = 0L) @@ -350,25 +345,99 @@ class FlowViewModelTest { viewModel = createViewModel(checkLists = listOf(c1, c2)) viewModel.uiState.test { - val initialState = awaitItem() - val startState = if (initialState is FlowUiState.Loading) awaitItem() as FlowUiState.Success else initialState as FlowUiState.Success - - // WHEN - viewModel.onCheckListReorder(fromIndex = 1, toIndex = 0) + val before = awaitSuccess() + // WHEN - drag C2 in front of C1 + viewModel.onItemReorder( + fromIndex = before.indexOfCheckList(2), + toIndex = before.indexOfCheckList(1), + ) // THEN - val updatedState = awaitItem() as FlowUiState.Success - assertEquals("C2", updatedState.checkLists[0].title) - assertEquals("C1", updatedState.checkLists[1].title) + val after = awaitItem() as FlowUiState.Success + assertEquals(listOf("C2", "C1"), after.checkLists.map { it.title }) + assertEquals(listOf(0, 1), after.checkLists.map { it.position }) } coVerify { - checkListRepository.updateCheckListsOrder(match { list -> - list[0].id == 2 && list[1].id == 1 - }) + checkListRepository.updateCheckListsOrder(match { list -> list.map { it.id } == listOf(2, 1) }) + } + } + + @Test + fun onItemReorder_acrossSections_isRejected() = runTest { + // GIVEN - both sections populated + val idea = Idea(id = 1, title = "I1", position = 0, content = "Content", color = 0L) + val list = CheckList(id = 2, title = "C1", position = 0, color = 0L) + + viewModel = createViewModel(ideas = listOf(idea), checkLists = listOf(list)) + + viewModel.uiState.test { + val before = awaitSuccess() + cancelAndIgnoreRemainingEvents() + + // WHEN - try to drop the idea card onto the checklist card + viewModel.onItemReorder( + fromIndex = before.indexOfIdea(1), + toIndex = before.indexOfCheckList(2), + ) + + // THEN - nothing moved and nothing was written + val after = viewModel.uiState.value as FlowUiState.Success + assertEquals(before.ideas, after.ideas) + assertEquals(before.checkLists, after.checkLists) } + + coVerify(exactly = 0) { ideaRepository.updateIdeasOrder(any()) } + coVerify(exactly = 0) { checkListRepository.updateCheckListsOrder(any()) } } + @Test + fun onItemReorder_ontoAHeader_isRejected() = runTest { + // GIVEN + val i1 = Idea(id = 1, title = "I1", position = 0, content = "Content", color = 0L) + + viewModel = createViewModel(ideas = listOf(i1)) + + viewModel.uiState.test { + val before = awaitSuccess() + cancelAndIgnoreRemainingEvents() + + // WHEN - drop the idea card onto its own section header + viewModel.onItemReorder( + fromIndex = before.indexOfIdea(1), + toIndex = before.items.indexOf(FlowItem.IdeasHeader), + ) + + // THEN + val after = viewModel.uiState.value as FlowUiState.Success + assertEquals(before.ideas, after.ideas) + } + + coVerify(exactly = 0) { ideaRepository.updateIdeasOrder(any()) } + } + + @Test + fun onItemReorder_withOutOfBoundsIndices_isIgnored() = runTest { + // GIVEN + val i1 = Idea(id = 1, title = "I1", position = 0, content = "Content", color = 0L) + + viewModel = createViewModel(ideas = listOf(i1)) + + viewModel.uiState.test { + val before = awaitSuccess() + cancelAndIgnoreRemainingEvents() + + // WHEN + viewModel.onItemReorder(fromIndex = -1, toIndex = 99) + viewModel.onItemReorder(fromIndex = 0, toIndex = 99) + + // THEN + val after = viewModel.uiState.value as FlowUiState.Success + assertEquals(before.ideas, after.ideas) + } + + coVerify(exactly = 0) { ideaRepository.updateIdeasOrder(any()) } + } // ── Banner logic ────────────────────────────────────────────────────────── @@ -801,4 +870,23 @@ class FlowViewModelTest { } } -} \ No newline at end of file + + // ── Helpers ─────────────────────────────────────────────────────────────── + + /** Consumes the initial `Loading` emission when there is one. */ + private suspend fun ReceiveTurbine.awaitSuccess(): FlowUiState.Success { + val first = awaitItem() + return if (first is FlowUiState.Success) first else awaitItem() as FlowUiState.Success + } + + // Locators over the FLAT item list: the reorder API speaks in item indices, + // not in per-section indices, so tests must not assume a header offset. + private fun FlowUiState.Success.indexOfTask(id: Int) = + items.indexOfFirst { it is FlowItem.TaskRow && it.task.id == id } + + private fun FlowUiState.Success.indexOfIdea(id: Int) = + items.indexOfFirst { it is FlowItem.IdeaCard && it.idea.id == id } + + private fun FlowUiState.Success.indexOfCheckList(id: Int) = + items.indexOfFirst { it is FlowItem.CheckListCard && it.checkList.id == id } +} From c672a15974c224e7121fe3ce447811e050b07c69 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sat, 17 Oct 2026 01:25:17 +0200 Subject: [PATCH 03/11] refactor: task row with same padding as before Also fixed a bug introduced in the latest commit that caused the reminder permission banner look strange. --- .../feature/flow/components/SectionedFlowView.kt | 16 +++++++++++++--- .../tasks/components/ReminderPermissionBanner.kt | 6 +++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt index 4a0392e2..8196bd49 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt @@ -63,6 +63,14 @@ private const val GRID_EDGE_DP = 12 private const val ROW_EXTRA_INSET_DP = 8 /** Bottom room for the FAB. */ private const val GRID_BOTTOM_DP = 96 +/** + * Vertical gap between grid cards (checklists / ideas). + * + * The grid itself uses `verticalItemSpacing = 0` because that value applies to + * *every* row — including the task rows, which must stay tightly grouped + * Cards get this gap via bottom padding instead. + */ +private const val CARD_VERTICAL_SPACING_DP = 9 /** * Main view for the Flow screen: a single staggered grid. @@ -141,7 +149,7 @@ fun SectionedFlowView( bottom = GRID_BOTTOM_DP.dp, ), horizontalArrangement = Arrangement.spacedBy(GRID_SPACING_DP.dp), - verticalItemSpacing = 9.dp, + verticalItemSpacing = 0.dp, ) { itemsIndexed( items = uiState.items, @@ -203,7 +211,8 @@ fun SectionedFlowView( ) { CheckListGridCard( checkList = item.checkList, - onClick = { onCheckListClick(item.checkList) } + onClick = { onCheckListClick(item.checkList) }, + modifier = Modifier.padding(bottom = CARD_VERTICAL_SPACING_DP.dp) ) } @@ -214,7 +223,8 @@ fun SectionedFlowView( ) { IdeaGridCard( idea = item.idea, - onClick = { onIdeaClick(item.idea) } + onClick = { onIdeaClick(item.idea) }, + modifier = Modifier.padding(bottom = CARD_VERTICAL_SPACING_DP.dp) ) } } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/ReminderPermissionBanner.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/ReminderPermissionBanner.kt index 48ed8806..6c0478f2 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/ReminderPermissionBanner.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/ReminderPermissionBanner.kt @@ -41,6 +41,10 @@ import androidx.core.net.toUri * * The visibility check (hasTasksWithReminder && !canScheduleExactAlarms) is done * by the caller (SectionedFlowView or equivalent) so this composable stays dumb. + * + * Horizontal insets are also the caller's responsibility: the Flow grid already + * provides them (12dp contentPadding + 8dp on the row), so the banner must not + * add its own horizontal padding, or it ends up narrower than the task rows. */ @Composable fun ReminderPermissionBanner( @@ -58,7 +62,7 @@ fun ReminderPermissionBanner( Row( modifier = Modifier .fillMaxWidth() - .padding(start = 20.dp, end = 20.dp, top = 4.dp, bottom = 10.dp) + .padding(top = 4.dp, bottom = 10.dp) .background( color = MaterialTheme.colorScheme.errorContainer, shape = RoundedCornerShape(16.dp) From 3a8ad695c5c38eeedeba6a6361c7bb28b9902670 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sat, 17 Oct 2026 02:05:14 +0200 Subject: [PATCH 04/11] fix: stop deleted task from flashing back when the undo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Race: the expiry job deleted the task from Room and immediately removed it from _pendingUndoTasks. Room delivers invalidations asynchronously, so the combine often recomputed with the stale pre-deletion task list while the filter no longer excluded the id — the task passed through and flashed on screen for a few frames until the fresh list arrived. Fix: on expiry the id is handed over to a new _committedDeleteIds set (before leaving _pendingUndoTasks, so it is never unfiltered in between) which keeps it excluded from the UI. The combine itself drops committed ids once its DB snapshot no longer contains them — the cleanup runs exactly on the emission that proves the deletion, closing the race for every interleaving. Undo FAB semantics are unchanged: it only tracks the still-undoable map. --- .../flowstate/feature/flow/FlowViewModel.kt | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt index 704c1302..5670e6c6 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt @@ -77,6 +77,16 @@ class FlowViewModel @Inject constructor( /** Tasks that have been swiped-to-dismiss but not yet permanently deleted. */ private val _pendingUndoTasks = MutableStateFlow>(emptyMap()) + /** + * Ids of tasks whose undo countdown expired and were deleted from the DB. + * + * They keep being filtered out of the UI (together with [_pendingUndoTasks]) + * until a DB emission confirms the task is really gone; the combine below + * then drops the id (self-healing). + * + */ + private val _committedDeleteIds = MutableStateFlow>(emptySet()) + /** Whether the undo FAB should be visible – true while any deletion is pending. */ val showUndoButton: StateFlow = _pendingUndoTasks .map { it.isNotEmpty() } @@ -112,7 +122,7 @@ class FlowViewModel @Inject constructor( // ── Init: combine repos + pending-filter ────────────────────────────────── init { - // Restore the last visited categoom DataStory frre once at startup. + // Restore the last visited category DataStory free once at startup. viewModelScope.launch { userPreferencesRepository.lastCategoryId.collect { saved -> if (!lastCategoryRestored && saved != null) { @@ -137,10 +147,21 @@ class FlowViewModel @Inject constructor( coreDataFlow, userPreferencesRepository.categoriesEnabled, _pendingUndoTasks, + _committedDeleteIds, _selectedCategoryId - ) { coreData, categoriesEnabled, pendingMap, selectedCategoryId -> - - val pendingIds = pendingMap.keys + ) { coreData, categoriesEnabled, pendingMap, committedIds, selectedCategoryId -> + + // Ids hidden from the list: still-undoable deletions plus the + // committed ones whose absence the DB has not confirmed yet. + val pendingIds = pendingMap.keys + committedIds + + // Self-heal committed deletions: as soon as this emission's task + // list no longer contains the id, it can be dropped. + val dbTaskIds = coreData.tasks.mapTo(HashSet()) { it.id } + val confirmedGone = committedIds.filterNot { it in dbTaskIds } + if (confirmedGone.isNotEmpty()) { + _committedDeleteIds.update { it - confirmedGone.toSet() } + } // If the persisted/selected category no longer exists, fall back to General // and persist the correction so we don't keep pointing at a phantom id. @@ -262,8 +283,11 @@ class FlowViewModel @Inject constructor( * The task is **not** deleted from the DB immediately. Instead, it is added * to [_pendingUndoTasks] which causes the `combine` above to filter it out * of the UI state optimistically. A per-task countdown of 3500ms - * starts; if it expires the task is permanently deleted. The user may cancel - * the countdown by pressing the undo FAB ([undoPendingDeletions]). + * starts; if it expires the task is permanently deleted and its id moves to + * [_committedDeleteIds], which keeps filtering it until the DB emission + * confirms the deletion. + * The user may cancel the countdown by pressing the undo FAB + * ([undoPendingDeletions]). */ fun onTaskSwiped(task: Task) { // Add to the pending map – the combine will filter it out of uiState @@ -284,7 +308,7 @@ class FlowViewModel @Inject constructor( // Time's up – permanently delete from DB reminderScheduler.cancel(task.id) deleteTaskUseCase(task) - // Clean up pending map; task is already gone from DB at this point + _committedDeleteIds.update { it + task.id } _pendingUndoTasks.update { it - task.id } deleteJobs.remove(task.id) } @@ -297,6 +321,10 @@ class FlowViewModel @Inject constructor( * that was waiting to be deleted. The tasks are still in the DB, so * simply removing them from [_pendingUndoTasks] makes them reappear in * the UI state automatically. + * + * Deletions whose countdown already expired (ids in [_committedDeleteIds]) + * are **not** restored — they are gone from the DB and no longer undoable; + * their ids stay in the filter set until the combine self-heals them. */ fun undoPendingDeletions() { // Cancel every pending countdown From 28ed2d43cdbe0be0fa6c155ac42f156618864e44 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sat, 17 Oct 2026 02:47:42 +0200 Subject: [PATCH 05/11] fix: drop grid-level animateItem from the reminder banner row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The banner row inherited the shared animateItemModifier in the staggered grid refactor, but the pre-grid LazyColumn rendered it as a plain item because the banner owns its enter/exit animation (AnimatedVisibility expand/shrink + fade). The layered animations collide when the tasks section disappears (last task deleted/completed): the item leaves the list while its internal AnimatedVisibility starts collapsing, and animateItem's fadeOut keeps a ghost of the banner — fading and shrinking at the same time — floating over the content that already slides up with the placement spring. The result is the fast upward shift followed by a visible flicker. Without animateItem the banner is removed instantly in the same layout pass, exactly like the old LazyColumn, and its AnimatedVisibility stays the single animation authority while it remains in the list. --- .../flowstate/feature/flow/components/SectionedFlowView.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt index 8196bd49..1189dfea 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt @@ -171,11 +171,9 @@ fun SectionedFlowView( modifier = animateItemModifier .padding(horizontal = ROW_EXTRA_INSET_DP.dp, vertical = 8.dp) ) - FlowItem.ReminderBanner -> ReminderPermissionBanner( visible = showPermissionBanner, - modifier = animateItemModifier - .padding(horizontal = ROW_EXTRA_INSET_DP.dp) + modifier = Modifier.padding(horizontal = ROW_EXTRA_INSET_DP.dp) // Deliberately WITHOUT animateItem: the banner owns its enter/exit animation through its internal AnimatedVisibility ) FlowItem.CheckListsHeader -> SectionHeader( From 5ff45576c581d50708779f561f002b70af396b2a Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sat, 17 Oct 2026 16:56:37 +0200 Subject: [PATCH 06/11] fix: stop vertical contentPadding from skewing the item animator Symptom ------- After the switch from LazyColumn to LazyVerticalStaggeredGrid, completing or deleting a task made the rows below it move up, but any row that was outside the viewport and got pulled into view during that animation did not travel with the rest of the list: it appeared a few frames late, visibly detached from the row above and then slid into place. With the reminder banner on screen the artifact was much easier to spot, because the banner reveals several rows at once; that made it look like a second, banner-specific bug, but the mechanism is the same one described here. Root cause ---------- LazyColumn and LazyVerticalStaggeredGrid do not report the same "end of content" to the item animator (foundation 1.10.x / 1.11.x): LazyListMeasure.kt itemAnimator.onMeasured(layoutMaxOffset = currentMainAxisOffset, ...) LazyStaggeredGridMeasure.kt:933 itemAnimator.onMeasured( layoutMaxOffset = currentItemOffsets.max() + contentPadding, ...) `contentPadding` there is the *sum* of the before/after main-axis paddings. This screen used `top = 8.dp, bottom = 96.dp` (room for the FAB), so the staggered grid reported the content end 104 dp further down than it really is. That offset only matters for rows that are measured for the first time in a given pass and therefore have no previous ItemInfo to animate from. They take the `movingInFromEndBound` branch of LazyLayoutItemAnimator.onMeasured, which initializes their animation at layoutMaxOffset + accumulatedOffset - mainAxisSizeWithSpacings i.e. 104 dp (312 px at 3x) *below* the position the row occupied before the removal. The row is born below its real slot and has to travel that extra distance with the placement spring, so it enters the viewport later than the rows that animate from their real previous positions, and visibly disconnected from them. Fix --- Do not put main-axis padding on the grid: * the 8 dp top gap is applied to the first row instead (it is always a header), so it is part of the measured content. * the 96 dp bottom room for the FAB becomes a real item, FlowItem.BottomSpacer (see its KDoc for the reasoning). With both of them inside the content, the animator's end bound equals the real content end and a row entering from below starts exactly at the position it had before the removal, which is the behavior the screen had with LazyColumn. --- .../markel/flowstate/feature/flow/FlowItem.kt | 8 ++++ .../flowstate/feature/flow/FlowUiState.kt | 3 ++ .../flow/components/SectionedFlowView.kt | 41 ++++++++++++++----- .../flowstate/feature/flow/FlowItemTest.kt | 18 ++++++-- 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt index d6e3fa12..ccbdc735 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt @@ -65,6 +65,13 @@ sealed interface FlowItem { override val section: Section? get() = Section.IDEAS override val spanAllColumns: Boolean get() = false } + + // ── Chrome ──────────────────────────────────────────────────────────────── + + /** + * Empty room after the last section so the FAB never covers content. + */ + data object BottomSpacer : FlowItem } /** @@ -88,4 +95,5 @@ internal fun FlowItem.stableKey(taskDeleteVersions: Map = emptyMap()): is FlowItem.CheckListCard -> "cl_${checkList.id}" FlowItem.IdeasHeader -> "ideas_header" is FlowItem.IdeaCard -> "idea_${idea.id}" + FlowItem.BottomSpacer -> "bottom_spacer" } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt index 1ba9b1a7..9c978c84 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt @@ -34,6 +34,9 @@ sealed interface FlowUiState { add(FlowItem.IdeasHeader) ideas.forEach { add(FlowItem.IdeaCard(it)) } } + // Only when there is something to scroll: an empty screen shows + // EmptyStateView and must keep `items.isEmpty()` true. + if (isNotEmpty()) add(FlowItem.BottomSpacer) } } } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt index 1189dfea..d67468de 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt @@ -11,10 +11,12 @@ import androidx.compose.foundation.interaction.MutableInteractionSource import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.asPaddingValues import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.systemBars import androidx.compose.foundation.lazy.staggeredgrid.LazyStaggeredGridItemScope @@ -59,9 +61,11 @@ import sh.calvin.reorderable.rememberReorderableLazyStaggeredGridState private const val GRID_MIN_COLUMN_DP = 170 private const val GRID_SPACING_DP = 10 private const val GRID_EDGE_DP = 12 +/** Top breathing room, applied to the first row instead of the container (see below). */ +private const val GRID_TOP_DP = 8 /** Extra inset for full-width rows so tasks keep today's 20dp margins (12 + 8). */ private const val ROW_EXTRA_INSET_DP = 8 -/** Bottom room for the FAB. */ +/** Bottom room for the FAB, rendered as [FlowItem.BottomSpacer] (see below). */ private const val GRID_BOTTOM_DP = 96 /** * Vertical gap between grid cards (checklists / ideas). @@ -142,11 +146,10 @@ fun SectionedFlowView( columns = StaggeredGridCells.Adaptive(minSize = GRID_MIN_COLUMN_DP.dp), state = gridState, modifier = modifier.fillMaxSize(), + // ── Keep the *main-axis* (top / bottom) content padding at 0 ───────────── contentPadding = PaddingValues( start = GRID_EDGE_DP.dp, end = GRID_EDGE_DP.dp, - top = 8.dp, - bottom = GRID_BOTTOM_DP.dp, ), horizontalArrangement = Arrangement.spacedBy(GRID_SPACING_DP.dp), verticalItemSpacing = 0.dp, @@ -158,33 +161,41 @@ fun SectionedFlowView( if (item.spanAllColumns) StaggeredGridItemSpan.FullLine else StaggeredGridItemSpan.SingleLane }, - ) { _, item -> + ) { index, item -> val animateItemModifier = Modifier.animateItem( fadeInSpec = itemFadeInSpec, placementSpec = itemPlacementSpec, fadeOutSpec = itemFadeOutSpec ) + // Top gap of the whole grid, applied to its first row (always a header) + // instead of to contentPadding. See the comment on contentPadding above. + val topInsetModifier = + if (index == 0) Modifier.padding(top = GRID_TOP_DP.dp) else Modifier + val itemModifier = animateItemModifier.then(topInsetModifier) + when (item) { FlowItem.TasksHeader -> SectionHeader( title = stringResource(R.string.tasks_m), - modifier = animateItemModifier + modifier = itemModifier .padding(horizontal = ROW_EXTRA_INSET_DP.dp, vertical = 8.dp) ) FlowItem.ReminderBanner -> ReminderPermissionBanner( visible = showPermissionBanner, - modifier = Modifier.padding(horizontal = ROW_EXTRA_INSET_DP.dp) // Deliberately WITHOUT animateItem: the banner owns its enter/exit animation through its internal AnimatedVisibility + modifier = Modifier + .padding(horizontal = ROW_EXTRA_INSET_DP.dp) + .then(topInsetModifier) // Deliberately WITHOUT animateItem: the banner owns its enter/exit animation through its internal AnimatedVisibility ) FlowItem.CheckListsHeader -> SectionHeader( title = stringResource(R.string.checklists_m), - modifier = animateItemModifier + modifier = itemModifier .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) ) FlowItem.IdeasHeader -> SectionHeader( title = stringResource(R.string.ideas_notes_m), - modifier = animateItemModifier + modifier = itemModifier .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) ) @@ -193,7 +204,7 @@ fun SectionedFlowView( TaskRowContent( reorderState = reorderState, key = item.stableKey(taskDeleteVersions), - animateItemModifier = animateItemModifier, + animateItemModifier = itemModifier, task = item.task, shape = taskRowShape(taskIndex, uiState.tasks.size), onDelete = { onTaskDelete(item.task) }, @@ -205,7 +216,7 @@ fun SectionedFlowView( is FlowItem.CheckListCard -> ReorderableCard( reorderState = reorderState, key = item.stableKey(taskDeleteVersions), - animateItemModifier = animateItemModifier, + animateItemModifier = itemModifier, ) { CheckListGridCard( checkList = item.checkList, @@ -217,7 +228,7 @@ fun SectionedFlowView( is FlowItem.IdeaCard -> ReorderableCard( reorderState = reorderState, key = item.stableKey(taskDeleteVersions), - animateItemModifier = animateItemModifier, + animateItemModifier = itemModifier, ) { IdeaGridCard( idea = item.idea, @@ -225,6 +236,14 @@ fun SectionedFlowView( modifier = Modifier.padding(bottom = CARD_VERTICAL_SPACING_DP.dp) ) } + + // No animateItem: an empty spacer has nothing to animate, and + // keeping it out of the animator makes the bottom bound stable. + FlowItem.BottomSpacer -> Spacer( + modifier = Modifier + .fillMaxWidth() + .height(GRID_BOTTOM_DP.dp) + ) } } } diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt index 5aa0f030..5b72784a 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt @@ -60,6 +60,7 @@ class FlowItemTest { FlowItem.CheckListCard(checkList(1)), FlowItem.IdeasHeader, FlowItem.IdeaCard(idea(1)), + FlowItem.BottomSpacer, ), items ) @@ -70,12 +71,17 @@ class FlowItemTest { assertEquals(emptyList(), state().items) assertEquals( - listOf(FlowItem.TasksHeader, FlowItem.ReminderBanner, FlowItem.TaskRow(task(1))), + listOf( + FlowItem.TasksHeader, + FlowItem.ReminderBanner, + FlowItem.TaskRow(task(1)), + FlowItem.BottomSpacer, + ), state(tasks = listOf(task(1))).items ) assertEquals( - listOf(FlowItem.IdeasHeader, FlowItem.IdeaCard(idea(1))), + listOf(FlowItem.IdeasHeader, FlowItem.IdeaCard(idea(1)), FlowItem.BottomSpacer), state(ideas = listOf(idea(1))).items ) } @@ -88,10 +94,10 @@ class FlowItemTest { @Test fun rows_isRecomputedAfterCopy() { val original = state(tasks = listOf(task(1)), ideas = listOf(idea(1))) - assertEquals(5, original.items.size) + assertEquals(6, original.items.size) val withoutIdeas = original.copy(ideas = emptyList()) - assertEquals(3, withoutIdeas.items.size) + assertEquals(4, withoutIdeas.items.size) val withCheckLists = withoutIdeas.copy(checkLists = listOf(checkList(1))) assertEquals( @@ -101,6 +107,7 @@ class FlowItemTest { FlowItem.TaskRow(task(1)), FlowItem.CheckListsHeader, FlowItem.CheckListCard(checkList(1)), + FlowItem.BottomSpacer, ), withCheckLists.items ) @@ -118,6 +125,7 @@ class FlowItemTest { FlowItem.CheckListCard(checkList(1)), FlowItem.IdeasHeader, FlowItem.IdeaCard(idea(1)), + FlowItem.BottomSpacer, ) assertEquals(items.size, items.map { it.stableKey() }.toSet().size) @@ -218,6 +226,7 @@ class FlowItemTest { assertNull(FlowItem.ReminderBanner.section) assertNull(FlowItem.CheckListsHeader.section) assertNull(FlowItem.IdeasHeader.section) + assertNull(FlowItem.BottomSpacer.section) assertEquals(FlowItem.Section.TASKS, FlowItem.TaskRow(task(1)).section) assertEquals(FlowItem.Section.CHECKLISTS, FlowItem.CheckListCard(checkList(1)).section) @@ -231,6 +240,7 @@ class FlowItemTest { assertTrue(FlowItem.CheckListsHeader.spanAllColumns) assertTrue(FlowItem.IdeasHeader.spanAllColumns) assertTrue(FlowItem.TaskRow(task(1)).spanAllColumns) + assertTrue(FlowItem.BottomSpacer.spanAllColumns) assertFalse(FlowItem.CheckListCard(checkList(1)).spanAllColumns) assertFalse(FlowItem.IdeaCard(idea(1)).spanAllColumns) From 2ad37a814d21879df86e3d0081904fab4ab582fd Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 18 Oct 2026 01:50:07 +0200 Subject: [PATCH 07/11] fix: pin the reminder banner above the grid instead of listing it Symptom ------- Deleting the task that owns the reminder also hides the "exact alarms" permission banner. When that happened, the rows below the viewport did not move with the list: they stayed hidden during the animation and then appeared, already parked in their final position. The rows that were already composed did move smoothly, which made the bug look like "only the last two rows pop in late". Root cause ---------- The banner was a FlowItem, i.e. one more row of the LazyVerticalStaggeredGrid, so its height was part of the content geometry. A row that has never been composed has no previous position to animate from: LazyLayoutItemAnimator synthesizes its start offset from the *end of the measured content* of that pass (the same bound the previous commit took out of contentPadding, see onMeasured -> movingInFromEndBound). Two consequences: * with AnimatedVisibility(shrinkVertically) the banner changed its height on every frame of the exit, so the content end was a moving target for ~300 ms, and the collapse lands in the same frame as the task removal (uiState and showReminderBanner are two StateFlows the ViewModel updates back to back). Rows entering from below were initialized from the wrong bound: the first one was born glued to the row above (gap 61 px instead of 137 px) and then slid into place. * collapsing the height in a single frame (fading it out and taking it out of the layout at once) fixes the glue but not the pop: the rows are simply re-placed at their final offset, which is already inside the viewport, so they appear planted mid-screen with no motion. There is no in-grid configuration that solves both at once: any change in the content geometry either gives the entering rows a wrong synthetic start offset, or gives them no animation at all. Fix --- Take the banner out of the lazy container. ReminderPermissionBanner is rendered in a Column above the LazyVerticalStaggeredGrid, so the height it animates is the height of the grid's *viewport* instead of the height of an item: * the grid's content offsets never change while the banner collapses, so the grid translates rigidly as a single layer while the parent shrinks the space above it, and every row keeps its relative position; * the rows it reveals come in through the bottom edge and travel with the rest of the list, exactly like during a scroll. FlowItem.ReminderBanner is removed, together with its entry in FlowUiState.items and the FlowItemTest expectations. The banner keeps its expand/shrink + fade animation, which is safe again outside the lazy layout. Behavior note ------------- The banner no longer scrolls away with the list: it stays pinned above it while the permission is missing. That is the usual pattern for a permission banner, and it is what makes the collapse free of item animations. Alternatives considered ----------------------- 1. Keeping the banner as a plain list item and letting the list animate it (no AnimatedVisibility at all): the approach that fixed issue #24. Measured in the same harness it does not work here: the rows that enter from below are still born at a synthetic offset (the gap above task12 is 125 px instead of 50 px, and the first checklist is initialized at 1075 px instead of 788 px), and they need ~350 ms to converge, so the "detached tail" artifact stays. The difference from #24: there the item that traveled wrongly had been composed before, so it had a previous offset and animateItem() only had to use it. Here the rows that misbehave are precisely the ones that were never composed, and the only thing the animator knows about them is the end of the measured content - which the banner moves when it leaves the list. 2. Fading the banner and collapsing its height in a single frame: no glue, but the entering rows are re-placed straight at their final offset and appear mid-screen without any motion (788 px in the harness, exactly where they end). 3. Keeping the height animation (the state before this commit): rows are born glued to the row above (gap 61 px instead of 137 px) and slide into place. Only taking the banner out of the lazy content removes the cause, because then the layout it changes is the viewport's, not the content's. --- .../markel/flowstate/feature/flow/FlowItem.kt | 15 +- .../flowstate/feature/flow/FlowUiState.kt | 5 +- .../flow/components/SectionedFlowView.kt | 182 +++++++++--------- .../components/ReminderPermissionBanner.kt | 29 ++- .../flowstate/feature/flow/FlowItemTest.kt | 11 +- 5 files changed, 123 insertions(+), 119 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt index ccbdc735..ab5059bc 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt @@ -7,15 +7,15 @@ import com.markel.flowstate.core.domain.Task /** * Flat item model for the Flow screen. * - * Every item the screen renders — including section headers and the reminder - * banner — is one of these. That makes the index reported by the `reorderable` - * library (`from.index` / `to.index`) map 1:1 onto `items[index]`, which removes - * the hardcoded `headerOffset` arithmetic that `SectionedFlowView` needs while - * headers are emitted as independent `item { }` blocks. + * Every row the grid renders — including the section headers — is one of these. + * That makes the index reported by the `reorderable` library (`from.index` / + * `to.index`) map 1:1 onto `items[index]`, which removes the hardcoded + * `headerOffset` arithmetic that `SectionedFlowView` needs while headers are + * emitted as independent `item { }` blocks. * * Reordering is scoped to a single [Section]: a move is only valid when the * dragged item and the target item share the same non-null [section]. Items whose - * [section] is `null` (headers, banner) are fixed chrome and never move. + * [section] is `null` (headers) are fixed chrome and never move. * * Adding a future item type (drawings, recordings, …) means adding a variant * here plus its card composable — the grid, the reorder state and the span @@ -42,8 +42,6 @@ sealed interface FlowItem { data object TasksHeader : FlowItem - data object ReminderBanner : FlowItem - data class TaskRow(val task: Task) : FlowItem { override val section: Section? get() = Section.TASKS } @@ -89,7 +87,6 @@ sealed interface FlowItem { internal fun FlowItem.stableKey(taskDeleteVersions: Map = emptyMap()): Any = when (this) { FlowItem.TasksHeader -> "tasks_header" - FlowItem.ReminderBanner -> "reminder_banner" is FlowItem.TaskRow -> Pair(task.id, taskDeleteVersions[task.id] ?: 0) FlowItem.CheckListsHeader -> "checklists_header" is FlowItem.CheckListCard -> "cl_${checkList.id}" diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt index 9c978c84..229d3f31 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt @@ -23,7 +23,6 @@ sealed interface FlowUiState { val items: List = buildList { if (tasks.isNotEmpty()) { add(FlowItem.TasksHeader) - add(FlowItem.ReminderBanner) tasks.forEach { add(FlowItem.TaskRow(it)) } } if (checkLists.isNotEmpty()) { @@ -50,8 +49,8 @@ sealed interface FlowUiState { * (tasks → checkLists → ideas) matches how [FlowUiState.Success.items] is built, * so the resulting `items` mirrors the list that was passed in. * - * Chrome items (headers, banner) are dropped — they are re-emitted by `items` in - * their canonical position. As a side effect a cross-section move (which the + * Chrome items (headers) are dropped — they are re-emitted by `items` in their + * canonical position. As a side effect a cross-section move (which the * caller rejects anyway) degrades safely instead of corrupting a typed list. */ internal fun FlowUiState.Success.withItems(items: List): FlowUiState.Success = copy( diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt index d67468de..12767d49 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt @@ -10,6 +10,7 @@ import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.interaction.MutableInteractionSource import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.WindowInsets @@ -142,108 +143,109 @@ fun SectionedFlowView( val itemFadeInSpec = spring(stiffness = Spring.StiffnessMediumLow) val itemFadeOutSpec = tween(durationMillis = 130, easing = FastOutLinearInEasing) - LazyVerticalStaggeredGrid( - columns = StaggeredGridCells.Adaptive(minSize = GRID_MIN_COLUMN_DP.dp), - state = gridState, - modifier = modifier.fillMaxSize(), - // ── Keep the *main-axis* (top / bottom) content padding at 0 ───────────── - contentPadding = PaddingValues( - start = GRID_EDGE_DP.dp, - end = GRID_EDGE_DP.dp, - ), - horizontalArrangement = Arrangement.spacedBy(GRID_SPACING_DP.dp), - verticalItemSpacing = 0.dp, - ) { - itemsIndexed( - items = uiState.items, - key = { _, item -> item.stableKey(taskDeleteVersions) }, - span = { _, item -> - if (item.spanAllColumns) StaggeredGridItemSpan.FullLine - else StaggeredGridItemSpan.SingleLane - }, - ) { index, item -> - val animateItemModifier = Modifier.animateItem( - fadeInSpec = itemFadeInSpec, - placementSpec = itemPlacementSpec, - fadeOutSpec = itemFadeOutSpec - ) - - // Top gap of the whole grid, applied to its first row (always a header) - // instead of to contentPadding. See the comment on contentPadding above. - val topInsetModifier = - if (index == 0) Modifier.padding(top = GRID_TOP_DP.dp) else Modifier - val itemModifier = animateItemModifier.then(topInsetModifier) + Column(modifier = modifier.fillMaxSize()) { + // ── The banner is pinned above the grid, not an item of it ─────────────── + ReminderPermissionBanner( + visible = showPermissionBanner, + modifier = Modifier.padding(start = 20.dp, end = 20.dp), + ) - when (item) { - FlowItem.TasksHeader -> SectionHeader( - title = stringResource(R.string.tasks_m), - modifier = itemModifier - .padding(horizontal = ROW_EXTRA_INSET_DP.dp, vertical = 8.dp) - ) - FlowItem.ReminderBanner -> ReminderPermissionBanner( - visible = showPermissionBanner, - modifier = Modifier - .padding(horizontal = ROW_EXTRA_INSET_DP.dp) - .then(topInsetModifier) // Deliberately WITHOUT animateItem: the banner owns its enter/exit animation through its internal AnimatedVisibility + LazyVerticalStaggeredGrid( + columns = StaggeredGridCells.Adaptive(minSize = GRID_MIN_COLUMN_DP.dp), + state = gridState, + modifier = Modifier.weight(1f).fillMaxWidth(), + // ── Keep the *main-axis* (top / bottom) content padding at 0 ───────────── + contentPadding = PaddingValues( + start = GRID_EDGE_DP.dp, + end = GRID_EDGE_DP.dp, + ), + horizontalArrangement = Arrangement.spacedBy(GRID_SPACING_DP.dp), + verticalItemSpacing = 0.dp, + ) { + itemsIndexed( + items = uiState.items, + key = { _, item -> item.stableKey(taskDeleteVersions) }, + span = { _, item -> + if (item.spanAllColumns) StaggeredGridItemSpan.FullLine + else StaggeredGridItemSpan.SingleLane + }, + ) { index, item -> + val animateItemModifier = Modifier.animateItem( + fadeInSpec = itemFadeInSpec, + placementSpec = itemPlacementSpec, + fadeOutSpec = itemFadeOutSpec ) - FlowItem.CheckListsHeader -> SectionHeader( - title = stringResource(R.string.checklists_m), - modifier = itemModifier - .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) - ) + // Top gap of the whole grid, applied to its first row (always a header) + // instead of to contentPadding. See the comment on contentPadding above. + val topInsetModifier = + if (index == 0) Modifier.padding(top = GRID_TOP_DP.dp) else Modifier + val itemModifier = animateItemModifier.then(topInsetModifier) - FlowItem.IdeasHeader -> SectionHeader( - title = stringResource(R.string.ideas_notes_m), - modifier = itemModifier - .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) - ) + when (item) { + FlowItem.TasksHeader -> SectionHeader( + title = stringResource(R.string.tasks_m), + modifier = itemModifier + .padding(horizontal = ROW_EXTRA_INSET_DP.dp, vertical = 8.dp) + ) + FlowItem.CheckListsHeader -> SectionHeader( + title = stringResource(R.string.checklists_m), + modifier = itemModifier + .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) + ) - is FlowItem.TaskRow -> { - val taskIndex = taskPositions[item.task.id] ?: 0 - TaskRowContent( + FlowItem.IdeasHeader -> SectionHeader( + title = stringResource(R.string.ideas_notes_m), + modifier = itemModifier + .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) + ) + + is FlowItem.TaskRow -> { + val taskIndex = taskPositions[item.task.id] ?: 0 + TaskRowContent( + reorderState = reorderState, + key = item.stableKey(taskDeleteVersions), + animateItemModifier = itemModifier, + task = item.task, + shape = taskRowShape(taskIndex, uiState.tasks.size), + onDelete = { onTaskDelete(item.task) }, + onComplete = { onTaskToggle(item.task) }, + onContentClick = { onTaskClick(item.task) }, + ) + } + + is FlowItem.CheckListCard -> ReorderableCard( reorderState = reorderState, key = item.stableKey(taskDeleteVersions), animateItemModifier = itemModifier, - task = item.task, - shape = taskRowShape(taskIndex, uiState.tasks.size), - onDelete = { onTaskDelete(item.task) }, - onComplete = { onTaskToggle(item.task) }, - onContentClick = { onTaskClick(item.task) }, - ) - } + ) { + CheckListGridCard( + checkList = item.checkList, + onClick = { onCheckListClick(item.checkList) }, + modifier = Modifier.padding(bottom = CARD_VERTICAL_SPACING_DP.dp) + ) + } - is FlowItem.CheckListCard -> ReorderableCard( - reorderState = reorderState, - key = item.stableKey(taskDeleteVersions), - animateItemModifier = itemModifier, - ) { - CheckListGridCard( - checkList = item.checkList, - onClick = { onCheckListClick(item.checkList) }, - modifier = Modifier.padding(bottom = CARD_VERTICAL_SPACING_DP.dp) - ) - } + is FlowItem.IdeaCard -> ReorderableCard( + reorderState = reorderState, + key = item.stableKey(taskDeleteVersions), + animateItemModifier = itemModifier, + ) { + IdeaGridCard( + idea = item.idea, + onClick = { onIdeaClick(item.idea) }, + modifier = Modifier.padding(bottom = CARD_VERTICAL_SPACING_DP.dp) + ) + } - is FlowItem.IdeaCard -> ReorderableCard( - reorderState = reorderState, - key = item.stableKey(taskDeleteVersions), - animateItemModifier = itemModifier, - ) { - IdeaGridCard( - idea = item.idea, - onClick = { onIdeaClick(item.idea) }, - modifier = Modifier.padding(bottom = CARD_VERTICAL_SPACING_DP.dp) + // No animateItem: an empty spacer has nothing to animate, and + // keeping it out of the animator makes the bottom bound stable. + FlowItem.BottomSpacer -> Spacer( + modifier = Modifier + .fillMaxWidth() + .height(GRID_BOTTOM_DP.dp) ) } - - // No animateItem: an empty spacer has nothing to animate, and - // keeping it out of the animator makes the bottom bound stable. - FlowItem.BottomSpacer -> Spacer( - modifier = Modifier - .fillMaxWidth() - .height(GRID_BOTTOM_DP.dp) - ) } } } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/ReminderPermissionBanner.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/ReminderPermissionBanner.kt index 6c0478f2..6083d589 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/ReminderPermissionBanner.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/ReminderPermissionBanner.kt @@ -5,6 +5,7 @@ import android.net.Uri import android.os.Build import android.provider.Settings import androidx.compose.animation.AnimatedVisibility +import androidx.compose.animation.core.tween import androidx.compose.animation.expandVertically import androidx.compose.animation.fadeIn import androidx.compose.animation.fadeOut @@ -34,17 +35,27 @@ import androidx.compose.ui.unit.dp import com.markel.flowstate.feature.tasks.R import androidx.core.net.toUri +private const val BANNER_EXPAND_MS = 250 +private const val BANNER_SHRINK_MS = 300 +private const val BANNER_FADE_IN_MS = 200 +private const val BANNER_FADE_OUT_MS = 150 + /** - * Contextual banner shown inside the task list when: + * Contextual banner shown above the task list when: * 1. At least one task has a reminderTime set (there is something that should ring), AND * 2. The exact alarm permission is missing (it won't ring). * * The visibility check (hasTasksWithReminder && !canScheduleExactAlarms) is done - * by the caller (SectionedFlowView or equivalent) so this composable stays dumb. + * by the caller (SectionedFlowView) so this composable stays dumb. * - * Horizontal insets are also the caller's responsibility: the Flow grid already - * provides them (12dp contentPadding + 8dp on the row), so the banner must not - * add its own horizontal padding, or it ends up narrower than the task rows. + * ── It must stay OUT of the scrolling container ────────────────────────────── + * Animating its height inside the LazyVerticalStaggeredGrid moved the end of the + * content on every frame of the exit, and that bound is what + * LazyLayoutItemAnimator uses to invent a start offset for rows that were never + * composed: they were born glued to the row above, or popped straight into their + * final slot when the collapse happened in one frame. Pinned above the grid the + * height belongs to the viewport, not to the content, so the grid translates + * rigidly and the rows it reveals come in through the bottom edge. */ @Composable fun ReminderPermissionBanner( @@ -55,14 +66,16 @@ fun ReminderPermissionBanner( AnimatedVisibility( visible = visible, - enter = fadeIn() + expandVertically(), - exit = fadeOut() + shrinkVertically(), + enter = fadeIn(animationSpec = tween(durationMillis = BANNER_FADE_IN_MS)) + + expandVertically(animationSpec = tween(durationMillis = BANNER_EXPAND_MS)), + exit = fadeOut(animationSpec = tween(durationMillis = BANNER_FADE_OUT_MS)) + + shrinkVertically(animationSpec = tween(durationMillis = BANNER_SHRINK_MS)), modifier = modifier ) { Row( modifier = Modifier .fillMaxWidth() - .padding(top = 4.dp, bottom = 10.dp) + .padding(top = 16.dp, bottom = 4.dp) .background( color = MaterialTheme.colorScheme.errorContainer, shape = RoundedCornerShape(16.dp) diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt index 5b72784a..4c9ffe8e 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt @@ -53,7 +53,6 @@ class FlowItemTest { assertEquals( listOf( FlowItem.TasksHeader, - FlowItem.ReminderBanner, FlowItem.TaskRow(task(1)), FlowItem.TaskRow(task(2)), FlowItem.CheckListsHeader, @@ -73,7 +72,6 @@ class FlowItemTest { assertEquals( listOf( FlowItem.TasksHeader, - FlowItem.ReminderBanner, FlowItem.TaskRow(task(1)), FlowItem.BottomSpacer, ), @@ -94,16 +92,15 @@ class FlowItemTest { @Test fun rows_isRecomputedAfterCopy() { val original = state(tasks = listOf(task(1)), ideas = listOf(idea(1))) - assertEquals(6, original.items.size) + assertEquals(5, original.items.size) val withoutIdeas = original.copy(ideas = emptyList()) - assertEquals(4, withoutIdeas.items.size) + assertEquals(3, withoutIdeas.items.size) val withCheckLists = withoutIdeas.copy(checkLists = listOf(checkList(1))) assertEquals( listOf( FlowItem.TasksHeader, - FlowItem.ReminderBanner, FlowItem.TaskRow(task(1)), FlowItem.CheckListsHeader, FlowItem.CheckListCard(checkList(1)), @@ -119,7 +116,6 @@ class FlowItemTest { fun stableKey_isUniqueAcrossAllRowVariants() { val items = listOf( FlowItem.TasksHeader, - FlowItem.ReminderBanner, FlowItem.TaskRow(task(1)), FlowItem.CheckListsHeader, FlowItem.CheckListCard(checkList(1)), @@ -207,7 +203,6 @@ class FlowItemTest { FlowItem.IdeasHeader, FlowItem.IdeaCard(idea(1)), FlowItem.TasksHeader, - FlowItem.ReminderBanner, FlowItem.TaskRow(task(1)), ) @@ -223,7 +218,6 @@ class FlowItemTest { @Test fun section_isNullForChrome_andSetForCards() { assertNull(FlowItem.TasksHeader.section) - assertNull(FlowItem.ReminderBanner.section) assertNull(FlowItem.CheckListsHeader.section) assertNull(FlowItem.IdeasHeader.section) assertNull(FlowItem.BottomSpacer.section) @@ -236,7 +230,6 @@ class FlowItemTest { @Test fun fullWidth_isTrueForChromeAndTasks_falseForGridCards() { assertTrue(FlowItem.TasksHeader.spanAllColumns) - assertTrue(FlowItem.ReminderBanner.spanAllColumns) assertTrue(FlowItem.CheckListsHeader.spanAllColumns) assertTrue(FlowItem.IdeasHeader.spanAllColumns) assertTrue(FlowItem.TaskRow(task(1)).spanAllColumns) From f4ed5047d0077414adadb35ba15be725a55faec0 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 18 Oct 2026 04:18:16 +0200 Subject: [PATCH 08/11] fix: stop a dragged card from jumping when it leaves its section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symptom ------- Long-pressing a checklist / idea card and dragging it over a row of another section (the first idea while holding the last checklist card, or any task row) made the held card jump to a different spot and then follow the finger from there, staying displaced until the drop. Reordering inside a single section was fine, and the case did not exist before the grid refactor. Root cause ---------- onItemReorder vetoes cross-section moves: val section = from.section ?: return if (section != to.section) return but reorderable cannot be told "no": onMove returns Unit and the library commits to the swap on the very next lines (3.0.0, ReorderableLazyCollection.kt): :641 oldDraggingItemIndex = draggingItem.index :643 onMove(from, to) -> our veto, nothing moves :645 predictedDraggingItemOffset = ... -> where the row *would* land :316 draggingItemOffset = draggedDelta + (initialOffset - predictedOffset) so the held row is translated by the delta of a swap that never happened. That is the jump. Nothing corrects it afterwards: the library then waits, at :651, for a layout change a vetoed move cannot produce; the timeout is swallowed by the catch (CancellationException) at :660, so the resets at :657-658 never run and the phantom offset survives until onDragStop(). The move mutex stays held for that second, onDrag() short-circuits on tryLock(), and while the row keeps accumulating drag deltas no further crossing or auto-scroll is processed. Section headers are never involved because they are not ReorderableItems, so they are absent from the library's reorderableKeys. Leaving a section always means overlapping a foreign *card*, which is precisely the row the guard rejects. This was unreachable before the refactor: ReorderableCarousel built its own rememberReorderableLazyListState per section, so one carousel never saw the rows of another. A single grid for every section puts them all in the same reorderableKeys set, and that is what makes the veto reachable. Fix --- Veto before the library picks its target, not inside onMove. The only lever the library offers is ReorderableItem(enabled = …): a disabled key is dropped from reorderableKeys (ReorderableLazyCollection.kt:832-837) so it can never be chosen as a swap target, while the row stays draggable — enabled gates being *swapped with*, not being *dragged*. * FlowItem.kt: isReorderTargetFor(draggedSection) holds the rule "a row is a swap target only while the dragged row comes from its own section", so the view's gate and the ViewModel's guard cannot drift apart. * SectionedFlowView.kt: draggedSection state, published by each handle's onDragStarted / onDragStopped and consumed as enabled by every row. It is also cleared from reorderState.isAnyItemDragging, so an onDragStarted that never turned into a real drag cannot leave the other sections locked. * FlowViewModel.kt: the guard stays — it is what protects the data — but its KDoc now records that it must be unreachable, and that any new rejection needs a matching gate on the view side. Behavior note ------------- Dragging a card over a foreign row now does nothing: the card follows the finger and animates back to its slot on drop. Swapping inside Tasks, CheckLists or Ideas is untouched. A one-frame window remains: the gate lands with the recomposition that follows onDragStarted, so a first onDrag in the same frame could in principle still pick a foreign target. In practice long-press detection and the drag deltas come in different pointer events. The data is safe either way (the guard still rejects), and a next crossing realigns the row. --- .../markel/flowstate/feature/flow/FlowItem.kt | 30 +++++++++- .../flowstate/feature/flow/FlowViewModel.kt | 12 +++- .../flow/components/SectionedFlowView.kt | 43 +++++++++++++- .../flowstate/feature/flow/FlowItemTest.kt | 57 +++++++++++++++++++ 4 files changed, 138 insertions(+), 4 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt index ab5059bc..7251a491 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt @@ -15,7 +15,9 @@ import com.markel.flowstate.core.domain.Task * * Reordering is scoped to a single [Section]: a move is only valid when the * dragged item and the target item share the same non-null [section]. Items whose - * [section] is `null` (headers) are fixed chrome and never move. + * [section] is `null` (headers) are fixed chrome and never move. Enforcing that + * is *not* just a data rule — the grid has to keep a drag from ever targeting a + * foreign row, see [isReorderTargetFor]. * * Adding a future item type (drawings, recordings, …) means adding a variant * here plus its card composable — the grid, the reorder state and the span @@ -72,6 +74,32 @@ sealed interface FlowItem { data object BottomSpacer : FlowItem } +/** + * Whether a row of [this] section may be used as the swap target of the row + * currently being dragged ([draggedSection]). + * + * `reorderable` has **no way to veto a move**: `onMove` returns `Unit` and the + * library immediately assumes the list changed. Right after invoking it it + * *predicts* the dragged row's new slot (`predictedDraggingItemOffset`) and + * compensates the row's visual offset with that prediction while it waits for the + * next layout pass (up to `MoveItemsLayoutInfoUpdateMaxWaitDuration`, 1 s). If the + * move is then rejected by the ViewModel no layout pass ever disagrees with the + * prediction, so the dragged row is left translated by a delta that does not + * exist — the visible jump. + * + * The rejection therefore has to happen *before* the library chooses a target, + * which is exactly what `ReorderableItem(enabled = …)` does: disabled keys are + * removed from the library's `reorderableKeys` set and are never picked as a + * target. + * + * A `null` [section] (chrome rows) is never a target while a drag is in flight; + * chrome is not a `ReorderableItem` anyway. With no drag in flight + * ([draggedSection] is `null`) every row is a valid target again, so the grid can + * never be left locked by a drag that ended without its stop callback. + */ +internal fun FlowItem.Section?.isReorderTargetFor(draggedSection: FlowItem.Section?): Boolean = + draggedSection == null || this == draggedSection + /** * Stable key for an item, used by the grid's `itemsIndexed`. * diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt index 5670e6c6..d34d5cd4 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt @@ -357,8 +357,16 @@ class FlowViewModel @Inject constructor( * * The move is only applied when both rows belong to the same non-null * [FlowItem.Section]: headers and the reminder banner are fixed chrome, and - * a card may not jump between the CheckLists and Ideas sections. When the - * guard rejects the move the row simply snaps back to its original slot. + * a card may not jump between different Items sections. + * + * This guard is a data-safety net, **not** the mechanism that keeps the drag + * inside its section. `reorderable` gets no answer back from `onMove`, so a + * move rejected here leaves the library compensating the dragged row against + * a slot that never materializes (the row visibly jumps and then follows the + * finger from the wrong anchor). `SectionedFlowView` prevents that by + * disabling foreign rows as swap targets for the duration of the drag; a + * rejected call here should therefore be unreachable, and any future + * rejection needs the same treatment on the view side. * * Note this is called on every item crossing during a drag, so it writes to * the database on each one — same as the carousel did. Persisting a single diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt index 12767d49..3364f27a 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt @@ -33,7 +33,9 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.graphicsLayer @@ -48,6 +50,7 @@ import com.markel.flowstate.core.domain.Idea import com.markel.flowstate.core.domain.Task import com.markel.flowstate.feature.flow.FlowItem import com.markel.flowstate.feature.flow.FlowUiState +import com.markel.flowstate.feature.flow.isReorderTargetFor import com.markel.flowstate.feature.flow.stableKey import com.markel.flowstate.feature.flow.tasks.components.AnimatableTaskItem import com.markel.flowstate.feature.flow.tasks.components.EmptyStateView @@ -123,6 +126,16 @@ fun SectionedFlowView( scrollThresholdPadding = WindowInsets.systemBars.asPaddingValues(), ) { from, to -> onItemReorder(from.index, to.index) } + // Section the current drag started in; null while nothing is being dragged. + // It gates `ReorderableItem.enabled` so the library can never request a + // cross-section swap, which it cannot undo. See [isReorderTargetFor]. + var draggedSection by remember { mutableStateOf(null) } + + val isAnyItemDragging = reorderState.isAnyItemDragging + LaunchedEffect(isAnyItemDragging) { + if (!isAnyItemDragging) draggedSection = null + } + LaunchedEffect(gridState) { snapshotFlow { gridState.firstVisibleItemIndex > 0 || gridState.firstVisibleItemScrollOffset > 90 @@ -206,6 +219,9 @@ fun SectionedFlowView( reorderState = reorderState, key = item.stableKey(taskDeleteVersions), animateItemModifier = itemModifier, + section = FlowItem.Section.TASKS, + draggedSection = draggedSection, + onDraggedSectionChanged = { draggedSection = it }, task = item.task, shape = taskRowShape(taskIndex, uiState.tasks.size), onDelete = { onTaskDelete(item.task) }, @@ -218,6 +234,9 @@ fun SectionedFlowView( reorderState = reorderState, key = item.stableKey(taskDeleteVersions), animateItemModifier = itemModifier, + section = FlowItem.Section.CHECKLISTS, + draggedSection = draggedSection, + onDraggedSectionChanged = { draggedSection = it }, ) { CheckListGridCard( checkList = item.checkList, @@ -230,6 +249,9 @@ fun SectionedFlowView( reorderState = reorderState, key = item.stableKey(taskDeleteVersions), animateItemModifier = itemModifier, + section = FlowItem.Section.IDEAS, + draggedSection = draggedSection, + onDraggedSectionChanged = { draggedSection = it }, ) { IdeaGridCard( idea = item.idea, @@ -268,17 +290,26 @@ private fun taskRowShape(index: Int, size: Int) = when { /** * Shared shell for every draggable row: long-press anywhere on the item starts * the drag, and the item lifts, scales and fades slightly while it is held. + * + * [section] / [draggedSection] keep the drag inside its own section: a row is + * only handed to the library as a swap target while the dragged row comes from + * the same section. [onDraggedSectionChanged] publishes the section on drag start + * and clears it on drag stop. */ @Composable private fun LazyStaggeredGridItemScope.ReorderableCard( reorderState: ReorderableLazyStaggeredGridState, key: Any, animateItemModifier: Modifier, + section: FlowItem.Section, + draggedSection: FlowItem.Section?, + onDraggedSectionChanged: (FlowItem.Section?) -> Unit, content: @Composable ReorderableCollectionItemScope.(isDragging: Boolean) -> Unit, ) { ReorderableItem( state = reorderState, key = key, + enabled = section.isReorderTargetFor(draggedSection), animateItemModifier = animateItemModifier, ) { isDragging -> val collectionScope = this @@ -288,7 +319,11 @@ private fun LazyStaggeredGridItemScope.ReorderableCard( ) Box( modifier = Modifier - .longPressDraggableHandle(interactionSource = remember { MutableInteractionSource() }) + .longPressDraggableHandle( + interactionSource = remember { MutableInteractionSource() }, + onDragStarted = { onDraggedSectionChanged(section) }, + onDragStopped = { onDraggedSectionChanged(null) }, + ) .graphicsLayer { scaleX = scale scaleY = scale @@ -306,6 +341,9 @@ private fun LazyStaggeredGridItemScope.TaskRowContent( reorderState: ReorderableLazyStaggeredGridState, key: Any, animateItemModifier: Modifier, + section: FlowItem.Section, + draggedSection: FlowItem.Section?, + onDraggedSectionChanged: (FlowItem.Section?) -> Unit, task: Task, shape: RoundedCornerShape, onDelete: () -> Unit, @@ -316,6 +354,9 @@ private fun LazyStaggeredGridItemScope.TaskRowContent( reorderState = reorderState, key = key, animateItemModifier = animateItemModifier, + section = section, + draggedSection = draggedSection, + onDraggedSectionChanged = onDraggedSectionChanged, ) { Box( modifier = Modifier diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt index 4c9ffe8e..1feee23a 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt @@ -238,4 +238,61 @@ class FlowItemTest { assertFalse(FlowItem.CheckListCard(checkList(1)).spanAllColumns) assertFalse(FlowItem.IdeaCard(idea(1)).spanAllColumns) } + + // ── isReorderTargetFor: the rule that keeps a drag inside its section ────── + + @Test + fun isReorderTargetFor_withNoDragInFlight_acceptsEverySection() { + FlowItem.Section.entries.forEach { section -> + assertTrue("$section", section.isReorderTargetFor(null)) + } + } + + @Test + fun isReorderTargetFor_whileDragging_onlyAcceptsTheSameSection() { + FlowItem.Section.entries.forEach { dragged -> + FlowItem.Section.entries.forEach { candidate -> + assertEquals( + "$candidate while dragging $dragged", + dragged == candidate, + candidate.isReorderTargetFor(dragged) + ) + } + } + } + + /** Chrome rows are never a swap target, so a drag can never be "into" a header. */ + @Test + fun isReorderTargetFor_chromeRows_areNeverTargetsWhileDragging() { + assertFalse( + FlowItem.IdeasHeader.section.isReorderTargetFor(FlowItem.Section.IDEAS) + ) + assertFalse( + FlowItem.BottomSpacer.section.isReorderTargetFor(FlowItem.Section.CHECKLISTS) + ) + } + + /** + * The view's target gating and the ViewModel's guard must agree, otherwise a + * swap the library considers legal is silently dropped by `onItemReorder`. + */ + @Test + fun isReorderTargetFor_agreesWithTheViewModelGuard() { + val items = state( + tasks = listOf(task(1)), + checkLists = listOf(checkList(1)), + ideas = listOf(idea(1)), + ).items + + FlowItem.Section.entries.forEach { dragged -> + items.forEach { item -> + val guardAllows = item.section != null && item.section == dragged + assertEquals( + "$item while dragging $dragged", + guardAllows, + item.section.isReorderTargetFor(dragged) + ) + } + } + } } From f2af718784a0e48e7778b076fb4ccf82e9159fa4 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 18 Oct 2026 20:27:01 +0200 Subject: [PATCH 09/11] test: cover the deleted-task flicker and the released delete ids --- .../feature/flow/FlowViewModelTest.kt | 89 ++++++++++++++++++- 1 file changed, 87 insertions(+), 2 deletions(-) diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt index 3a1aa2e4..fc6f7e69 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt @@ -21,6 +21,8 @@ import io.mockk.mockk import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.advanceTimeBy @@ -62,9 +64,14 @@ class FlowViewModelTest { checkLists: List = emptyList(), categories: List = emptyList(), categoriesEnabled: Boolean = false, - applicationScope: CoroutineScope = defaultTestApplicationScope + applicationScope: CoroutineScope = defaultTestApplicationScope, + /** + * Override when a test needs to push new DB snapshots (defaults to a + * static [flowOf], which never re-emits). + */ + tasksFlow: Flow> = flowOf(tasks) ): FlowViewModel { - coEvery { taskRepository.getTasks() } returns flowOf(tasks) + coEvery { taskRepository.getTasks() } returns tasksFlow coEvery { ideaRepository.getIdeas() } returns flowOf(ideas) coEvery { checkListRepository.getLists() } returns flowOf(checkLists) coEvery { categoryRepository.getCategories() } returns flowOf(categories) @@ -274,6 +281,84 @@ class FlowViewModelTest { } } + // ── Deletion visibility (regression) ───────────────────────────────────── + // + // `onTaskSwiped` hides the task optimistically, but the DB confirms the + // deletion asynchronously: Room commits the write when the suspend function + // returns and re-emits the query afterward. In between, the task is still + // in the DB snapshot, so the filter has to keep hiding it — otherwise it + // flashes back into the list for a frame. + // The static `flowOf` used by `createViewModel` never re-emits, so it holds + // that window open indefinitely, which is exactly what these tests need. + + /** Tasks currently visible in the UI state, i.e. what the grid renders. */ + private fun visibleTasks(): List = + (viewModel.uiState.value as FlowUiState.Success).tasks + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun onTaskSwiped_keepsTaskHidden_afterTheUndoWindowExpiresBeforeTheDbConfirms() = runTest { + val testApplicationScope = CoroutineScope(SupervisorJob() + UnconfinedTestDispatcher(testScheduler)) + val task = Task(id = 1, title = "T", isDone = false, position = 0) + viewModel = createViewModel(tasks = listOf(task), applicationScope = testApplicationScope) + + viewModel.onTaskSwiped(task) + assertTrue(visibleTasks().isEmpty()) + + // The countdown expires and the id leaves the undoable set, but the DB + // keeps reporting the task: it must not come back. + advanceTimeBy(4_000L) + assertTrue(visibleTasks().isEmpty()) + coVerify { deleteTaskUseCase(task) } + + // AND the undo FAB switches itself off + viewModel.showUndoButton.test { assertFalse(awaitItem()) } + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun committedDeleteId_isReleasedOnceTheDbStopsReportingTheTask() = runTest { + val testApplicationScope = CoroutineScope(SupervisorJob() + UnconfinedTestDispatcher(testScheduler)) + val task = Task(id = 7, title = "T7", isDone = false, position = 0) + val dbTasks = MutableStateFlow(listOf(task)) + viewModel = createViewModel(tasksFlow = dbTasks, applicationScope = testApplicationScope) + + viewModel.onTaskSwiped(task) + advanceTimeBy(4_000L) + // Still hidden while the DB keeps reporting it — same window as above. + assertTrue(visibleTasks().isEmpty()) + + dbTasks.value = emptyList() // the DB confirms the deletion + assertTrue(visibleTasks().isEmpty()) + + // A task re-created with the same id must be visible again: if the id + // stayed in the committed set it would be hidden forever. + dbTasks.value = listOf(task) + assertEquals(listOf("T7"), visibleTasks().map { it.title }) + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun onTaskSwiped_keepsEveryTaskHidden_whenSeveralDeletionsCommitTogether() = runTest { + val testApplicationScope = CoroutineScope(SupervisorJob() + UnconfinedTestDispatcher(testScheduler)) + val t1 = Task(id = 1, title = "T1", isDone = false, position = 0) + val t2 = Task(id = 2, title = "T2", isDone = false, position = 1) + val dbTasks = MutableStateFlow(listOf(t1, t2)) + viewModel = createViewModel(tasksFlow = dbTasks, applicationScope = testApplicationScope) + + viewModel.onTaskSwiped(t1) + advanceTimeBy(1_000L) + viewModel.onTaskSwiped(t2) + advanceTimeBy(3_000L) // t = 4s: t1 committed, t2 still undoable + assertTrue(visibleTasks().isEmpty()) + advanceTimeBy(1_000L) // t = 5s: t2 committed too + assertTrue(visibleTasks().isEmpty()) + + // The DB confirms both at once: neither may resurrect. + dbTasks.value = emptyList() + assertTrue(visibleTasks().isEmpty()) + } + // ── Reorder logic ───────────────────────────────────────────────────────── // // onItemReorder() takes indices into the FLAT item list (headers included), From 8ab3d4fb22395194c61840b82d472c14784ca89d Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 18 Oct 2026 20:52:51 +0200 Subject: [PATCH 10/11] refactor: persist the reorder once, when the drag ends This has a trade-off, but I think is logical, if the app process ends mid-drag, i.e. by closing the app, the new order won't be persisted, but I think that is better than persisting the last saved drag, which may not be the goal position. --- .../flowstate/feature/flow/FlowScreen.kt | 1 + .../flowstate/feature/flow/FlowViewModel.kt | 50 ++++++++++++++++--- .../flow/components/SectionedFlowView.kt | 6 ++- .../feature/flow/FlowViewModelTest.kt | 33 ++++++++++++ 4 files changed, 82 insertions(+), 8 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowScreen.kt index 38dd63cd..dc48bb5a 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowScreen.kt @@ -117,6 +117,7 @@ fun FlowScreen( uiState = flowUiState, onScrolled = { isHeaderMinimized = true }, onItemReorder = { from, to -> flowViewModel.onItemReorder(from, to) }, + onReorderCommit = flowViewModel::commitReorder, onTaskClick = { onNavigateToTaskEditor(it.id) }, onTaskDelete = { task -> flowViewModel.onTaskSwiped(task) }, onTaskToggle = { taskViewModel.toggleTaskDone(it) }, diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt index d34d5cd4..126f2bab 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt @@ -352,6 +352,15 @@ class FlowViewModel @Inject constructor( // ── Reorder ──────────────────────────────────────────────────────── + /** + * Sections whose order changed since the last [commitReorder]. + * + * A drag is a burst of [onItemReorder] calls, one per slot crossed, and each + * write renumbers and persists the *whole* section. Holding the write back + * until the finger lifts turns that burst into a single write. + */ + private val pendingReorderSections = mutableSetOf() + /** * Applies a drag between two rows of the flat [FlowItem] list. * @@ -368,9 +377,9 @@ class FlowViewModel @Inject constructor( * rejected call here should therefore be unreachable, and any future * rejection needs the same treatment on the view side. * - * Note this is called on every item crossing during a drag, so it writes to - * the database on each one — same as the carousel did. Persisting a single - * time on drop is a follow-up. + * Note this is called on every item crossing during a drag, so it must stay + * cheap: it only reorders rows in memory. The database write is deferred to + * [commitReorder], which the grid calls once when the drag ends. */ fun onItemReorder(fromIndex: Int, toIndex: Int) { val state = _uiState.value as? FlowUiState.Success ?: return @@ -405,11 +414,38 @@ class FlowViewModel @Inject constructor( _uiState.value = renumbered + // The database is written once, when the drag ends: see [commitReorder]. + pendingReorderSections += section + } + + /** + * Persists every section whose order changed since the last commit. + * + * Called by the grid when the drag gesture ends. While the finger is down + * [onItemReorder] only moves rows in the UI state, so a drag across N slots + * costs **one** write of the section instead of N. + * + * Safe to call unconditionally — it returns immediately when nothing moved, + * which is the case on the effect's very first run (before any drag). + * + * The order is lost if the process dies mid-drag; that is deliberate, since + * a partially dragged list is not worth a write per crossing. + */ + fun commitReorder() { + if (pendingReorderSections.isEmpty()) return + val state = _uiState.value as? FlowUiState.Success ?: return + + val sections = pendingReorderSections.toSet() + pendingReorderSections.clear() + + // `renumbered` already fixed the positions, so the lists can go as they are. viewModelScope.launch { - when (section) { - FlowItem.Section.TASKS -> taskRepository.updateTasksOrder(renumbered.tasks) - FlowItem.Section.CHECKLISTS -> checkListRepository.updateCheckListsOrder(renumbered.checkLists) - FlowItem.Section.IDEAS -> ideaRepository.updateIdeasOrder(renumbered.ideas) + sections.forEach { section -> + when (section) { + FlowItem.Section.TASKS -> taskRepository.updateTasksOrder(state.tasks) + FlowItem.Section.CHECKLISTS -> checkListRepository.updateCheckListsOrder(state.checkLists) + FlowItem.Section.IDEAS -> ideaRepository.updateIdeasOrder(state.ideas) + } } } } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt index 3364f27a..74006cdf 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt @@ -98,6 +98,7 @@ fun SectionedFlowView( uiState: FlowUiState, onScrolled: () -> Unit, onItemReorder: (fromIndex: Int, toIndex: Int) -> Unit, + onReorderCommit: () -> Unit, // Task callbacks onTaskClick: (Task) -> Unit, onTaskDelete: (Task) -> Unit, @@ -133,7 +134,10 @@ fun SectionedFlowView( val isAnyItemDragging = reorderState.isAnyItemDragging LaunchedEffect(isAnyItemDragging) { - if (!isAnyItemDragging) draggedSection = null + if (!isAnyItemDragging) { + draggedSection = null + onReorderCommit() // let the ViewModel persist the order in one go. + } } LaunchedEffect(gridState) { diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt index fc6f7e69..26036a70 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowViewModelTest.kt @@ -389,6 +389,8 @@ class FlowViewModelTest { assertEquals(listOf(0, 1, 2), after.tasks.map { it.position }) } + // The write is deferred until the drag ends + viewModel.commitReorder() coVerify { taskRepository.updateTasksOrder(match { list -> list.map { it.id } == listOf(2, 3, 1) }) } @@ -416,6 +418,7 @@ class FlowViewModelTest { assertEquals(listOf(0, 1), after.ideas.map { it.position }) } + viewModel.commitReorder() coVerify { ideaRepository.updateIdeasOrder(match { list -> list.map { it.id } == listOf(2, 1) }) } @@ -443,11 +446,41 @@ class FlowViewModelTest { assertEquals(listOf(0, 1), after.checkLists.map { it.position }) } + viewModel.commitReorder() coVerify { checkListRepository.updateCheckListsOrder(match { list -> list.map { it.id } == listOf(2, 1) }) } } + @Test + fun onItemReorder_persistsOnce_whenADragCrossesSeveralSlots() = runTest { + // GIVEN – five tasks, so a drag from one end to the other crosses 4 slots + val tasks = (1..5).map { Task(id = it, title = "T$it", isDone = false, position = it - 1) } + viewModel = createViewModel(tasks = tasks) + + val before = (viewModel.uiState.value as FlowUiState.Success) + val firstTask = before.items.indexOfFirst { it is FlowItem.TaskRow } + val lastTask = before.items.indexOfLast { it is FlowItem.TaskRow } + + // WHEN – drag T1 down to the end, one slot at a time (what the library does) + for (to in firstTask + 1..lastTask) { + viewModel.onItemReorder(fromIndex = to - 1, toIndex = to) + } + + // THEN – the order moved but nothing was written yet + assertEquals( + listOf("T2", "T3", "T4", "T5", "T1"), + (viewModel.uiState.value as FlowUiState.Success).tasks.map { it.title } + ) + coVerify(exactly = 0) { taskRepository.updateTasksOrder(any()) } + + // AND – lifting the finger writes the final order exactly once + viewModel.commitReorder() + coVerify(exactly = 1) { + taskRepository.updateTasksOrder(match { list -> list.map { it.id } == listOf(2, 3, 4, 5, 1) }) + } + } + @Test fun onItemReorder_acrossSections_isRejected() = runTest { // GIVEN - both sections populated From 17a8c9569cf585945256f1c0de188f50262278e1 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Mon, 19 Oct 2026 03:00:32 +0200 Subject: [PATCH 11/11] fix: header misaligned Also reordered the layout, showing ideas first and then checklists. Bumped version name to 3.5.3 --- app/build.gradle.kts | 2 +- .../flowstate/feature/flow/FlowUiState.kt | 10 ++++---- .../flow/components/SectionedFlowView.kt | 24 ++++++++++++------- .../flowstate/feature/flow/FlowItemTest.kt | 10 ++++---- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index d703568c..0a405155 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -16,7 +16,7 @@ android { minSdk = 31 // Android 12 targetSdk = 36 versionCode = 3_005_002 - versionName = "3.5.2" + versionName = "3.5.3" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt index 229d3f31..aa5114ab 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt @@ -25,14 +25,14 @@ sealed interface FlowUiState { add(FlowItem.TasksHeader) tasks.forEach { add(FlowItem.TaskRow(it)) } } - if (checkLists.isNotEmpty()) { - add(FlowItem.CheckListsHeader) - checkLists.forEach { add(FlowItem.CheckListCard(it)) } - } if (ideas.isNotEmpty()) { add(FlowItem.IdeasHeader) ideas.forEach { add(FlowItem.IdeaCard(it)) } } + if (checkLists.isNotEmpty()) { + add(FlowItem.CheckListsHeader) + checkLists.forEach { add(FlowItem.CheckListCard(it)) } + } // Only when there is something to scroll: an empty screen shows // EmptyStateView and must keep `items.isEmpty()` true. if (isNotEmpty()) add(FlowItem.BottomSpacer) @@ -46,7 +46,7 @@ sealed interface FlowUiState { * Used by the reorder callback: a drag hands back a shuffled `List`, * and this projects it back onto the three typed lists. `filterIsInstance` * preserves the relative order of each section and the assignment order - * (tasks → checkLists → ideas) matches how [FlowUiState.Success.items] is built, + * (tasks → ideas → checklists) matches how [FlowUiState.Success.items] is built, * so the resulting `items` mirrors the list that was passed in. * * Chrome items (headers) are dropped — they are re-emitted by `items` in their diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt index 74006cdf..8ad81a13 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/components/SectionedFlowView.kt @@ -67,8 +67,12 @@ private const val GRID_SPACING_DP = 10 private const val GRID_EDGE_DP = 12 /** Top breathing room, applied to the first row instead of the container (see below). */ private const val GRID_TOP_DP = 8 -/** Extra inset for full-width rows so tasks keep today's 20dp margins (12 + 8). */ +/** Extra inset for headers*/ private const val ROW_EXTRA_INSET_DP = 8 +/** Gap above a section header when another section precedes it. */ +private const val SECTION_GAP_DP = 24 +/** Gap between a section header and the first row or card it introduces. */ +private const val SECTION_HEADER_BOTTOM_DP = 12 /** Bottom room for the FAB, rendered as [FlowItem.BottomSpacer] (see below). */ private const val GRID_BOTTOM_DP = 96 /** @@ -85,8 +89,8 @@ private const val CARD_VERTICAL_SPACING_DP = 9 * * Layout: * - Tasks → full-width rows using [AnimatableTaskItem] (swipe-to-dismiss) - * - CheckLists → cards, one per column, height driven by their content * - Ideas → cards, one per column, height driven by their content + * - CheckLists → cards, one per column, height driven by their content * * Everything (headers included) is emitted from one [FlowItem] list, which is * what lets the index reported by `reorderable` map 1:1 onto `items[index]`. @@ -199,22 +203,26 @@ fun SectionedFlowView( if (index == 0) Modifier.padding(top = GRID_TOP_DP.dp) else Modifier val itemModifier = animateItemModifier.then(topInsetModifier) + // Room above a header that may have another section above it. + val sectionTopPadding = if (index == 0) 8.dp else SECTION_GAP_DP.dp + when (item) { FlowItem.TasksHeader -> SectionHeader( title = stringResource(R.string.tasks_m), modifier = itemModifier .padding(horizontal = ROW_EXTRA_INSET_DP.dp, vertical = 8.dp) ) - FlowItem.CheckListsHeader -> SectionHeader( - title = stringResource(R.string.checklists_m), - modifier = itemModifier - .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) - ) FlowItem.IdeasHeader -> SectionHeader( title = stringResource(R.string.ideas_notes_m), modifier = itemModifier - .padding(start = 20.dp, end = 20.dp, top = 24.dp, bottom = 12.dp) + .padding(start = ROW_EXTRA_INSET_DP.dp, end= ROW_EXTRA_INSET_DP.dp, top = sectionTopPadding, bottom = SECTION_HEADER_BOTTOM_DP.dp) + ) + + FlowItem.CheckListsHeader -> SectionHeader( + title = stringResource(R.string.checklists_m), + modifier = itemModifier + .padding(start = ROW_EXTRA_INSET_DP.dp, end= ROW_EXTRA_INSET_DP.dp, top = sectionTopPadding, bottom = SECTION_HEADER_BOTTOM_DP.dp) ) is FlowItem.TaskRow -> { diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt index 1feee23a..fb75874d 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt @@ -40,7 +40,7 @@ class FlowItemTest { checkLists: List = emptyList(), ) = FlowUiState.Success(tasks = tasks, ideas = ideas, checkLists = checkLists) - // ── items: construcción ──────────────────────────────────────────────────── + // ── items: construction ──────────────────────────────────────────────────── @Test fun rows_withAllSections_emitsHeadersThenItems_inOrder() { @@ -55,10 +55,10 @@ class FlowItemTest { FlowItem.TasksHeader, FlowItem.TaskRow(task(1)), FlowItem.TaskRow(task(2)), - FlowItem.CheckListsHeader, - FlowItem.CheckListCard(checkList(1)), FlowItem.IdeasHeader, FlowItem.IdeaCard(idea(1)), + FlowItem.CheckListsHeader, + FlowItem.CheckListCard(checkList(1)), FlowItem.BottomSpacer, ), items @@ -146,7 +146,7 @@ class FlowItemTest { assertEquals(Pair(7, 0), item.stableKey()) assertEquals(Pair(7, 3), item.stableKey(mapOf(7 to 3))) - // Ids distintos siguen siendo distintos con el mismo contador + // Different Ids keep being different with the same counter assertNotEquals( FlowItem.TaskRow(task(7)).stableKey(mapOf(7 to 1)), FlowItem.TaskRow(task(8)).stableKey(mapOf(7 to 1)) @@ -191,7 +191,7 @@ class FlowItemTest { val rebuilt = original.withItems(items) assertEquals(listOf(idea(3), idea(1), idea(2)), rebuilt.ideas) - // Las secciones que no se han tocado se conservan + // Sections remain untouched assertEquals(original.tasks, rebuilt.tasks) assertEquals(original.checkLists, rebuilt.checkLists) }