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/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/FlowItem.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt new file mode 100644 index 00000000..7251a491 --- /dev/null +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowItem.kt @@ -0,0 +1,124 @@ +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 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) 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 + * 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 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 + } + + // ── Chrome ──────────────────────────────────────────────────────────────── + + /** + * Empty room after the last section so the FAB never covers content. + */ + 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`. + * + * 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" + 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}" + FlowItem.BottomSpacer -> "bottom_spacer" + } 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..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 @@ -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,25 @@ 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) }, + onReorderCommit = flowViewModel::commitReorder, 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/FlowUiState.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowUiState.kt index 95d84eee..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 @@ -15,5 +15,46 @@ 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) + tasks.forEach { add(FlowItem.TaskRow(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) + } + } +} + +/** + * 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 → 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 + * 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/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/FlowViewModel.kt index 9dafc759..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 @@ -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 @@ -324,50 +352,101 @@ 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) + /** + * 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() - viewModelScope.launch { - taskRepository.updateTasksOrder(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 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 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 + 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 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 = renumbered - _uiState.value = currentState.copy(ideas = updated) - - viewModelScope.launch { - ideaRepository.updateIdeasOrder(updated) - } + // The database is written once, when the drag ends: see [commitReorder]. + pendingReorderSections += section } - 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) } + /** + * 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 - _uiState.value = currentState.copy(checkLists = updated) + val sections = pendingReorderSections.toSet() + pendingReorderSections.clear() + // `renumbered` already fixed the positions, so the lists can go as they are. viewModelScope.launch { - checkListRepository.updateCheckListsOrder(updated) + 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/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..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 @@ -6,19 +6,38 @@ 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.Column +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 +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.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.RectangleShape import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight @@ -29,81 +48,114 @@ 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.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 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 +/** Top breathing room, applied to the first row instead of the container (see below). */ +private const val GRID_TOP_DP = 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 /** - * Main sectioned view for the Flow screen. + * 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. * * 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) + * - Ideas → cards, one per column, height driven by their content + * - CheckLists → 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, + onReorderCommit: () -> 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 + val reorderState = rememberReorderableLazyStaggeredGridState( + gridState, + scrollThresholdPadding = WindowInsets.systemBars.asPaddingValues(), + ) { from, to -> onItemReorder(from.index, to.index) } - if (fromTaskIndex in uiState.tasks.indices && toTaskIndex in uiState.tasks.indices) { - onTaskReorder(fromTaskIndex, toTaskIndex) + // 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 + onReorderCommit() // let the ViewModel persist the order in one go. } } - 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 +164,228 @@ fun SectionedFlowView( val itemFadeInSpec = spring(stiffness = Spring.StiffnessMediumLow) val itemFadeOutSpec = tween(durationMillis = 130, easing = FastOutLinearInEasing) - LazyColumn( - state = outerListState, - modifier = modifier.fillMaxSize(), - contentPadding = PaddingValues(bottom = 40.dp) - ) { - // ── Tasks ────────────────────────────────────────────────────────── - if (uiState.tasks.isNotEmpty()) { - item(key = "tasks_header") { - SectionHeader( - title = stringResource(R.string.tasks_m), - modifier = Modifier - .animateItem( - fadeInSpec = itemFadeInSpec, - placementSpec = itemPlacementSpec, - fadeOutSpec = itemFadeOutSpec - ) - .padding(horizontal = 20.dp, vertical = 8.dp) - ) - } - item { - ReminderPermissionBanner(showPermissionBanner) - } + 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), + ) + + 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.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 + 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) + + // 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) ) - ) { isDragging -> - val scale by animateFloatAsState( - targetValue = if (isDragging) 1.03f else 1f, - label = "task_drag_scale" + + FlowItem.IdeasHeader -> SectionHeader( + title = stringResource(R.string.ideas_notes_m), + modifier = itemModifier + .padding(start = ROW_EXTRA_INSET_DP.dp, end= ROW_EXTRA_INSET_DP.dp, top = sectionTopPadding, bottom = SECTION_HEADER_BOTTOM_DP.dp) ) - 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) } + + 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 -> { + val taskIndex = taskPositions[item.task.id] ?: 0 + TaskRowContent( + 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) }, + onComplete = { onTaskToggle(item.task) }, + onContentClick = { onTaskClick(item.task) }, ) } - } - } - } - // ── CheckLists ───────────────────────────────────────────────────── - if (uiState.checkLists.isNotEmpty()) { - item(key = "checklists_header") { - SectionHeader( - title = stringResource(R.string.checklists_m), - modifier = Modifier - .animateItem( - fadeInSpec = itemFadeInSpec, - placementSpec = itemPlacementSpec, - fadeOutSpec = itemFadeOutSpec - ) - .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 -> + is FlowItem.CheckListCard -> ReorderableCard( + reorderState = reorderState, + key = item.stableKey(taskDeleteVersions), + animateItemModifier = itemModifier, + section = FlowItem.Section.CHECKLISTS, + draggedSection = draggedSection, + onDraggedSectionChanged = { draggedSection = it }, + ) { CheckListGridCard( - checkList = checkList, - onClick = { onCheckListClick(checkList) }, - modifier = Modifier.width(160.dp) + checkList = item.checkList, + onClick = { onCheckListClick(item.checkList) }, + modifier = Modifier.padding(bottom = CARD_VERTICAL_SPACING_DP.dp) ) } - } - } - } - // ── Ideas ────────────────────────────────────────────────────────── - if (uiState.ideas.isNotEmpty()) { - item(key = "ideas_header") { - SectionHeader( - title = stringResource(R.string.ideas_notes_m), - modifier = Modifier - .animateItem( - fadeInSpec = itemFadeInSpec, - placementSpec = itemPlacementSpec, - fadeOutSpec = itemFadeOutSpec - ) - .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 + is FlowItem.IdeaCard -> ReorderableCard( + reorderState = reorderState, + key = item.stableKey(taskDeleteVersions), + animateItemModifier = itemModifier, + section = FlowItem.Section.IDEAS, + draggedSection = draggedSection, + onDraggedSectionChanged = { draggedSection = it }, + ) { IdeaGridCard( - idea = idea, - onClick = { onIdeaClick(idea) }, - modifier = Modifier - .width(200.dp) - .height(160.dp) + 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) + ) } } } } } +/** 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. + * + * [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 + val scale by animateFloatAsState( + targetValue = if (isDragging) 1.04f else 1f, + label = "grid_drag_scale" + ) + Box( + modifier = Modifier + .longPressDraggableHandle( + interactionSource = remember { MutableInteractionSource() }, + onDragStarted = { onDraggedSectionChanged(section) }, + onDragStopped = { onDraggedSectionChanged(null) }, + ) + .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, + section: FlowItem.Section, + draggedSection: FlowItem.Section?, + onDraggedSectionChanged: (FlowItem.Section?) -> Unit, + task: Task, + shape: RoundedCornerShape, + onDelete: () -> Unit, + onComplete: () -> Unit, + onContentClick: () -> Unit, +) { + ReorderableCard( + reorderState = reorderState, + key = key, + animateItemModifier = animateItemModifier, + section = section, + draggedSection = draggedSection, + onDraggedSectionChanged = onDraggedSectionChanged, + ) { + 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/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..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,13 +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. + * + * ── 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( @@ -51,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(start = 20.dp, end = 20.dp, 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 new file mode 100644 index 00000000..fb75874d --- /dev/null +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/FlowItemTest.kt @@ -0,0 +1,298 @@ +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: construction ──────────────────────────────────────────────────── + + @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.TaskRow(task(1)), + FlowItem.TaskRow(task(2)), + FlowItem.IdeasHeader, + FlowItem.IdeaCard(idea(1)), + FlowItem.CheckListsHeader, + FlowItem.CheckListCard(checkList(1)), + FlowItem.BottomSpacer, + ), + items + ) + } + + @Test + fun rows_withEmptySections_omitsTheirHeaders() { + assertEquals(emptyList(), state().items) + + assertEquals( + listOf( + FlowItem.TasksHeader, + FlowItem.TaskRow(task(1)), + FlowItem.BottomSpacer, + ), + state(tasks = listOf(task(1))).items + ) + + assertEquals( + listOf(FlowItem.IdeasHeader, FlowItem.IdeaCard(idea(1)), FlowItem.BottomSpacer), + 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.TaskRow(task(1)), + FlowItem.CheckListsHeader, + FlowItem.CheckListCard(checkList(1)), + FlowItem.BottomSpacer, + ), + withCheckLists.items + ) + } + + // ── stableKey ───────────────────────────────────────────────────────────── + + @Test + fun stableKey_isUniqueAcrossAllRowVariants() { + val items = listOf( + FlowItem.TasksHeader, + FlowItem.TaskRow(task(1)), + FlowItem.CheckListsHeader, + FlowItem.CheckListCard(checkList(1)), + FlowItem.IdeasHeader, + FlowItem.IdeaCard(idea(1)), + FlowItem.BottomSpacer, + ) + + 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))) + // 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)) + ) + } + + @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) + // Sections remain untouched + 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.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.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) + assertEquals(FlowItem.Section.IDEAS, FlowItem.IdeaCard(idea(1)).section) + } + + @Test + fun fullWidth_isTrueForChromeAndTasks_falseForGridCards() { + assertTrue(FlowItem.TasksHeader.spanAllColumns) + 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) + } + + // ── 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) + ) + } + } + } +} 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..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 @@ -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 @@ -20,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 @@ -61,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) @@ -273,10 +281,92 @@ class FlowViewModelTest { } } - // ── Reorder Logic ───────────────────────────────────────────────────────── + // ── 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 onTaskReorder_updatesStateOptimistically_andCallsRepository() = runTest { + 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), + // 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 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 +375,29 @@ 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 + // The write is deferred until the drag ends + viewModel.commitReorder() 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 +405,27 @@ 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 }) } + viewModel.commitReorder() 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 +433,129 @@ 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 }) } + viewModel.commitReorder() 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_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 + 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 +988,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 } +}