diff --git a/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/done.md b/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/done.md new file mode 100644 index 00000000..87a16488 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/done.md @@ -0,0 +1,36 @@ +# Done + +- **Branch / head SHA**: `alanvardy-var-1102-drag-to-move` @ `2c3f41c` + (`fix: clear stale drag state on cancel and reject foreign drops`) +- **Mechanical checks**: `bash scripts/test.sh` → `gate: ok` both before and + after the review fixes. `make test-unit` 485 → 487 tests passing (2 added). + Covers `make build` (iOS sim, warnings-as-errors), `make test`, + `make build-mac`, `make watch-build`, `scripts/tests/run.sh` (26 passed), + `shellcheck`. No warnings flagged; no blockers from mechanical checks. +- **Rebase**: no conflicts — no rebase was in progress and the branch was + already based on `main`. +- **Review outcome**: + - One bounded `reviewer` pass over the 3-file source diff (the `DELETEME` + placeholder removal and step-artifacts commits are chores). Fresh context. + - **Blocker fixed (#1)**: a cancelled drag never reached `performDrop`, so + `draggingChecklistID`/`draggingFolderID` leaked and could drive a move when + a later drag of the other kind passed over rows (silent persisted-order + corruption). Fix: each `onDrag` clears the other kind's id, edit-mode exit + clears both, and `performDrop` rejects a drop unless a drag of its own kind + is in flight. `ContentView.swift`. + - **Fix applied (#2)**: symmetric unknown-id no-op tests for + `moveChecklist(id:onto:)` (`dragChecklistOntoUnknownIDIsANoOp`, + `dragUnknownChecklistIsANoOp`). + - **Fix applied (#3)**: doc note that `moveChecklist(id:onto:)` is called once + per row entered during a live-reorder drag. + - **Optional (declined, remains manual)**: on-device check that a long scrub + over rows settles without oscillation — already an explicit manual item in + `implement.md`; static review cannot confirm SwiftUI runtime behavior. + - **Deferred**: duplicate `import CheckStitchCore` at `ContentView.swift:1,3` + is pre-existing on `main`, out of this diff's scope. +- **Remaining manual items**: the device/visual checks listed in + `implement.md` (Phase 1 checklist drag within/loose, cross-section no-op, + boundary drag, edit-mode tap behavior; Phase 2 folder drag, collapsed-folder + drag, row↔header no-op, chevron/Move-to-Folder still work). Sync/render + tickets cannot close on static evidence — verify the installed bundle on the + target. \ No newline at end of file diff --git a/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/implement.md b/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/implement.md new file mode 100644 index 00000000..44219b5e --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/implement.md @@ -0,0 +1,27 @@ +# Implementation Summary + +## Commits +| Phase | Commit | Description | +|-------|--------|-------------| +| 1 | 534ce2b | Checklist drag-reorder (walking skeleton) | +| 2 | 65cdbc1 | Folder drag-reorder | + +## Automated Checks +- [x] `make test-unit` (Phase 1 + Phase 2) — 480 then 485 tests, all passing +- [x] `make build` (iOS simulator, warnings-as-errors) — passed both phases +- [x] `make build-mac` (macOS slice) — passed both phases +- [x] `bash scripts/test.sh` printed `gate: ok` (Phase 2; covers make build/test/build-mac/watch-build, scripts/tests/run.sh, shellcheck) + +## Manual Verification Items (from the plan) + +Phase 1 — Checklist drag-reorder: +- [ ] `make run`; tap Edit; long-press a checklist row and drag it over another row in the same folder — the dragged row takes that slot and the order survives relaunch. +- [ ] In edit mode, drag a loose checklist over a folder member (and vice versa) — both sections are unchanged (cross-section no-op). +- [ ] In edit mode, drag the first loose row past the last loose row — it lands last (boundary). +- [ ] Leave edit mode: tapping a row still pushes the detail screen, and the remove/folder/chevron controls are gone. + +Phase 2 — Folder drag-reorder: +- [ ] `make run`; tap Edit; drag a folder header over another folder header — the folder takes that slot and the order survives relaunch. +- [ ] Drag a collapsed folder's header — it reorders without expanding. +- [ ] Drag a checklist row over a folder header (and a folder header over a checklist row) — no reorder occurs in either direction. +- [ ] The chevron up/down nudges and the "Move to Folder" menu still work. \ No newline at end of file diff --git a/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/medium.md b/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/medium.md new file mode 100644 index 00000000..cdb3845f --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/medium.md @@ -0,0 +1,60 @@ +# Task + +Add a drag-to-move interface to the main checklist list (the `ContentView.swift` +`checklistList` screen, shared by iOS and macOS) so the user can reorder +checklists and folders by dragging instead of (or alongside) the current +up/down nudge buttons. + +Two capabilities are requested: +1. **Reorder folders** — drag a folder header to change its position in the list. +2. **Reorder checklists** — drag a checklist row to reorder it within its own + folder (and within the loose group). Moving a checklist *between* folders or + into/out of the loose group is **optional / "nice to have"** — handle it only + if it drops out cleanly from the same drag plumbing; otherwise keep the + existing "move to folder" context-menu action. + +The list is **not** a SwiftUI `List` — it is a custom `LazyVStack` inside a +`ScrollView` (see `ContentView.swift:checklistList`), so drag-and-drop must be +built with `.onDrag`/`.onDrop` (+ a `DropDelegate`), not `.onMove`. Reordering a +row inside a folder must map the folder's *filtered* member index onto an index +in the store's *global* `checklists` array, since member order within a folder +is derived from global array order. + +Persistence and ordering already exist and must be reused, not rebuilt: +- Folder order **is** the persisted array order; `store.moveFolders(from:to:)` + reorders local-first and stamps no revision. `store.moveChecklists(from:to:)` + is the same for checklists. +- Checklist folder membership is `Checklist.folderID`; `store.moveChecklist(id:toFolder:)` + already files/un-files a checklist (bumps coarse revision for LWW). +- `listVM.moveFolder(id:up:)` / `listVM.moveChecklist(id:up:)` are the existing + up/down nudges this replaces/supplements. + +Do not change the data model, codec version, or sync contract — no migration. + +## Why MEDIUM + +MULTI_MODULE + BROAD_TEST_SURFACE with M1/M2 holding. The change spans the +ContentView list UI, new reorder/position methods in `ChecklistStore` and +`ChecklistListViewModel`, and tests — but the ordering approach (persisted array +order, `folderID`, existing `move*` methods) and the SwiftUI drag/drop technique +are already known, so no research or design sign-off is needed, and there is no +schema/migration or new integration. + +## Key files + +- `CheckStitch/ContentView.swift` — `checklistList`, `folderSection(for:)`, + `checklistRow(for:)`, the folder/checklist context menus and current up/down + buttons (the drag interface replaces/supplements these). +- `CheckStitch/ChecklistListViewModel.swift` — `moveChecklist(id:toFolder:)`, + `moveFolder(id:up:)`, `moveChecklist(id:up:)`, `checklists(in:)`; add + drag-target methods that map filtered/section indices to global store indices. +- `CheckStitch/ChecklistStore.swift` — `moveChecklists(from:to:)` (≈L457), + `moveChecklists`/`moveChecklist(id:toFolder:)` (≈L483), `moveFolders(from:to:)` + (≈L531). May need a method that reorders within a folder's members, or moves a + checklist into a folder at a position. +- `CheckStitchCore/Sources/CheckStitchCore/Checklist.swift` — + `ChecklistGrouping.sections` / `isLoose` (read-only ordering helpers) — + likely untouched. +- Tests: `CheckStitchTests/ChecklistStoreTests.swift`, + `CheckStitchTests/ChecklistListViewModelTests.swift` (new reorder/position + logic, happy + boundary paths); UI smoke in `CheckStitchUITests/`. diff --git a/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/plan.md b/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/plan.md new file mode 100644 index 00000000..36ace9a5 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1102-drag-to-move/plan.md @@ -0,0 +1,442 @@ +# Implementation Plan + +## Overview + +Add drag-to-reorder to the shared iOS/macOS `ContentView` checklist list (a +custom `LazyVStack`, not a SwiftUI `List`), as two tracer slices over the +already-persisted array order: (1) drag a checklist row to reorder it within +its own folder / within the loose group; (2) drag a folder header to reorder +folders. All index mapping lives in `ChecklistListViewModel` (unit-testable); +the store's existing `moveChecklists(from:to:)` / `moveFolders(from:to:)` do +the persistence. The edit-mode chevron nudges and the "Move to Folder" context +menu stay. + +Scope decisions baked in (from `medium.md`): +- **Cross-folder checklist moves are out of scope.** Dragging a row onto another + section is a no-op; filing a checklist into a folder stays with the existing + "Move to Folder" menu (`moveChecklist(id:toFolder:)`), which owns the LWW + revision bump. +- **Drag is enabled in edit mode only**, matching the existing reorder chevrons. + Normal mode keeps the row a plain `NavigationLink` tap target. Flipping this + later is a one-line change to the `isEditing` conditions below. +- No data-model, codec, revision or sync change; no new dependency. + +Recon correction: the view model and store live in the app target +(`CheckStitch/ChecklistListViewModel.swift`, `CheckStitch/ChecklistStore.swift`), +not `CheckStitchCore/` as the AGENTS.md layout section claims. Only +`ChecklistGrouping` lives in the core package. `ChecklistStore` needs **no** +change — `moveChecklists(from:to:)` and `moveFolders(from:to:)` already do the +reorder + persist, with the `moved` index arithmetic (`ChecklistStore.swift:424`). + +Swift 6 note: the app target sets `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor` +and the SDK's `DropDelegate` is `@MainActor` (`@preconcurrency`), so the +delegate below can read `@State` bindings and call the `@MainActor` view model +directly — no `nonisolated`/`assumeIsolated` workaround needed. + +--- + +## Phase 1: Checklist drag-reorder (walking skeleton) + +Thinnest end-to-end path: in edit mode, drag a checklist row over another row in +the **same section**; the dragged row takes the target's slot and the new order +persists. One row builder serves both loose rows and folder members, so this +phase covers "reorder within its own folder" and "reorder within loose" in one +slice. + +### Changes + +#### 1. View model drag mapping + +**File**: `CheckStitch/ChecklistListViewModel.swift` +**Action**: modify + +Add after `moveChecklist(id:up:)` (current end of file, ≈L86): + +```swift +/// The section a checklist renders in: its folder id when that folder is +/// known, otherwise nil (the loose group). Mirrors `checklists(in:)`/`isLoose` +/// so a skewed (unknown) `folderID` gates as loose, exactly as it renders. +private func sectionID(for checklist: Checklist) -> UUID? { + guard let folderID = checklist.folderID, + store.folders.contains(where: { $0.id == folderID }) else { return nil } + return folderID +} + +/// Maps a checklist drag onto the store's global `checklists` array. `targetID` +/// is the row the drag entered: the dragged checklist takes that row's slot in +/// its section, so a downward drag lands after the target and an upward drag +/// before it. Cross-section drags, self-drops and unknown ids are silent +/// no-ops — cross-folder filing stays with `moveChecklist(id:toFolder:)`. +func moveChecklist(id: UUID, onto targetID: UUID) { + guard id != targetID else { return } + guard let from = store.checklists.firstIndex(where: { $0.id == id }), + let to = store.checklists.firstIndex(where: { $0.id == targetID }) else { return } + guard sectionID(for: store.checklists[from]) == sectionID(for: store.checklists[to]) else { return } + store.moveChecklists(from: IndexSet(integer: from), to: from < to ? to + 1 : to) +} +``` + +The `from < to ? to + 1 : to` destination is the `moved` arithmetic +(`ChecklistStore.swift:424`): remove-then-reinsert with the target's slot +displaced, generalized from the nudge's `index + 2` for one step down. + +#### 2. Drag state + reusable drop delegate + +**File**: `CheckStitch/ContentView.swift` +**Action**: modify + +Add next to the existing `@State` properties (≈L28, beside `isEditing`): + +```swift +/// The checklist currently being dragged for reorder; nil when no drag. +@State private var draggingChecklistID: UUID? +``` + +Add file-private next to `struct ContentView`: + +```swift +/// Live-reorder DropDelegate for one row/header in the width-capped card. +/// `dropEntered` moves the dragged item into the target's slot through the view +/// model, so the section→global index mapping stays unit-tested. A nil +/// `draggingID` (no drag of this kind in flight) rejects the drop, so a +/// checklist drag cannot land on a folder header and vice versa. +private struct ReorderDropDelegate: DropDelegate { + let targetID: UUID + @Binding var draggingID: UUID? + let move: (UUID, UUID) -> Void + + func dropUpdated(info: DropInfo) -> DropProposal? { + draggingID == nil ? nil : DropProposal(operation: .move) + } + + func dropEntered(info: DropInfo) { + guard let draggingID, draggingID != targetID else { return } + withAnimation { move(draggingID, targetID) } + } + + func performDrop(info: DropInfo) -> Bool { + draggingID = nil + return true + } +} +``` + +#### 3. Wire the hooks into `checklistRow` + +**File**: `CheckStitch/ContentView.swift` +**Action**: modify + +`checklistRow(for:)` (≈L493) currently ends with the padded `HStack`. Make it +`@ViewBuilder`, bind the row to a local, and apply the hooks only in edit mode +(content unchanged): + +```swift +@ViewBuilder +private func checklistRow(for checklist: Checklist) -> some View { + let row = HStack(spacing: 12) { + // ... existing remove / name / folder-menu / chevrons / navigation + // and reminders-button content, byte-for-byte unchanged ... + } + .padding(.horizontal, 16) + .padding(.vertical, 12) + + if isEditing { + row + .onDrag { + draggingChecklistID = checklist.id + return NSItemProvider(object: checklist.id.uuidString as NSString) + } + .onDrop(of: [.text], delegate: ReorderDropDelegate( + targetID: checklist.id, + draggingID: $draggingChecklistID, + move: { listVM.moveChecklist(id: $0, onto: $1) })) + } else { + row + } +} +``` + +`UniformTypeIdentifiers` is already imported in this file (`.text`). + +#### 4. Tests + +**File**: `CheckStitchTests/ChecklistListViewModelTests.swift` +**Action**: modify + +Append a drag section (Swift Testing, `@MainActor struct`, `#expect`; helpers +`makeViewModel()`, `makeIsolatedDefaults()` already exist): + +```swift +// MARK: Drag to move + +@Test +func dragChecklistDownOntoRowTakesItsSlot() { + let viewModel = makeViewModel() + let first = viewModel.createChecklist() + let second = viewModel.createChecklist() + let third = viewModel.createChecklist() + viewModel.moveChecklist(id: first, onto: third) + #expect(viewModel.checklists.map(\.id) == [second, third, first]) +} + +@Test +func dragChecklistUpOntoRowTakesItsSlot() { + let viewModel = makeViewModel() + let first = viewModel.createChecklist() + let second = viewModel.createChecklist() + let third = viewModel.createChecklist() + viewModel.moveChecklist(id: third, onto: first) + #expect(viewModel.checklists.map(\.id) == [third, first, second]) +} + +@Test +func dragChecklistOntoItselfIsANoOp() { + let viewModel = makeViewModel() + let first = viewModel.createChecklist() + let second = viewModel.createChecklist() + viewModel.moveChecklist(id: first, onto: first) + #expect(viewModel.checklists.map(\.id) == [first, second]) +} + +@Test +func dragChecklistOntoAnotherSectionsRowIsANoOp() { + let viewModel = makeViewModel() + let folder = viewModel.createFolder(name: "Work") + let filed = viewModel.createChecklist() + let loose = viewModel.createChecklist() + viewModel.moveChecklist(id: filed, toFolder: folder) + + viewModel.moveChecklist(id: filed, onto: loose) + + let f = viewModel.folders.first { $0.id == folder }! + #expect(viewModel.checklists(in: f).map(\.id) == [filed]) + #expect(viewModel.checklists(in: nil).map(\.id) == [loose]) +} + +@Test +func dragChecklistWithinFolderReordersAroundOtherFoldersMembers() { + // Global order [c1(fA), x(fB), c2(fA)]: the mapping must skip x entirely. + let viewModel = makeViewModel() + let folderA = viewModel.createFolder(name: "Work") + let folderB = viewModel.createFolder(name: "Personal") + let c1 = viewModel.createChecklist() + let x = viewModel.createChecklist() + let c2 = viewModel.createChecklist() + viewModel.moveChecklist(id: c1, toFolder: folderA) + viewModel.moveChecklist(id: x, toFolder: folderB) + viewModel.moveChecklist(id: c2, toFolder: folderA) + + viewModel.moveChecklist(id: c1, onto: c2) + + let a = viewModel.folders.first { $0.id == folderA }! + let b = viewModel.folders.first { $0.id == folderB }! + #expect(viewModel.checklists(in: a).map(\.id) == [c2, c1]) + #expect(viewModel.checklists(in: b).map(\.id) == [x]) +} + +@Test +func dragChecklistOrderPersistsAndReloads() { + let defaults = makeIsolatedDefaults() + let first = ChecklistListViewModel(store: ChecklistStore(defaults: defaults, textEditDelay: nil)) + let a = first.createChecklist() + let b = first.createChecklist() + first.moveChecklist(id: b, onto: a) + + let reloaded = ChecklistListViewModel(store: ChecklistStore(defaults: defaults, textEditDelay: nil)) + #expect(reloaded.checklists.map(\.id) == [b, a]) +} +``` + +#### 5. Accessibility identifiers (drag affordances) + +Leave the existing `moveChecklistUp-` / `moveChecklistDown-` ids +untouched (the chevrons stay). No new identifier is required — the UI smoke does +not drive drags. Do **not** add a drag-only identifier that no test uses. + +### Verification + +#### Automated +- [x] `make test-unit` passes (new drag tests + existing suite) +- [x] `make build` passes (shared `ContentView` compiles for the iOS simulator) +- [x] `make build-mac` passes (macOS slice of the shared view) + +#### Manual +- [ ] `make run`; tap Edit; long-press a checklist row and drag it over another + row in the same folder — the dragged row takes that slot and the order + survives relaunch. +- [ ] In edit mode, drag a loose checklist over a folder member (and vice + versa) — both sections are unchanged (cross-section no-op). +- [ ] In edit mode, drag the first loose row past the last loose row — it lands + last (boundary). +- [ ] Leave edit mode: tapping a row still pushes the detail screen, and the + remove/folder/chevron controls are gone. + +--- + +## Phase 2: Folder drag-reorder + +Adds the second capability: drag a folder header to change its position among +folders. Reuses `ReorderDropDelegate` from Phase 1 (contract: `targetID`, +`@Binding draggingID`, `move` closure). Depends on Phase 1's delegate, not its +internals. + +### Changes + +#### 1. View model drag mapping + +**File**: `CheckStitch/ChecklistListViewModel.swift` +**Action**: modify + +Add after `moveFolder(id:up:)` (or at the end, next to the Phase 1 method): + +```swift +/// Maps a folder drag onto the store's `folders` array. Same take-the-slot +/// semantics as `moveChecklist(id:onto:)`; self-drops and unknown ids are +/// silent no-ops. Folder order is the persisted array order, so no revision +/// is stamped (mirrors `moveFolders`/`moveFolder(id:up:)`). +func moveFolder(id: UUID, onto targetID: UUID) { + guard id != targetID else { return } + guard let from = store.folders.firstIndex(where: { $0.id == id }), + let to = store.folders.firstIndex(where: { $0.id == targetID }) else { return } + store.moveFolders(from: IndexSet(integer: from), to: from < to ? to + 1 : to) +} +``` + +#### 2. Drag state + hooks on the folder header + +**File**: `CheckStitch/ContentView.swift` +**Action**: modify + +Add beside `draggingChecklistID`: + +```swift +/// The folder currently being dragged for reorder; nil when no drag. +@State private var draggingFolderID: UUID? +``` + +`folderHeader(for:isCollapsed:)` (≈L584, already `@ViewBuilder`) — bind the +existing header `HStack` to a local and apply the hooks in edit mode (content +unchanged): + +```swift +@ViewBuilder +private func folderHeader(for folder: Folder, isCollapsed: Bool) -> some View { + let header = HStack(spacing: 8) { + // ... existing disclosure button, folder glyph, name, edit controls ... + } + .padding(.horizontal, 16) + .padding(.vertical, 8) + + if isEditing { + header + .onDrag { + draggingFolderID = folder.id + return NSItemProvider(object: folder.id.uuidString as NSString) + } + .onDrop(of: [.text], delegate: ReorderDropDelegate( + targetID: folder.id, + draggingID: $draggingFolderID, + move: { listVM.moveFolder(id: $0, onto: $1) })) + } else { + header + } +} +``` + +The two drag kinds are distinguished by their `@State` binding: a checklist +drag leaves `draggingFolderID` nil (folder delegate rejects it), and a folder +drag leaves `draggingChecklistID` nil (checklist delegate rejects it). Folder +headers are all rendered before the loose group, so folder indices map directly +onto `store.folders`. + +#### 3. Tests + +**File**: `CheckStitchTests/ChecklistListViewModelTests.swift` +**Action**: modify + +```swift +@Test +func dragFolderDownOntoFolderTakesItsSlot() { + let viewModel = makeViewModel() + let first = viewModel.createFolder(name: "Work") + let second = viewModel.createFolder(name: "Personal") + let third = viewModel.createFolder(name: "Errands") + viewModel.moveFolder(id: first, onto: third) + #expect(viewModel.folders.map(\.id) == [second, third, first]) +} + +@Test +func dragFolderUpOntoFolderTakesItsSlot() { + let viewModel = makeViewModel() + let first = viewModel.createFolder(name: "Work") + let second = viewModel.createFolder(name: "Personal") + let third = viewModel.createFolder(name: "Errands") + viewModel.moveFolder(id: third, onto: first) + #expect(viewModel.folders.map(\.id) == [third, first, second]) +} + +@Test +func dragFolderOntoItselfIsANoOp() { + let viewModel = makeViewModel() + let first = viewModel.createFolder(name: "Work") + let second = viewModel.createFolder(name: "Personal") + viewModel.moveFolder(id: first, onto: first) + #expect(viewModel.folders.map(\.id) == [first, second]) +} + +@Test +func dragFolderOntoUnknownIDIsANoOp() { + let viewModel = makeViewModel() + let only = viewModel.createFolder(name: "Work") + viewModel.moveFolder(id: only, onto: UUID()) + #expect(viewModel.folders.map(\.id) == [only]) +} + +@Test +func dragFolderOrderPersistsAndReloads() { + let defaults = makeIsolatedDefaults() + let first = ChecklistListViewModel(store: ChecklistStore(defaults: defaults, textEditDelay: nil)) + let a = first.createFolder(name: "Work") + let b = first.createFolder(name: "Personal") + first.moveFolder(id: b, onto: a) + + let reloaded = ChecklistListViewModel(store: ChecklistStore(defaults: defaults, textEditDelay: nil)) + #expect(reloaded.folders.map(\.id) == [b, a]) +} +``` + +### Verification + +#### Automated +- [x] `make test-unit` passes +- [x] `make build` passes (iOS simulator) +- [x] `make build-mac` passes (macOS) +- [x] `bash scripts/test.sh` prints `gate: ok` (run once, after this phase; + it covers `make build`, `make test`, `make build-mac`, `make watch-build`, + `scripts/tests/run.sh`, `shellcheck`) + +#### Manual +- [ ] `make run`; tap Edit; drag a folder header over another folder header — + the folder takes that slot and the order survives relaunch. +- [ ] Drag a collapsed folder's header — it reorders without expanding. +- [ ] Drag a checklist row over a folder header (and a folder header over a + checklist row) — no reorder occurs in either direction. +- [ ] The chevron up/down nudges and the "Move to Folder" menu still work. + +--- + +## Notes for the implementer + +- **Do not change** `ChecklistStore`, the codec version, `Checklist`/ + `Folder`, `ChecklistGrouping`, or any `revision`/`modifiedAt` stamping. + Reorder stays local-first with no revision bump; cross-folder filing keeps + the existing `moveChecklist(id:toFolder:)` LWW path. +- Only these files change: `CheckStitch/ChecklistListViewModel.swift`, + `CheckStitch/ContentView.swift`, + `CheckStitchTests/ChecklistListViewModelTests.swift`. +- New files under `CheckStitch/` need no `project.pbxproj` edit (the project + uses `PBXFileSystemSynchronizedRootGroup`), but this plan adds none. +- No new user-facing strings, so `Localizable.xcstrings` and + `scripts/l10n-check.sh` are untouched. +- Gate legs compile with warnings-as-errors; keep the delegate free of + unused-parameter and actor-isolation warnings. \ No newline at end of file diff --git a/CheckStitch/ChecklistListViewModel.swift b/CheckStitch/ChecklistListViewModel.swift index e5190140..94f748c9 100644 --- a/CheckStitch/ChecklistListViewModel.swift +++ b/CheckStitch/ChecklistListViewModel.swift @@ -54,6 +54,17 @@ final class ChecklistListViewModel { store.moveFolders(from: IndexSet(integer: index), to: up ? index - 1 : index + 2) } + /// Maps a folder drag onto the store's `folders` array. Same take-the-slot + /// semantics as `moveChecklist(id:onto:)`; self-drops and unknown ids are + /// silent no-ops. Folder order is the persisted array order, so no revision + /// is stamped (mirrors `moveFolders`/`moveFolder(id:up:)`). + func moveFolder(id: UUID, onto targetID: UUID) { + guard id != targetID else { return } + guard let from = store.folders.firstIndex(where: { $0.id == id }), + let to = store.folders.firstIndex(where: { $0.id == targetID }) else { return } + store.moveFolders(from: IndexSet(integer: from), to: from < to ? to + 1 : to) + } + /// The folder waiting for its confirm/cancel in the delete dialog; `nil` hides it. var folderPendingRemoval: UUID? @@ -87,4 +98,28 @@ final class ChecklistListViewModel { guard let index = store.checklists.firstIndex(where: { $0.id == id }) else { return } store.moveChecklists(from: IndexSet(integer: index), to: up ? index - 1 : index + 2) } + + /// The section a checklist renders in: its folder id when that folder is + /// known, otherwise nil (the loose group). Mirrors `checklists(in:)`/`isLoose` + /// so a skewed (unknown) `folderID` gates as loose, exactly as it renders. + private func sectionID(for checklist: Checklist) -> UUID? { + guard let folderID = checklist.folderID, + store.folders.contains(where: { $0.id == folderID }) else { return nil } + return folderID + } + + /// Maps a checklist drag onto the store's global `checklists` array. `targetID` + /// is the row the drag entered: the dragged checklist takes that row's slot in + /// its section, so a downward drag lands after the target and an upward drag + /// before it. A single live-reorder drag calls this once per row entered, so + /// each call persists a move. Cross-section drags, self-drops and unknown ids + /// are silent no-ops — cross-folder filing stays with + /// `moveChecklist(id:toFolder:)`. + func moveChecklist(id: UUID, onto targetID: UUID) { + guard id != targetID else { return } + guard let from = store.checklists.firstIndex(where: { $0.id == id }), + let to = store.checklists.firstIndex(where: { $0.id == targetID }) else { return } + guard sectionID(for: store.checklists[from]) == sectionID(for: store.checklists[to]) else { return } + store.moveChecklists(from: IndexSet(integer: from), to: from < to ? to + 1 : to) + } } diff --git a/CheckStitch/ContentView.swift b/CheckStitch/ContentView.swift index 845f2111..48c79c24 100644 --- a/CheckStitch/ContentView.swift +++ b/CheckStitch/ContentView.swift @@ -25,6 +25,10 @@ struct ContentView: View { /// Present when the main-screen rows are in edit mode (remove/move /// controls instead of navigation and the run button). @State private var isEditing = false + /// The checklist currently being dragged for reorder; nil when no drag. + @State private var draggingChecklistID: UUID? + /// The folder currently being dragged for reorder; nil when no drag. + @State private var draggingFolderID: UUID? /// Whether the "New Folder" create alert is open. @State private var isCreatingFolder = false /// Buffered folder name behind the create alert's text field. @@ -318,7 +322,11 @@ struct ContentView: View { // Leave edit mode when the last checklist goes: the empty state has no // toggle, so a later create must not open into a stale edit state. .onChange(of: listVM.checklists.isEmpty) { _, isEmpty in - if isEmpty { isEditing = false } + if isEmpty { + isEditing = false + draggingChecklistID = nil + draggingFolderID = nil + } } } @@ -410,6 +418,10 @@ struct ContentView: View { private var editToggleButton: some View { Button(isEditing ? "Done" : "Edit") { withAnimation { isEditing.toggle() } + if !isEditing { + draggingChecklistID = nil + draggingFolderID = nil + } } .accessibilityIdentifier("editChecklistsButton") } @@ -490,8 +502,9 @@ struct ContentView: View { /// the detail screen, the play button turns the list into reminders. In /// edit mode the row swaps to a leading remove control and plain name text, /// so a tap can neither push the detail screen nor create reminders. + @ViewBuilder private func checklistRow(for checklist: Checklist) -> some View { - HStack(spacing: 12) { + let row = HStack(spacing: 12) { if isEditing { Button { listVM.checklistPendingRemoval = checklist.id @@ -527,6 +540,24 @@ struct ContentView: View { } .padding(.horizontal, 16) .padding(.vertical, 12) + + if isEditing { + row + .onDrag { + // A cancelled drag never reaches `performDrop`, so clear the + // other kind's state: a stale id must not drive a move when + // a later drag of the other kind passes over these rows. + draggingFolderID = nil + draggingChecklistID = checklist.id + return NSItemProvider(object: checklist.id.uuidString as NSString) + } + .onDrop(of: [.text], delegate: ReorderDropDelegate( + targetID: checklist.id, + draggingID: $draggingChecklistID, + move: { listVM.moveChecklist(id: $0, onto: $1) })) + } else { + row + } } /// Trailing per-row move controls in edit mode. The first row cannot move @@ -573,7 +604,7 @@ struct ContentView: View { @ViewBuilder private func folderHeader(for folder: Folder, isCollapsed: Bool) -> some View { - HStack(spacing: 8) { + let header = HStack(spacing: 8) { Button { withAnimation { listVM.setFolderCollapsed(id: folder.id, !isCollapsed) } } label: { @@ -593,6 +624,23 @@ struct ContentView: View { } .padding(.horizontal, 16) .padding(.vertical, 8) + + if isEditing { + header + .onDrag { + // See `checklistRow`: clears the checklist drag's stale id so + // a cancelled checklist drag cannot move a row mid-folder-drag. + draggingChecklistID = nil + draggingFolderID = folder.id + return NSItemProvider(object: folder.id.uuidString as NSString) + } + .onDrop(of: [.text], delegate: ReorderDropDelegate( + targetID: folder.id, + draggingID: $draggingFolderID, + move: { listVM.moveFolder(id: $0, onto: $1) })) + } else { + header + } } /// Trailing per-folder rename/reorder controls in edit mode, mirroring the @@ -693,6 +741,34 @@ struct ContentView: View { } } +/// Live-reorder DropDelegate for one row/header in the width-capped card. +/// `dropEntered` moves the dragged item into the target's slot through the view +/// model, so the section→global index mapping stays unit-tested. A nil +/// `draggingID` (no drag of this kind in flight) rejects the drop, so a +/// checklist drag cannot land on a folder header and vice versa. +private struct ReorderDropDelegate: DropDelegate { + let targetID: UUID + @Binding var draggingID: UUID? + let move: (UUID, UUID) -> Void + + func dropUpdated(info: DropInfo) -> DropProposal? { + draggingID == nil ? nil : DropProposal(operation: .move) + } + + func dropEntered(info: DropInfo) { + guard let draggingID, draggingID != targetID else { return } + withAnimation { move(draggingID, targetID) } + } + + func performDrop(info: DropInfo) -> Bool { + // Reject a foreign drop (no drag of this kind in flight); only clear our + // own id, so an unrelated `.text` drop cannot consume the gesture. + guard draggingID != nil else { return false } + draggingID = nil + return true + } +} + extension ContentView { /// Write-through language binding: reads the live holder (so a value that /// arrives over sync or lands from another scene updates the picker) and diff --git a/CheckStitchTests/ChecklistListViewModelTests.swift b/CheckStitchTests/ChecklistListViewModelTests.swift index 52ba9d1b..16832001 100644 --- a/CheckStitchTests/ChecklistListViewModelTests.swift +++ b/CheckStitchTests/ChecklistListViewModelTests.swift @@ -164,4 +164,150 @@ struct ChecklistListViewModelTests { viewModel.setFolderCollapsed(id: folder, false) #expect(viewModel.folders.first?.isCollapsed == false) } + + // MARK: Drag to move + + @Test + func dragChecklistDownOntoRowTakesItsSlot() { + let viewModel = makeViewModel() + let first = viewModel.createChecklist() + let second = viewModel.createChecklist() + let third = viewModel.createChecklist() + viewModel.moveChecklist(id: first, onto: third) + #expect(viewModel.checklists.map(\.id) == [second, third, first]) + } + + @Test + func dragChecklistUpOntoRowTakesItsSlot() { + let viewModel = makeViewModel() + let first = viewModel.createChecklist() + let second = viewModel.createChecklist() + let third = viewModel.createChecklist() + viewModel.moveChecklist(id: third, onto: first) + #expect(viewModel.checklists.map(\.id) == [third, first, second]) + } + + @Test + func dragChecklistOntoItselfIsANoOp() { + let viewModel = makeViewModel() + let first = viewModel.createChecklist() + let second = viewModel.createChecklist() + viewModel.moveChecklist(id: first, onto: first) + #expect(viewModel.checklists.map(\.id) == [first, second]) + } + + @Test + func dragChecklistOntoUnknownIDIsANoOp() { + let viewModel = makeViewModel() + let first = viewModel.createChecklist() + let second = viewModel.createChecklist() + viewModel.moveChecklist(id: first, onto: UUID()) + #expect(viewModel.checklists.map(\.id) == [first, second]) + } + + @Test + func dragUnknownChecklistIsANoOp() { + let viewModel = makeViewModel() + let first = viewModel.createChecklist() + let second = viewModel.createChecklist() + viewModel.moveChecklist(id: UUID(), onto: second) + #expect(viewModel.checklists.map(\.id) == [first, second]) + } + + @Test + func dragChecklistOntoAnotherSectionsRowIsANoOp() { + let viewModel = makeViewModel() + let folder = viewModel.createFolder(name: "Work") + let filed = viewModel.createChecklist() + let loose = viewModel.createChecklist() + viewModel.moveChecklist(id: filed, toFolder: folder) + + viewModel.moveChecklist(id: filed, onto: loose) + + let f = viewModel.folders.first { $0.id == folder }! + #expect(viewModel.checklists(in: f).map(\.id) == [filed]) + #expect(viewModel.checklists(in: nil).map(\.id) == [loose]) + } + + @Test + func dragChecklistWithinFolderReordersAroundOtherFoldersMembers() { + // Global order [c1(fA), x(fB), c2(fA)]: the mapping must skip x entirely. + let viewModel = makeViewModel() + let folderA = viewModel.createFolder(name: "Work") + let folderB = viewModel.createFolder(name: "Personal") + let c1 = viewModel.createChecklist() + let x = viewModel.createChecklist() + let c2 = viewModel.createChecklist() + viewModel.moveChecklist(id: c1, toFolder: folderA) + viewModel.moveChecklist(id: x, toFolder: folderB) + viewModel.moveChecklist(id: c2, toFolder: folderA) + + viewModel.moveChecklist(id: c1, onto: c2) + + let a = viewModel.folders.first { $0.id == folderA }! + let b = viewModel.folders.first { $0.id == folderB }! + #expect(viewModel.checklists(in: a).map(\.id) == [c2, c1]) + #expect(viewModel.checklists(in: b).map(\.id) == [x]) + } + + @Test + func dragChecklistOrderPersistsAndReloads() { + let defaults = makeIsolatedDefaults() + let first = ChecklistListViewModel(store: ChecklistStore(defaults: defaults, textEditDelay: nil)) + let a = first.createChecklist() + let b = first.createChecklist() + first.moveChecklist(id: b, onto: a) + + let reloaded = ChecklistListViewModel(store: ChecklistStore(defaults: defaults, textEditDelay: nil)) + #expect(reloaded.checklists.map(\.id) == [b, a]) + } + + @Test + func dragFolderDownOntoFolderTakesItsSlot() { + let viewModel = makeViewModel() + let first = viewModel.createFolder(name: "Work") + let second = viewModel.createFolder(name: "Personal") + let third = viewModel.createFolder(name: "Errands") + viewModel.moveFolder(id: first, onto: third) + #expect(viewModel.folders.map(\.id) == [second, third, first]) + } + + @Test + func dragFolderUpOntoFolderTakesItsSlot() { + let viewModel = makeViewModel() + let first = viewModel.createFolder(name: "Work") + let second = viewModel.createFolder(name: "Personal") + let third = viewModel.createFolder(name: "Errands") + viewModel.moveFolder(id: third, onto: first) + #expect(viewModel.folders.map(\.id) == [third, first, second]) + } + + @Test + func dragFolderOntoItselfIsANoOp() { + let viewModel = makeViewModel() + let first = viewModel.createFolder(name: "Work") + let second = viewModel.createFolder(name: "Personal") + viewModel.moveFolder(id: first, onto: first) + #expect(viewModel.folders.map(\.id) == [first, second]) + } + + @Test + func dragFolderOntoUnknownIDIsANoOp() { + let viewModel = makeViewModel() + let only = viewModel.createFolder(name: "Work") + viewModel.moveFolder(id: only, onto: UUID()) + #expect(viewModel.folders.map(\.id) == [only]) + } + + @Test + func dragFolderOrderPersistsAndReloads() { + let defaults = makeIsolatedDefaults() + let first = ChecklistListViewModel(store: ChecklistStore(defaults: defaults, textEditDelay: nil)) + let a = first.createFolder(name: "Work") + let b = first.createFolder(name: "Personal") + first.moveFolder(id: b, onto: a) + + let reloaded = ChecklistListViewModel(store: ChecklistStore(defaults: defaults, textEditDelay: nil)) + #expect(reloaded.folders.map(\.id) == [b, a]) + } }