From 10f4e9b4529f2385bd9249dbb2190f324eb5f27c Mon Sep 17 00:00:00 2001 From: rzen Date: Sun, 9 Aug 2026 11:22:23 -0400 Subject: [PATCH] =?UTF-8?q?A=20lane's=20context=20menu=20finds=20its=20fiv?= =?UTF-8?q?e=20groups=20=E2=80=94=20style,=20clipboard,=20width,=20the=20f?= =?UTF-8?q?old,=20and=20the=20trash?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LaneView.laneMenu/laneActions restructured to the owner's shape (card 22f660b0), twinning the card menu's own reshape from earlier today (72ca222, card fe66c461): Rename/Properties…/Style ▸ (Symbol, Color), a divider, then Copy/Cut/Paste, a divider, then Width — Increase/Decrease/Reset, a divider, then Collapse Lane / Expand Lane, a divider, then Send to Trash (relabeled from "Delete"). Every row routes through existing machinery — no new commands, no new store method. ClipboardStore's targeted copy(from:targeting:)/cut(from:targeting:) — added for the card menu — are reused verbatim here: a new clipboardTarget computed property wraps targetIDs (the lane's existing widen-to-selection rule) as an ItemReferenceSet, exactly the card menu's clipboardTarget one type over. Paste does not retarget, the card menu's own posture (a destination operation with no per-item widening precedent). Reset Width is exactly setLaneWidth(lane.id, units: 1) — the same call Decrease already makes at the floor — which setLaneWidth's own remove-at-default rule already turns into an absent width key, so no new write path is needed. Style ▸ Symbol and ▸ Color both open the one existing style popover (the card menu's v1 posture, unchanged), and the quick-style recents row is dropped from this menu for symmetry with the card menu's own drop — StyleMenuItems is no longer called from LaneView, though it and QuickStyleRow are left in place (unused, easy to restore) exactly as the card menu's own commit chose to leave them. Properties… is a disabled placeholder row, the owner's own word, left out of laneActions since an always-disabled row has nothing to announce a custom action for. Unlike CardFaceView, LaneView's Copy/Cut/Paste enablement (copyEnabled/ cutEnabled/pasteEnabled) calls ClipboardStore's real predicates directly rather than reducing them to selection/snapshot-free forms: this body is already unconditionally subscribed to store.selection (isSelected) and store.snapshot (headerInk) every pass — the struct's own "Equality gate" doc section says so — so nothing new is subscribed, and the predicate cost is paid once per lane (a handful) rather than once per card (hundreds), the axis the card menu's own reduction was protecting. Journaled on the card: Style's one-popover posture and the dropped recents row both flagged "needs owner review" (mirroring the card menu's own flags); DESIGN/11-command-nexus.md's Lane row is owed a rewrite, left for the main session, same as the card menu's commit left its own Card row. Tests: a new targeted copy/cut test proving ClipboardStore's targeted overloads work on a lane id (writes a lane manifest, arms a lane pending-cut) — the exact call LaneView's new Copy/Cut rows make — plus the full existing suite: 3220 tests, 3 pre-existing environmental failures (PointerLatencyTests, confirmed by isolated rerun, unrelated to this change), all else passing. Claude-Session: https://claude.ai/code/session_014PtZdPwqZuqEDLc6wZMtEy --- Kanban/UI/Board/LaneView.swift | 212 +++++++++++++++++++++++++++---- KanbanTests/ClipboardTests.swift | 32 +++++ 2 files changed, 218 insertions(+), 26 deletions(-) diff --git a/Kanban/UI/Board/LaneView.swift b/Kanban/UI/Board/LaneView.swift index fd450a0..1bf9f1a 100644 --- a/Kanban/UI/Board/LaneView.swift +++ b/Kanban/UI/Board/LaneView.swift @@ -22,9 +22,10 @@ import SwiftUI /// ### The lane's one context menu /// /// "The lane has one context menu (settled), invoked on the header or on lane empty space alike" -/// (03-board-ui.md § Lane), so both surfaces attach the *same* `laneMenu`. It carries Rename, Style…, -/// the quick-style recents row, the Width stepper and Delete — 11-command-nexus.md ▸ Context menus' -/// Lane row, in its order, complete as of m5. +/// (03-board-ui.md § Lane), so both surfaces attach the *same* `laneMenu`. **Redesigned 2026-08-09** +/// (Pipeline card 22f660b0) to the owner's five groups — Rename/Properties…/Style, Copy/Cut/Paste, +/// Width, Collapse/Expand, Send to Trash — `laneMenu`'s own doc comment carries the full shape and +/// every journaled deviation. /// /// ### Collapsed: the slim strip /// @@ -488,10 +489,9 @@ struct LaneView: View, Equatable { // **The context menu's plain rows, additionally as custom actions** — "where SwiftUI // additionally surfaces menu items as custom accessibility actions, that's free // improvement, not a separate design surface" (10-accessibility.md). The menu itself - // stays the inventory and is reachable the standard way (VO-⇧-M); this is the same four - // commands one rotor turn closer. Style… is deliberately absent: it opens a popover — - // its own accessible surface — and the quick-style swatch `Picker` beside it is not an - // action at all. + // stays the inventory and is reachable the standard way (VO-⇧-M); this is the same + // commands one rotor turn closer — see `laneActions`' own doc comment for what is left out + // and why (Style…, Properties…). .accessibilityActions { laneActions } // The lane's half of the Style… popover. Anchored on the header because that is the // lane's own furniture — `styleEditorPresentation` decides whether this lane is the @@ -538,45 +538,154 @@ struct LaneView: View, Equatable { // MARK: - The lane's one context menu - /// Rename, Style…, the quick-style recents row, the Width control, Delete — 11-command-nexus.md ▸ - /// Context menus' Lane row, in its order, complete as of m5. + /// **The lane context menu, redesigned** (2026-08-09 ▸ "redesign context menu for lanes", Pipeline + /// card 22f660b0, twinning the card menu's own reshape the same day — card fe66c461): five groups, + /// a divider between each, the owner's shape verbatim — + /// + /// 1. Rename, Properties…, Style ▸ (Symbol, Color) + /// 2. Copy, Cut, Paste + /// 3. Width — Increase, Decrease, Reset + /// 4. Collapse Lane / Expand Lane + /// 5. Send to Trash + /// + /// Every row routes through the *existing* command it twins — nothing here is new capability, + /// only a new arrangement of ones the app already has (`BoardRenameCommand`, `StyleEditorSession`, + /// `ClipboardStore`, `store.setLaneWidth`, `store.setLaneCollapsed`, `store.delete`). + /// + /// ### Properties… — a placeholder, deliberately inert + /// + /// The owner's own word for it. There is no properties pane yet, so the row opens nothing and is + /// unconditionally `.disabled(true)` — reserved furniture for a future card, not a stub wired to + /// dead code. It is left out of `laneActions` below for the same reason: a VoiceOver custom action + /// that always does nothing has nothing to announce a reason for, where a sighted row can at least + /// be *seen* greyed out beside its live neighbors. + /// + /// ### Style ▸ Symbol / Color — one popover, not two (v1, flagged for owner review) + /// + /// The card menu's own posture, unchanged here: `StyleEditorSession` carries only a `StyleTarget`, + /// no notion of "arrived here for the symbol section" versus "the color section", so both rows + /// open the identical existing popover rather than two that do not exist. Flagged for owner + /// review exactly as the card menu's is: a future pass that teaches the editor to pre-focus a + /// section should aim these two rows at it. + /// + /// **The quick-style recents row is gone from this menu too**, for symmetry with the card menu's + /// own drop (`CardFaceView.boardMenu`'s doc comment) — the owner's list names only Symbol and Color + /// under Style, neither the card's list nor this one mentions the recents row, and leaving it on + /// the lane alone while the card dropped it would be an asymmetry nothing asked for. `StyleMenuItems` + /// (the row's old bundle of "Style…" plus `QuickStyleRow`) is no longer called from here; this menu + /// builds the submenu directly, `Menu("Style") { Button("Symbol"); Button("Color") }`, the card + /// menu's own shape. **Flagged for owner review** — easy to bring back as a third row under Style + /// if the drop was not intended. + /// + /// ### Copy / Cut / Paste — targeting + /// + /// **Copy and Cut widen exactly as Delete and Style already do** — `clipboardTarget`, `targetIDs`'s + /// `ItemReferenceSet` wrapper, the card menu's own extension of "right-clicking something outside + /// the selection acts on what was clicked" to the clipboard (`ClipboardStore.copy(from:targeting:)`/ + /// `cut(from:targeting:)`, added for the card menu, reused verbatim here). Multi-selection semantics + /// are `ClipboardStore`'s own: a multi-lane selection copies/cuts every member, in flatten order. + /// + /// **Paste does not retarget to the clicked lane** — the card menu's own posture: Paste is a + /// *destination* operation (`PasteTarget`), not an item operation, with no per-item widening + /// precedent anywhere in this codebase. This row calls the one existing rule + /// (`ClipboardStore.paste(into:)`) rather than inventing a click-anchored variant, exactly as the + /// card menu's Paste row does. + /// + /// ### Width — Increase / Decrease / Reset + /// + /// The existing labeled `Section` (`widthControl`), gaining a third row: **Reset Width** calls + /// `store.setLaneWidth(lane.id, units: 1)` — the *exact* call Decrease already makes at the floor, + /// no new store method. `setLaneWidth`'s own remove-at-default rule already turns a landing on one + /// unit into an absent `width` key (`LaneWidthWriteTests.landingOnOneRemovesTheKey`), so "reset" + /// falls out of machinery that already exists rather than adding a second write path that has to + /// agree with the first. Disabled at the identical floor Decrease already respects: a lane + /// already displaying one unit has nothing to reset. + /// + /// ### Collapse / Expand — one row, title flips + /// + /// `collapseControl`, unchanged: "a menu row that toggles a *geometry* reads better as the verb it + /// performs than as a state it reports" (its own doc comment) — this app's one context-menu toggle + /// precedent, distinct from the menu-bar's stable-titled checkmark `Toggle` (`ShowTrashCommand`), + /// which lives in `Commands` rather than a `.contextMenu` and needs a stable title as a defaults key. + /// + /// ### Send to Trash + /// + /// Relabeled from "Delete" — the owner's own word for the group, and the more accurate one for + /// what this row does: the staged move into `/.trash/` (`store.delete`, unchanged), not a + /// permanent delete (that word stays the trash lane's own menu, out of this card's scope). Lanes + /// rejoined the trash 2026-07-29/30 (`f7c8088`, `8014bde`); this row has called `store.delete` + /// through that whole history and still does. + /// + /// ### Render-safety: why Copy/Cut/Paste read the real predicates here, unlike the card menu + /// + /// The card menu reduces its own Copy/Cut/Paste `.disabled` checks to selection/snapshot-free forms + /// because a card face is otherwise never subscribed to `store.selection`/`store.snapshot` at all — + /// `.contextMenu`'s builder is not lazy, so reading either there would be the very O(board) + /// regression `isSelected` exists to prevent (`CardFaceView.boardMenu`'s own doc comment). + /// + /// **That premise does not hold for a lane.** This struct's own "Equality gate" section states it + /// plainly: `store.selection` is read unconditionally, every body pass, for `isSelected` + /// (`.overlay(selectionStroke)`, outside the collapsed/expanded split), and `store.snapshot` is + /// read unconditionally for `headerInk` (both branches) — "`.equatable()` has no say" over either. + /// So calling `ClipboardStore`'s real `canCopy(from:targeting:)`/`canCut(from:targeting:)`/ + /// `canPaste(into:)` here (`copyEnabled`/`cutEnabled`/`pasteEnabled` below) adds no *new* + /// subscription — only the predicate's own cheap work, paid once per **lane** (a handful on any + /// board) rather than once per **card** (hundreds), which is the axis the card menu's reduction + /// was actually protecting. No O(board) traversal is added either way: every predicate here reads + /// `clipboardTarget` (at most the live selection) against the already-resident `store.snapshot`. @ViewBuilder private var laneMenu: some View { + // Group 1 — Rename, Properties…, Style ▸ (Symbol, Color). + // // Rename: Board ▸ Rename's exact store path (`BoardRenameCommand`) — `beginRename(of: // currentTitle:)`, seeded with the lane's live title. The menu-bar item additionally requires // this lane to be the *sole* selection; a context menu already names its target by where it // was invoked, so — standard macOS practice — it acts on the clicked lane outright. Button("Rename") { beginRename() } .disabled(!store.acceptsBoardMutations) + // Properties…: deliberately inert — see the type comment's own section. + Button("Properties…") {} + .disabled(true) + Menu("Style") { + // Both rows open the identical popover — see the type comment's "one popover, not two". + Button("Symbol") { store.transient.beginStyleEditor(for: styleTarget) } + .disabled(!store.acceptsBoardMutations) + Button("Color") { store.transient.beginStyleEditor(for: styleTarget) } + .disabled(!store.acceptsBoardMutations) + } + // Both rows share this exact condition, so the submenu itself greys out with them rather than + // opening to reveal two disabled rows. + .disabled(!store.acceptsBoardMutations) Divider() - // Deferred, `CardFaceView`'s reason (`StyleMenuItems`): a non-escaping menu builder makes an - // eagerly computed target a body-time selection read. - StyleMenuItems(store: store, recents: appModel.styleRecents, target: { styleTarget }) + // Group 2 — Copy, Cut, Paste. + Button("Copy") { appModel.clipboard.copy(from: store, targeting: clipboardTarget) } + .disabled(!copyEnabled) + Button("Cut") { appModel.clipboard.cut(from: store, targeting: clipboardTarget) } + .disabled(!cutEnabled) + Button("Paste") { appModel.clipboard.paste(into: store) } + .disabled(!pasteEnabled) Divider() + // Group 3 — Width: Increase, Decrease, Reset. widthControl - // **The fold's own row, under the width control it belongs beside** (03-board-ui.md § Lane ▸ - // Collapsed lanes) — one row whose title says which direction it goes, the Show/Hide Trash - // shape rather than a checkmark: a menu row that toggles a *geometry* reads better as the verb - // it performs than as a state it reports. - // - // **Single-lane, like the width stepper above it and unlike Style… and Delete**: the design - // gives the width batch to the ⌥⌘→/⌥⌘← menu items and keeps the stepper on the lane whose menu - // is open, and the fold has no keyboard face to carry a batch. So this row acts on the clicked - // lane outright, which is also what its singular title promises. + Divider() + + // Group 4 — the fold's own row, one row whose title says which direction it goes — see the + // type comment's own section. collapseControl .disabled(!store.acceptsBoardMutations) Divider() - // Delete: File ▸ Delete's exact store path (`store.delete`), on the same widened target set - // Style… above reads (`targetIDs`, `styleTarget`'s `Set` sibling below) — the - // successor-selection rule is `delete(_:)`'s own, so this row gets it for free. - Button("Delete") { deleteTargets() } + // Group 5 — Send to Trash: File ▸ Delete's exact store path (`store.delete`), on the same + // widened target set Style… above reads (`targetIDs`, `styleTarget`'s `Set` sibling + // below) — the successor-selection rule is `delete(_:)`'s own, so this row gets it for free. + // Relabeled from "Delete" — see the type comment. + Button("Send to Trash") { deleteTargets() } .disabled(!store.acceptsBoardMutations) } @@ -584,18 +693,30 @@ struct LaneView: View, Equatable { /// call site). Every one of them calls the *same* private method its menu row does, so the two /// surfaces cannot drift into meaning different things — which is the only way "not a separate /// design surface" is checkable rather than merely intended. + /// + /// **Style and Properties… are absent** — Style for `CardFaceView.boardActions`' own reason (it + /// opens a popover, its own accessible surface); Properties… for the type comment's own reason (an + /// always-disabled row with nothing behind it has nothing to announce a custom action for). @ViewBuilder private var laneActions: some View { let units = LaneLayoutMath.displayUnits(of: lane) Button("Rename") { beginRename() } .disabled(!store.acceptsBoardMutations) + Button("Copy") { appModel.clipboard.copy(from: store, targeting: clipboardTarget) } + .disabled(!copyEnabled) + Button("Cut") { appModel.clipboard.cut(from: store, targeting: clipboardTarget) } + .disabled(!cutEnabled) + Button("Paste") { appModel.clipboard.paste(into: store) } + .disabled(!pasteEnabled) Button("Increase Width") { store.setLaneWidth(lane.id, units: units + 1) } .disabled(!store.acceptsBoardMutations) Button("Decrease Width") { store.setLaneWidth(lane.id, units: units - 1) } .disabled(!store.acceptsBoardMutations || units <= 1) + Button("Reset Width") { store.setLaneWidth(lane.id, units: 1) } + .disabled(!store.acceptsBoardMutations || units <= 1) collapseControl .disabled(!store.acceptsBoardMutations) - Button("Delete") { deleteTargets() } + Button("Send to Trash") { deleteTargets() } .disabled(!store.acceptsBoardMutations) } @@ -639,6 +760,9 @@ struct LaneView: View, Equatable { /// precise control … it never touches the window, it **re-divides** the existing width across the /// new unit total" (03-board-ui.md § Lane). A +/− pair rather than a slider or a fixed 1×/2×/3× /// list, because the control is uncapped in one direction and floored at one unit in the other. + /// **Reset Width** (2026-08-09 ▸ "redesign context menu for lanes") joins them at the same floor — + /// see `laneMenu`'s own "Width" section for why it is exactly `setLaneWidth(lane.id, units: 1)` + /// and no new store method. /// /// **Single-lane by nature**, unlike the style entries above it: the design gives the batch to /// the ⌥⌘→/⌥⌘← menu items and keeps the stepper on the lane whose menu is open. @@ -654,6 +778,11 @@ struct LaneView: View, Equatable { // A one-unit lane cannot shrink (`width` is ≥ 1), and an item whose only outcome is a // no-op reads better disabled than dead — `LaneWidthCommands`' rule, same floor. .disabled(units <= 1) + Button("Reset Width") { + store.setLaneWidth(lane.id, units: 1) + } + // Nothing to reset at the floor already — the same no-op-reads-disabled rule as Decrease. + .disabled(units <= 1) } .disabled(!store.acceptsBoardMutations) } @@ -678,6 +807,37 @@ struct LaneView: View, Equatable { return store.selection.ids } + // MARK: - Copy / Cut / Paste + + /// Copy's and Cut's own widened target — `styleTarget`/`targetIDs`' exact pairing, one type over + /// (`CardFaceView.clipboardTarget`'s twin): the clicked lane alone, or the live selection when the + /// clicked lane is a member of it. `.board`, always — a lane's context menu never opens from the + /// trash (`TrashLaneView` carries its own menu, out of this card's scope). + private var clipboardTarget: ItemReferenceSet { + ItemReferenceSet(ids: targetIDs, container: .board) + } + + /// Copy's context-menu enablement — the real predicate, not a render-safe reduction of it. See + /// `laneMenu`'s own "Render-safety" section for why a lane face can afford to call + /// `ClipboardStore.canCopy(from:targeting:)` directly where `CardFaceView` cannot: this body is + /// already unconditionally subscribed to `store.selection`/`store.snapshot` (`isSelected`, + /// `headerInk`), so nothing new is subscribed by reading them again here. + private var copyEnabled: Bool { + appModel.clipboard.canCopy(from: store, targeting: clipboardTarget) + } + + /// `copyEnabled`'s cut twin — `canCut(from:targeting:)`, `copyEnabled`'s own reasoning. + private var cutEnabled: Bool { + appModel.clipboard.canCut(from: store, targeting: clipboardTarget) + } + + /// Paste's context-menu enablement. **Does not retarget to the clicked lane** — `canPaste(into:)`, + /// the same generic call ⌘V and Edit ▸ Paste use, the card menu's own Paste row's reasoning: Paste + /// is a destination operation with no per-item widening precedent anywhere in this codebase. + private var pasteEnabled: Bool { + appModel.clipboard.canPaste(into: store) + } + private var headerContent: some View { HStack(alignment: .firstTextBaseline, spacing: BoardMetrics.laneHeaderSpacing(bodyPointSize: pointSize)) { Image(systemName: ItemSymbol.name(lane.icon, fallback: ItemSymbol.lane)) diff --git a/KanbanTests/ClipboardTests.swift b/KanbanTests/ClipboardTests.swift index 6aef1a1..88efc46 100644 --- a/KanbanTests/ClipboardTests.swift +++ b/KanbanTests/ClipboardTests.swift @@ -746,6 +746,38 @@ struct ClipboardTargetedCopyCutTests { #expect(!harness.clipboard.canCopy(from: harness.store, targeting: target)) #expect(!harness.clipboard.canCut(from: harness.store, targeting: target)) } + + /// **The lane context menu's own Copy/Cut** (`LaneView`, 2026-08-09 ▸ "redesign context menu for + /// lanes") — the identical targeted overloads the card menu's Copy/Cut already exercise above, + /// aimed at a lane id instead of a card id for the first time. Nothing in `ClipboardStore` branches + /// on kind at the API surface, but `SelectionGrammar.kind(of:in:)` does internally + /// (`canCopy(from:targeting:)`'s own guard), so this is worth its own test rather than assumed by + /// analogy: a lane target must write a **lane** manifest, and it must ignore an unrelated live + /// card selection exactly as the card menu's targeted copy ignores an unrelated live selection. + @Test("A targeted copy/cut of a lane writes a lane manifest, ignoring an unrelated live selection") + func targetedCopyAndCutWorkOnALaneID() async throws { + let harness = try makeClipboardHarness() + defer { harness.tearDown() } + + // The live selection names a card the target never mentions — `LaneView.clipboardTarget`'s + // own "right-clicking something outside the selection acts on what was clicked" widening. + harness.store.select([clipboardCard4], in: .board) + let target = ItemReferenceSet(ids: [clipboardLane1], container: .board) + + #expect(harness.clipboard.canCopy(from: harness.store, targeting: target)) + #expect(harness.clipboard.canCut(from: harness.store, targeting: target)) + + harness.clipboard.copy(from: harness.store, targeting: target) + await harness.clipboard.stagingSettled() + + let manifest = try #require(harness.clipboard.payload) + #expect(manifest.kind == .lane) + #expect(manifest.entries.map(\.id) == [Ident.lane1]) + + harness.clipboard.cut(from: harness.store, targeting: target) + #expect(harness.store.transient.pendingCut.ids == [clipboardLane1]) + #expect(harness.store.transient.pendingCut.container == .board) + } } // MARK: - Availability