From c4591ead2c4a8b0ccb01928bb83859eecc10fe5f Mon Sep 17 00:00:00 2001 From: rzen Date: Sat, 8 Aug 2026 14:47:50 -0400 Subject: [PATCH] =?UTF-8?q?Cards=20open=20to=20be=20read=20=E2=80=94=20the?= =?UTF-8?q?=20mobile=20detail=20screen=20turns=20read-only,=20editing=20mo?= =?UTF-8?q?ves=20behind=20a=20transactional=20Save?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CardDetailScreen renders title, inline-Markdown body, and a quiet dates footer; all writing moves to the new CardEditScreen, a full-screen cover with segmented Details/Body panes. Drafts commit in one perform bracket on Save only — Cancel guards dirty drafts with a discard confirmation, and a failed write keeps the sheet and its drafts and raises an alert instead of dismissing. CardAttributesSection becomes a pure binding-driven editor with no write path of its own. The UI test walk crosses the new split, with body-pane and discard coverage, and a deterministic replaceAllText helper retires the flaky ⌘A select-all. Claude-Session: https://claude.ai/code/session_01SR4XGjmBE16ZUYWpfFHXwY --- KanbanMobile/CHANGELOG.md | 4 + .../Screens/CardAttributesSection.swift | 56 ++-- KanbanMobile/Screens/CardDetailScreen.swift | 199 +++++++------ KanbanMobile/Screens/CardEditScreen.swift | 276 ++++++++++++++++++ .../BoardsNavigationUITests.swift | 168 ++++++++--- KanbanMobileUITests/MobileUITestSupport.swift | 41 +++ 6 files changed, 580 insertions(+), 164 deletions(-) create mode 100644 KanbanMobile/Screens/CardEditScreen.swift diff --git a/KanbanMobile/CHANGELOG.md b/KanbanMobile/CHANGELOG.md index 7186e90..854f513 100644 --- a/KanbanMobile/CHANGELOG.md +++ b/KanbanMobile/CHANGELOG.md @@ -1,5 +1,9 @@ **August 2026** +Tapping a card now shows a readable view of its title and text with formatting, instead of dropping straight into the editor. + +Editing a card happens on its own screen with Details and Body sections, and changes apply only when you tap Save. + Press and hold a lane or card to drag it into a new order. Settings now lives behind the gear button on the board list instead of a separate tab, so boards fill the whole screen. diff --git a/KanbanMobile/Screens/CardAttributesSection.swift b/KanbanMobile/Screens/CardAttributesSection.swift index f0353fd..6aeb3c5 100644 --- a/KanbanMobile/Screens/CardAttributesSection.swift +++ b/KanbanMobile/Screens/CardAttributesSection.swift @@ -1,9 +1,14 @@ import SwiftUI -/// The card detail screen's Attributes section: icon, icon colour, and background colour — the -/// three *typed* style fields a card carries (`FrontmatterFields.icon`/`.iconColor`/`.background`). -/// Every pick writes immediately through `BoardWriter.updateIndex` — these are one-tap choices -/// from a fixed set, not free text, so there is no draft to debounce the way title/body have. +/// The card editor's Attributes section: icon, icon colour, and background colour — the three +/// *typed* style fields a card carries (`FrontmatterFields.icon`/`.iconColor`/`.background`). +/// +/// **Drafted, not written.** Every pick here mutates a `Binding` the caller owns — +/// `CardEditScreen`'s own `@State` drafts — rather than reaching for `BoardWriter` itself: the +/// transactional edit model commits title, body, and these three keys together in one `Save`, so +/// this section has no session, no card, and no write code of its own. It is a pure "edit these +/// three bindings" view; the icon picker sheet is unchanged mechanically, it just feeds a binding +/// instead of firing a write. /// /// **There is no labels row.** `labels` is not a field either `BoardModel` or `FrontmatterDocument` /// exposes as typed: `BoardModel.document`'s own doc comment names it as one of the reserved keys @@ -13,10 +18,9 @@ import SwiftUI /// surgical edits exist to keep (agent-written or hand-written overlays round-trip untouched). If /// a typed `labels` field is ever added to the storage schema, its editor belongs in this section. struct CardAttributesSection: View { - let card: Card - let laneID: ItemID - let cardID: ItemID - let session: BoardSession + @Binding var icon: String? + @Binding var iconColor: String? + @Binding var background: String? @State private var isPresentingIconPicker = false @@ -26,7 +30,7 @@ struct CardAttributesSection: View { isPresentingIconPicker = true } label: { LabeledContent("Icon") { - if let icon = card.icon.value, !icon.isEmpty { + if let icon, !icon.isEmpty { Image(systemName: icon) } else { Text("None").foregroundStyle(.secondary) @@ -35,17 +39,17 @@ struct CardAttributesSection: View { } .tint(.primary) - swatchRow(title: "Icon Color", swatches: CardPalette.foregrounds, current: card.iconColor.value) { name in - setStyle(FrontmatterKeys.iconColor, to: name) + swatchRow(title: "Icon Color", swatches: CardPalette.foregrounds, current: iconColor) { name in + iconColor = name } - swatchRow(title: "Background", swatches: CardPalette.backgrounds, current: card.background.value) { name in - setStyle(FrontmatterKeys.background, to: name) + swatchRow(title: "Background", swatches: CardPalette.backgrounds, current: background) { name in + background = name } } .sheet(isPresented: $isPresentingIconPicker) { - IconPickerSheet(current: card.icon.value) { name in - setStyle(FrontmatterKeys.icon, to: name) + IconPickerSheet(current: icon) { name in + icon = name } } } @@ -77,28 +81,6 @@ struct CardAttributesSection: View { } .padding(.vertical, 4) } - - /// Writes one style key immediately. `operation: .style` is the vocabulary's own case for - /// "`updateIndex` on behalf of styling flows" (`WriteOperation.style`); `card.title.value` is - /// read before the closure runs so a failure banner can still name the card by the title on - /// screen. - private func setStyle(_ key: String, to value: String?) { - let laneID = self.laneID - let cardID = self.cardID - let cardTitle = card.title.value - Task { - // The closure's throws type must be spelled out — a trailing closure literal does not - // pick up `perform`'s `throws(BoardWriteError)` from context alone. - await session.perform { (root: URL) throws(BoardWriteError) -> Void in - let folder = root - .appendingPathComponent(laneID.rawValue, isDirectory: true) - .appendingPathComponent(cardID.rawValue, isDirectory: true) - try BoardWriter.updateIndex(inItemFolder: folder, operation: .style(title: cardTitle)) { document in - document.setStyleValue(value, for: key) - } - } - } - } } /// One colour well: a filled circle, a slashed placeholder for "None", and a selection ring. diff --git a/KanbanMobile/Screens/CardDetailScreen.swift b/KanbanMobile/Screens/CardDetailScreen.swift index 6541ad2..b8cfaab 100644 --- a/KanbanMobile/Screens/CardDetailScreen.swift +++ b/KanbanMobile/Screens/CardDetailScreen.swift @@ -1,22 +1,17 @@ import SwiftUI -/// The card editor — title, body, and attributes — `BoardRoute.card`'s destination. +/// The card's read-only view — `BoardRoute.card`'s destination. /// /// Holds board-relative IDs only, never a `Card` value: every body evaluation re-reads the card -/// from `session.snapshot`, so a write this screen makes (or one that lands from elsewhere while -/// it's open) is reflected the moment the session's reload lands, and a card trashed on another -/// device is noticed rather than edited into thin air. +/// from `session.snapshot`, so a write this screen's own edit sheet makes (or one that lands from +/// elsewhere while it's open) is reflected the moment the session's reload lands, and a card +/// trashed on another device is noticed rather than shown stale. /// -/// ### Save timing -/// -/// Title and body are edited into local `@State` drafts and committed through `BoardWriter` only -/// when they differ from the snapshot's own value — never on every keystroke. Three triggers cover -/// every way editing can end without dropping a change: the field's own submit (title, on Return), -/// `onDisappear` (the user navigates back), and `scenePhase` leaving `.active` (the user backgrounds -/// the app, or is interrupted, mid-edit in the body editor). A `Done` toolbar button folds in the -/// same commit and drops focus, for a user who wants an explicit "I'm finished" without leaving the -/// screen. All four funnel through `commitAll()`, so there is exactly one place that decides what -/// "changed" means for each field. +/// **Read-only, by design.** There is no draft here and nothing this screen ever writes — editing +/// lives entirely in `CardEditScreen`, presented full-screen from the Edit button. That split is +/// the whole point of the transactional model: a screen that only ever renders `session.snapshot` +/// cannot itself go stale relative to disk, and there is no auto-save timing to reason about +/// because there is no save at all on this side of the Edit button. struct CardDetailScreen: View { let boardRoot: URL let laneID: ItemID @@ -24,11 +19,8 @@ struct CardDetailScreen: View { @Environment(BoardIndexStore.self) private var index @Environment(\.dismiss) private var dismiss - @Environment(\.scenePhase) private var scenePhase - @State private var titleDraft = "" - @State private var bodyDraft = "" - @FocusState private var isBodyFocused: Bool + @State private var isPresentingEdit = false private var session: BoardSession { index.session(forBoardAt: boardRoot) } @@ -41,58 +33,35 @@ struct CardDetailScreen: View { .navigationTitle("Card") .navigationBarTitleDisplayMode(.inline) .toolbar { - ToolbarItem(placement: .confirmationAction) { - Button("Done") { - isBodyFocused = false - commitAll() + if card != nil { + ToolbarItem(placement: .primaryAction) { + Button("Edit") { isPresentingEdit = true } } } } .task { session.open() } - // Seeds the drafts once per card. `.task(id:)` re-runs only when `card?.id` changes — - // nil while the session is still opening, then the card's own id once it lands — so a - // reload that lands *while this screen is open* (including the reload this screen's - // own commit triggers) never clobbers text the user is mid-typing. - .task(id: card?.id) { - guard let card else { return } - titleDraft = card.title.value ?? "" - bodyDraft = card.body - } - .onDisappear { commitAll() } - .onChange(of: scenePhase) { _, newPhase in - if newPhase != .active { commitAll() } + .fullScreenCover(isPresented: $isPresentingEdit) { + CardEditScreen(boardRoot: boardRoot, laneID: laneID, cardID: cardID) } } @ViewBuilder private var content: some View { if let card { - Form { - if session.lastError != nil { - Section { + ScrollView { + VStack(alignment: .leading, spacing: 20) { + if session.lastError != nil { Label("Showing the last saved version — a recent update didn't load.", systemImage: "exclamationmark.triangle") .font(.footnote) .foregroundStyle(.secondary) } - } - Section("Title") { - TextField("Untitled Card", text: $titleDraft) - .onSubmit { commitTitle(against: card) } - } - - Section("Body") { - TextEditor(text: $bodyDraft) - .frame(minHeight: 200) - .focused($isBodyFocused) - } - - CardAttributesSection(card: card, laneID: laneID, cardID: cardID, session: session) - - Section("Details") { - LabeledContent("Created", value: card.created.value.map(Self.formatted) ?? "—") - LabeledContent("Modified", value: card.modified.value.map(Self.formatted) ?? "—") + titleBlock(for: card) + bodyBlock(for: card) + detailsFooter(for: card) } + .padding() + .frame(maxWidth: .infinity, alignment: .leading) } } else if case .ready = session.phase { ContentUnavailableView("Card Removed", systemImage: "trash", description: Text("This card was deleted.")) @@ -102,56 +71,100 @@ struct CardDetailScreen: View { } } - private func commitAll() { - guard let card else { return } - commitTitle(against: card) - commitBody(against: card) + /// The title, its icon when the card has one, and a subtle background tint lifted from the + /// card's own `background` swatch — passive display only, no swatches to tap here. An + /// unrecognized colour name (the same leniency `ItemIconView`/`CardPalette` document for + /// themselves) just skips the tint rather than guessing at one. + @ViewBuilder + private func titleBlock(for card: Card) -> some View { + HStack(alignment: .firstTextBaseline, spacing: 10) { + ItemIconView(icon: card.icon.value, iconColor: card.iconColor.value) + titleText(for: card) + } + .padding() + .frame(maxWidth: .infinity, alignment: .leading) + .background( + (tintColor(for: card)?.opacity(0.15) ?? Color.clear), + in: RoundedRectangle(cornerRadius: 12) + ) } - private func commitTitle(against card: Card) { - let trimmed = titleDraft.trimmingCharacters(in: .whitespacesAndNewlines) - guard trimmed != (card.title.value ?? "") else { return } - let laneID = self.laneID - let cardID = self.cardID - let newValue: String? = trimmed.isEmpty ? nil : trimmed - Task { - // The closure's throws type must be spelled out — a trailing closure literal does not - // pick up `perform`'s `throws(BoardWriteError)` from context alone. - await session.perform { (root: URL) throws(BoardWriteError) -> Void in - let folder = root - .appendingPathComponent(laneID.rawValue, isDirectory: true) - .appendingPathComponent(cardID.rawValue, isDirectory: true) - // The Mac's own rename path, verbatim (`BoardStore.setTitle`): plain `set`/`remove` - // — `setStyleValue` is the style gesture's helper, not the title's — and - // `.rename(title: nil)` so `updateIndex` enriches the operation from the document - // it reads rather than trusting a snapshot that may have aged. - try BoardWriter.updateIndex(inItemFolder: folder, operation: .rename(title: nil)) { document in - if let newValue { - document.set(FrontmatterKeys.title, to: .string(newValue)) - } else { - document.remove(FrontmatterKeys.title) - } + @ViewBuilder + private func titleText(for card: Card) -> some View { + if let title = card.title.value, !title.isEmpty { + Text(title) + .font(.title.bold()) + } else { + Text("Untitled Card") + .font(.title.bold()) + .foregroundStyle(.secondary) + } + } + + private func tintColor(for card: Card) -> Color? { + card.background.value.flatMap { CardPalette.color(named: $0, in: CardPalette.backgrounds) } + } + + /// The body, rendered as Markdown — see `MarkdownBlocks` for why this is a per-paragraph + /// `AttributedString(markdown:)` pass rather than a full block parser. + @ViewBuilder + private func bodyBlock(for card: Card) -> some View { + let paragraphs = MarkdownBlocks.paragraphs(in: card.body) + if paragraphs.isEmpty { + Text("No description") + .foregroundStyle(.secondary) + } else { + VStack(alignment: .leading, spacing: 12) { + ForEach(Array(paragraphs.enumerated()), id: \.offset) { _, paragraph in + Text(MarkdownBlocks.rendered(paragraph)) } } } } - private func commitBody(against card: Card) { - guard bodyDraft != card.body else { return } - let laneID = self.laneID - let cardID = self.cardID - let newBody = bodyDraft - Task { - await session.perform { (root: URL) throws(BoardWriteError) -> Bool in - let folder = root - .appendingPathComponent(laneID.rawValue, isDirectory: true) - .appendingPathComponent(cardID.rawValue, isDirectory: true) - return try BoardWriter.writeBody(inItemFolder: folder, body: newBody) - } + @ViewBuilder + private func detailsFooter(for card: Card) -> some View { + VStack(alignment: .leading, spacing: 4) { + Text("Created \(card.created.value.map(Self.formatted) ?? "—")") + Text("Modified \(card.modified.value.map(Self.formatted) ?? "—")") } + .font(.footnote) + .foregroundStyle(.secondary) + .padding(.top, 8) } private static func formatted(_ date: Date) -> String { date.formatted(date: .abbreviated, time: .shortened) } } + +/// Dependency-free Markdown rendering for a card body: no `swift-markdown` block parser here, just +/// `Foundation`'s own `AttributedString(markdown:options:)` run once per paragraph. +/// +/// **Why per-paragraph.** `AttributedString`'s built-in parser has no "render this as a sequence of +/// blocks" mode short of `.full`, which produces a block tree this view would still have to walk — +/// no simpler than doing the walk ourselves, and it collapses the very newlines a card body relies +/// on to separate paragraphs. Splitting on blank lines first and parsing each paragraph with +/// `.inlineOnlyPreservingWhitespace` gets both halves of what a card body needs cheaply: inline +/// emphasis/links/code render as styled runs, and the newlines *inside* a paragraph (soft line +/// breaks) survive as literal whitespace instead of being folded into a single space. +enum MarkdownBlocks { + + /// Splits `body` on runs of one or more blank lines — Markdown's own paragraph boundary — and + /// trims each block, so a body with trailing blank lines or extra spacing between paragraphs + /// doesn't render phantom empty blocks. + static func paragraphs(in body: String) -> [String] { + body + .components(separatedBy: "\n\n") + .map { $0.trimmingCharacters(in: .whitespacesAndNewlines) } + .filter { !$0.isEmpty } + } + + /// One paragraph, inline-parsed. Malformed Markdown (an unterminated code span, say) falls back + /// to the raw paragraph text rather than dropping it — the same "never lose the author's text" + /// posture `CardPalette.color(named:in:)` takes for an unrecognized colour name. + static func rendered(_ paragraph: String) -> AttributedString { + let options = AttributedString.MarkdownParsingOptions(interpretedSyntax: .inlineOnlyPreservingWhitespace) + return (try? AttributedString(markdown: paragraph, options: options)) ?? AttributedString(paragraph) + } +} diff --git a/KanbanMobile/Screens/CardEditScreen.swift b/KanbanMobile/Screens/CardEditScreen.swift new file mode 100644 index 0000000..7bd6f4a --- /dev/null +++ b/KanbanMobile/Screens/CardEditScreen.swift @@ -0,0 +1,276 @@ +import SwiftUI + +/// The card editor — title, body, and attributes, all drafted and committed together — presented +/// full-screen from `CardDetailScreen`'s Edit button. +/// +/// Holds board-relative IDs only, never a `Card` value, for the same reason every other screen in +/// this stack does: the card is re-read from `session.snapshot` on every body evaluation, so a +/// write landing from elsewhere while this sheet is open (a trash from another device, say) is +/// noticed rather than edited into thin air. +/// +/// ### Transactional, not auto-saving +/// +/// Every earlier screen in this app commits a field the moment it stops being edited — `onSubmit`, +/// `onDisappear`, a scene-phase change. This one commits nothing until `Save`, deliberately: a +/// `fullScreenCover` has no interactive swipe-to-dismiss, so the only way out mid-edit is a button +/// this screen controls, which makes "ask before discarding" both possible and the more honest +/// default for a screen with five drafted fields open across two panes at once. There is no +/// scenePhase or onDisappear commit anywhere below — backgrounding the app or losing focus leaves +/// the drafts exactly where they were, waiting for `Save` or `Cancel`. +/// +/// `Cancel` on a clean sheet dismisses immediately; on a dirty one it raises a confirmation before +/// discarding. `Save` writes every changed field in one `session.perform` bracket — title through +/// `.rename`, the three style keys through one `.style` call, body through `writeBody` — writes +/// nothing at all when nothing changed, and dismisses only on success: a failed write keeps the +/// sheet and its drafts and raises an alert (see `save()`). +struct CardEditScreen: View { + let boardRoot: URL + let laneID: ItemID + let cardID: ItemID + + @Environment(BoardIndexStore.self) private var index + @Environment(\.dismiss) private var dismiss + + private enum Pane: String, CaseIterable, Hashable { + case details = "Details" + case body = "Body" + } + + @State private var pane: Pane = .details + + @State private var titleDraft = "" + @State private var bodyDraft = "" + @State private var iconDraft: String? + @State private var iconColorDraft: String? + @State private var backgroundDraft: String? + @State private var isConfirmingDiscard = false + @State private var saveError: BoardSessionError? + @FocusState private var isBodyFocused: Bool + + private var session: BoardSession { index.session(forBoardAt: boardRoot) } + + private var card: Card? { + session.snapshot?.lanes.first { $0.id == laneID }?.cards.first { $0.id == cardID } + } + + var body: some View { + NavigationStack { + content + .navigationTitle("Card") + .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .cancellationAction) { + Button("Cancel") { requestDismiss() } + } + if card != nil { + ToolbarItem(placement: .principal) { + Picker("Pane", selection: $pane) { + ForEach(Pane.allCases, id: \.self) { pane in + Text(pane.rawValue).tag(pane) + } + } + .pickerStyle(.segmented) + .frame(maxWidth: 220) + } + ToolbarItem(placement: .confirmationAction) { + Button("Save") { save() } + } + } + } + .task { session.open() } + // Seeds the drafts once per card. `.task(id:)` re-runs only when `card?.id` + // changes — nil while the session is still opening, then the card's own id once + // it lands — so a reload that lands *while this sheet is open* never clobbers a + // draft the user is mid-typing. Unlike the old in-place editor, nothing this + // screen does triggers such a reload before `Save` — but another device's write + // can still land, and the same guard covers that case too. + .task(id: card?.id) { + guard let card else { return } + titleDraft = card.title.value ?? "" + bodyDraft = card.body + iconDraft = card.icon.value + iconColorDraft = card.iconColor.value + backgroundDraft = card.background.value + } + .confirmationDialog( + "Discard Changes?", + isPresented: $isConfirmingDiscard, + titleVisibility: .visible + ) { + Button("Discard", role: .destructive) { dismiss() } + Button("Cancel", role: .cancel) {} + } + .alert( + "Couldn't Save", + isPresented: Binding(get: { saveError != nil }, set: { if !$0 { saveError = nil } }), + presenting: saveError + ) { _ in + Button("OK") { saveError = nil } + } message: { error in + Text(error.description) + } + } + } + + @ViewBuilder + private var content: some View { + if let card { + switch pane { + case .details: + detailsForm(for: card) + case .body: + TextEditor(text: $bodyDraft) + .focused($isBodyFocused) + .padding(.horizontal, 12) + .padding(.top, 8) + } + } else if case .ready = session.phase { + // The card is gone from a `.ready` snapshot — deleted elsewhere while this sheet was + // open. There is nothing left to edit; dismiss the cover rather than leave drafts for + // a card that no longer exists. + ContentUnavailableView("Card Removed", systemImage: "trash", description: Text("This card was deleted.")) + .task { dismiss() } + } else { + ProgressView("Opening card") + } + } + + @ViewBuilder + private func detailsForm(for card: Card) -> some View { + Form { + if session.lastError != nil { + Section { + Label("Showing the last saved version — a recent update didn't load.", systemImage: "exclamationmark.triangle") + .font(.footnote) + .foregroundStyle(.secondary) + } + } + + Section("Title") { + TextField("Untitled Card", text: $titleDraft) + } + + CardAttributesSection(icon: $iconDraft, iconColor: $iconColorDraft, background: $backgroundDraft) + + Section("Details") { + LabeledContent("Created", value: card.created.value.map(Self.formatted) ?? "—") + LabeledContent("Modified", value: card.modified.value.map(Self.formatted) ?? "—") + } + } + } + + /// Any draft differing from the card's own current snapshot value — title compared trimmed, + /// exactly as `Save` compares it, so this predicate and the write it gates never disagree. + private var isDirty: Bool { + guard let card else { return false } + let trimmedTitle = titleDraft.trimmingCharacters(in: .whitespacesAndNewlines) + return trimmedTitle != (card.title.value ?? "") + || bodyDraft != card.body + || iconDraft != card.icon.value + || iconColorDraft != card.iconColor.value + || backgroundDraft != card.background.value + } + + private func requestDismiss() { + if isDirty { + isConfirmingDiscard = true + } else { + dismiss() + } + } + + /// One `session.perform` bracket, three independent writes inside it gated on whatever + /// actually changed — a clean `Save` (nothing dirty) makes no write at all and just dismisses. + /// Awaits the perform before dismissing: `perform` already awaits its own reload, so by the + /// time this returns `CardDetailScreen`'s snapshot underneath already reflects the write, and + /// the read screen shows the new values the instant the cover animates away. + /// + /// Dismissal is gated on the perform's own `Result` — the one caller of `perform` in this app + /// that can't ignore it. The auto-committing screens could fire and forget because their + /// drafts outlived the write attempt; here dismissing *is* draft destruction, so a failed + /// write keeps the sheet (and every draft) exactly where it was and raises an alert instead. + /// `perform` reloads even on failure, so a partially-applied multi-call write (title landed, + /// body didn't) re-seeds nothing — the drafts keep the user's full intent and a retry rewrites + /// only what still differs. + private func save() { + guard let card else { + dismiss() + return + } + + let trimmedTitle = titleDraft.trimmingCharacters(in: .whitespacesAndNewlines) + let titleChanged = trimmedTitle != (card.title.value ?? "") + let newTitle: String? = trimmedTitle.isEmpty ? nil : trimmedTitle + + let iconChanged = iconDraft != card.icon.value + let iconColorChanged = iconColorDraft != card.iconColor.value + let backgroundChanged = backgroundDraft != card.background.value + let styleChanged = iconChanged || iconColorChanged || backgroundChanged + + let bodyChanged = bodyDraft != card.body + + guard titleChanged || styleChanged || bodyChanged else { + dismiss() + return + } + + let laneID = self.laneID + let cardID = self.cardID + let cardTitle = card.title.value + let newIcon = iconDraft + let newIconColor = iconColorDraft + let newBackground = backgroundDraft + let newBody = bodyDraft + + Task { + // The closure's throws type must be spelled out — a trailing closure literal does not + // pick up `perform`'s `throws(BoardWriteError)` from context alone. + let outcome = await session.perform { (root: URL) throws(BoardWriteError) -> Void in + let folder = root + .appendingPathComponent(laneID.rawValue, isDirectory: true) + .appendingPathComponent(cardID.rawValue, isDirectory: true) + + if titleChanged { + // The Mac's own rename path, verbatim (`BoardStore.setTitle`): plain + // `set`/`remove` — `setStyleValue` is the style gesture's helper, not the + // title's — and `.rename(title: nil)` so `updateIndex` enriches the operation + // from the document it reads rather than trusting a snapshot that may have + // aged. + try BoardWriter.updateIndex(inItemFolder: folder, operation: .rename(title: nil)) { document in + if let newTitle { + document.set(FrontmatterKeys.title, to: .string(newTitle)) + } else { + document.remove(FrontmatterKeys.title) + } + } + } + + if styleChanged { + // One `.style` call for all three keys — `operation: .style` is the + // vocabulary's own case for "`updateIndex` on behalf of styling flows" + // (`WriteOperation.style`), and each changed key writes through + // `setStyleValue` exactly as the old immediate-write attributes section did. + try BoardWriter.updateIndex(inItemFolder: folder, operation: .style(title: cardTitle)) { document in + if iconChanged { document.setStyleValue(newIcon, for: FrontmatterKeys.icon) } + if iconColorChanged { document.setStyleValue(newIconColor, for: FrontmatterKeys.iconColor) } + if backgroundChanged { document.setStyleValue(newBackground, for: FrontmatterKeys.background) } + } + } + + if bodyChanged { + _ = try BoardWriter.writeBody(inItemFolder: folder, body: newBody) + } + } + switch outcome { + case .success: + dismiss() + case let .failure(error): + saveError = error + } + } + } + + private static func formatted(_ date: Date) -> String { + date.formatted(date: .abbreviated, time: .shortened) + } +} diff --git a/KanbanMobileUITests/BoardsNavigationUITests.swift b/KanbanMobileUITests/BoardsNavigationUITests.swift index ee35419..0aec30e 100644 --- a/KanbanMobileUITests/BoardsNavigationUITests.swift +++ b/KanbanMobileUITests/BoardsNavigationUITests.swift @@ -1,43 +1,31 @@ import XCTest /// Board list → lanes → cards → card detail, then an edit that has to reach disk — the one -/// end-to-end walk of the mobile MVP's navigation stack (`BoardRoute`'s three destinations). +/// end-to-end walk of the mobile MVP's navigation stack (`BoardRoute`'s three destinations), plus +/// the transactional edit sheet `CardDetailScreen`'s Edit button presents. final class BoardsNavigationUITests: XCTestCase { @MainActor func testBoardsNavigationAndTitleEdit() throws { let (app, root) = XCUIApplication.launchedWithFixtureBoard() + app.navigateToFirstCard() - // Boards is the app's root screen (`BoardsTabView`), so no navigation is needed to reach - // it. The row's label is the flattened `BoardSummaryRow` — title plus the "N lanes · N - // cards · modified" subtitle `BoardIndexStore`'s first scan fills in. - let boardRow = app.element(labelContaining: RichBoard.title) + // CardDetailScreen is read-only: the fixture card's title renders as plain text, not a + // field — the read/edit split this suite now has to cross to reach any field at all. XCTAssertTrue( - boardRow.waitForExistence(timeout: XCUIApplication.uiTimeout), - "the \"\(RichBoard.title)\" row never appeared — check the fixture copy or the first scan" + app.staticTexts[RichBoard.firstLaneFirstCard].waitForExistence(timeout: XCUIApplication.uiTimeout), + "the read view's title text never appeared" ) - boardRow.tap() - let laneRow = app.element(labelContaining: RichBoard.firstLane) - XCTAssertTrue( - laneRow.waitForExistence(timeout: XCUIApplication.uiTimeout), - "the \"\(RichBoard.firstLane)\" lane row never appeared" - ) - laneRow.tap() + app.openCardEdit() - let cardRow = app.element(labelContaining: RichBoard.firstLaneFirstCard) - XCTAssertTrue( - cardRow.waitForExistence(timeout: XCUIApplication.uiTimeout), - "the \"\(RichBoard.firstLaneFirstCard)\" card row never appeared" - ) - cardRow.tap() - - // CardDetailScreen: the only `textField` on this screen is the title - // (`Section("Title")`) — the body is a `TextEditor`, which is a `textView`. + // CardEditScreen opens on the Details pane; its only textField is the title + // (`Section("Title")`) — the body pane is a bare `TextEditor`, a `textView`, and is not + // this pane's concern. let titleField = app.textFields.firstMatch XCTAssertTrue( titleField.waitForExistence(timeout: XCUIApplication.uiTimeout), - "CardDetailScreen's title field never appeared" + "CardEditScreen's title field never appeared" ) XCTAssertEqual( titleField.value as? String, RichBoard.firstLaneFirstCard, @@ -45,21 +33,133 @@ final class BoardsNavigationUITests: XCTestCase { ) let sentinel = "Retitled by UI test" - titleField.tap() - // Select-all via a hardware-keyboard shortcut (iOS answers ⌘A the same as macOS when a - // keyboard is attached, which the Simulator always presents one as) rather than a - // backspace-per-character workaround, whose delete count only works if the tap happened - // to land the cursor at the end of the existing text. - titleField.typeKey("a", modifierFlags: .command) - titleField.typeText(sentinel) + titleField.replaceAllText(with: sentinel) - // Commit the way the screen commits (`CardDetailScreen.body`'s `.confirmationAction`): - // the Done button folds `commitAll()` in and drops focus, without navigating back. - app.navigationBars.buttons["Done"].tap() + app.navigationBars.buttons["Save"].tap() + // The cover dismisses back to the read view underneath, which should already show the new + // title — `session.perform` awaits its own reload before `Save` dismisses. + XCTAssertTrue( + app.staticTexts[sentinel].waitForExistence(timeout: XCUIApplication.uiTimeout), + "the read view never picked up the retitled card" + ) XCTAssertTrue( waitForFile(under: root, containing: sentinel), "the retitled card never landed on disk under \(root.path)" ) } + + @MainActor + func testCardEditBodyPaneWritesToDisk() throws { + let (app, root) = XCUIApplication.launchedWithFixtureBoard() + app.navigateToFirstCard() + app.openCardEdit() + + app.buttons["Body"].tap() + + let bodyEditor = app.textViews.firstMatch + XCTAssertTrue( + bodyEditor.waitForExistence(timeout: XCUIApplication.uiTimeout), + "the Body pane's text editor never appeared" + ) + + let sentinel = "Rewritten body by UI test" + bodyEditor.replaceAllText(with: sentinel) + + app.navigationBars.buttons["Save"].tap() + + // The read view renders the body as Markdown; a plain sentinel with no markup renders + // back out as itself. + XCTAssertTrue( + app.staticTexts[sentinel].waitForExistence(timeout: XCUIApplication.uiTimeout), + "the read view never picked up the rewritten body" + ) + XCTAssertTrue( + waitForFile(under: root, containing: sentinel), + "the rewritten body never landed on disk under \(root.path)" + ) + } + + @MainActor + func testCardEditCancelWithDirtyDraftsDiscardsOnDisk() throws { + let (app, root) = XCUIApplication.launchedWithFixtureBoard() + app.navigateToFirstCard() + app.openCardEdit() + + let titleField = app.textFields.firstMatch + XCTAssertTrue( + titleField.waitForExistence(timeout: XCUIApplication.uiTimeout), + "CardEditScreen's title field never appeared" + ) + + let sentinel = "Abandoned edit" + titleField.replaceAllText(with: sentinel) + + // Cancel on a dirty sheet raises a confirmation rather than dismissing outright — the + // sheet's only exit, since a `fullScreenCover` has no interactive swipe-dismiss. + app.navigationBars.buttons["Cancel"].tap() + + let discardButton = app.buttons["Discard"] + XCTAssertTrue( + discardButton.waitForExistence(timeout: XCUIApplication.uiTimeout), + "the discard confirmation never appeared for a dirty Cancel" + ) + discardButton.tap() + + // Back on the read view, still showing the fixture's own title — nothing was ever written. + XCTAssertTrue( + app.staticTexts[RichBoard.firstLaneFirstCard].waitForExistence(timeout: XCUIApplication.uiTimeout), + "the read view did not return to the card's original title after Discard" + ) + XCTAssertTrue( + fileDoesNotContain(under: root, substring: sentinel), + "the discarded title leaked onto disk under \(root.path)" + ) + } +} + +private extension XCUIApplication { + + /// Boards → the fixture's first lane → its first card, landing on `CardDetailScreen`'s read + /// view. Shared by every test in this file so the navigation-and-assert boilerplate is written + /// once. + @MainActor + func navigateToFirstCard() { + let boardRow = element(labelContaining: RichBoard.title) + XCTAssertTrue( + boardRow.waitForExistence(timeout: XCUIApplication.uiTimeout), + "the \"\(RichBoard.title)\" row never appeared — check the fixture copy or the first scan" + ) + boardRow.tap() + + let laneRow = element(labelContaining: RichBoard.firstLane) + XCTAssertTrue( + laneRow.waitForExistence(timeout: XCUIApplication.uiTimeout), + "the \"\(RichBoard.firstLane)\" lane row never appeared" + ) + laneRow.tap() + + let cardRow = element(labelContaining: RichBoard.firstLaneFirstCard) + XCTAssertTrue( + cardRow.waitForExistence(timeout: XCUIApplication.uiTimeout), + "the \"\(RichBoard.firstLaneFirstCard)\" card row never appeared" + ) + cardRow.tap() + } + + /// Taps `CardDetailScreen`'s Edit button and waits for `CardEditScreen`'s cover to land. + @MainActor + func openCardEdit() { + let editButton = navigationBars.buttons["Edit"] + XCTAssertTrue( + editButton.waitForExistence(timeout: XCUIApplication.uiTimeout), + "the read view's Edit button never appeared" + ) + editButton.tap() + + XCTAssertTrue( + navigationBars.buttons["Save"].waitForExistence(timeout: XCUIApplication.uiTimeout), + "CardEditScreen's cover never appeared" + ) + } } diff --git a/KanbanMobileUITests/MobileUITestSupport.swift b/KanbanMobileUITests/MobileUITestSupport.swift index 2f40166..7d17101 100644 --- a/KanbanMobileUITests/MobileUITestSupport.swift +++ b/KanbanMobileUITests/MobileUITestSupport.swift @@ -208,6 +208,39 @@ extension XCUIApplication { } } +extension XCUIElement { + + /// Replaces this field's/editor's entire text with `text`, wholesale. + /// + /// **Neither ⌘A nor the long-press callout survived contact with this suite.** ⌘A is delivered + /// as a `UIKeyCommand` down the responder chain, and that routing is not guaranteed wired up the + /// instant `tap()` returns — behind a `fullScreenCover`'s own presentation transition, or right + /// after a `TextEditor` is freshly mounted by a pane switch, ⌘A sent too early is silently + /// dropped while ordinary character input (a different, lower-level path) still lands, so the + /// retyped text ends up inserted at whatever the stale cursor position was rather than replacing + /// anything — and a fixed settle before it only wins *sometimes*, because under a full suite + /// run's extra load the gap it needs to cover stretches past any delay worth hard-coding. The + /// long-press "Select All" callout fares worse: it never appeared at all in this environment (the + /// Simulator's hardware keyboard appears to suppress the touch selection UI outright). + /// + /// What is left, and has been reliable through every run: plain character `typeText` always + /// lands, so the fix sidesteps selection entirely. A tap near the field's trailing/bottom edge — + /// past wherever the current text actually ends — is where both `UITextField` and `UITextView` + /// place the caret at the *end* of the text (the standard "tap in the empty run-out" behavior, + /// not an assumption this suite is inventing), which is what makes a plain backspace-per-character + /// safe here where the original in-place editor's own comment once ruled it out: that caveat was + /// about a tap landing *mid-text*, not about the technique itself. + @MainActor + func replaceAllText(with text: String) { + let trailingEdge = coordinate(withNormalizedOffset: CGVector(dx: 0.95, dy: 0.5)) + trailingEdge.tap() + if let current = value as? String, !current.isEmpty { + typeText(String(repeating: XCUIKeyboardKey.delete.rawValue, count: current.count)) + } + typeText(text) + } +} + // MARK: - Polling the filesystem /// Waits up to `timeout` for some file under `root` (searched recursively) to contain @@ -223,6 +256,14 @@ func waitForFile(under root: URL, containing substring: String, timeout: TimeInt return fileExists(under: root, containing: substring) } +/// A point-in-time negative check — `waitForFile`'s substring test with no polling, for asserting a +/// draft was never written. Discarding a `CardEditScreen` never calls `session.perform` at all, so +/// there is no async write to wait out; polling the full timeout for something that never becomes +/// true would only slow the suite down for no better an answer than one immediate look. +func fileDoesNotContain(under root: URL, substring: String) -> Bool { + !fileExists(under: root, containing: substring) +} + /// Waits up to `timeout` for the directory at `url` to exist — or, with `toExist: false`, to be /// gone. The package-shaped counterpart to `waitForFile(under:containing:)`, and what a move /// assertion needs: `relocateBoard` hands the actual transfer to a detached task and answers the UI