rzen/lanework
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Navigation's Move Left/Right learn a real card — the context menu's disabled rows compute a per-card cross-lane destination and land it through the drop's own rank machinery

Browse Source 
CardMoveTarget (BoardCommands.swift), LaneMoveTarget's cousin: validates
the clicked card's own current lane against the live lane order rather
than the board's live selection, refuses a target that reaches outside
that lane (no coherent left for a spread), and answers an index —
the clicked card's own position in its lane, clamped — for the
adjacent live lane in either direction. Trash is never a candidate
(not a Lane); a collapsed lane is a fine landing (a fold hides cards,
it doesn't close the lane).

CardFaceView's Navigation rows now call moveCardAcrossLane(by:), which
hands CardMoveTarget's answer straight to BoardStore.moveCards(_:toLane:at:)
— the exact call a released drag makes, so rank-minting, the undo step,
the watcher echo and the banner all come free. Targeting is Copy/Cut's
own widening (targetIDs): the clicked card, or the live selection when
the clicked card is a member of it.

Enablement stays render-safe the way isSelected/selectedCount already
are: three new CardFaceView parameters (hasLeftNeighbor, hasRightNeighbor,
selectionSpansLanes) are hoisted once per lane in LaneView.scrollableCards
and handed down as compared parameters, never read from inside a card
face's own .disabled. Caught and fixed a real regression here during
development: an early cut of the multi-lane-spread check answered true
for any lane that simply didn't contain the selected card, which
flipped a compared parameter for most of the board on an ordinary
single-card select and defeated CardFaceView's equality gate wholesale
(BoardRenderPerformanceTests.selectionStillRepaints caught it at 151 of
180 card faces).

Tests: CardMoveTargetTests (KeyboardGrammarTests.swift) pins the pure
predicate — leftmost/rightmost lane, single lane, index clamping, a
widened group anchoring on the clicked member rather than its own
extent, the multi-lane-spread refusal, and an integration test feeding
the answer straight through moveCards. CardFaceViewEquatableTests
gains a case pinning the three new compared parameters.

Claude-Session: https://claude.ai/code/session_014PtZdPwqZuqEDLc6wZMtEy
This commit is contained in:
rzen
2026-08-09 10:37:55 -04:00
parent c27cc93ec1
commit 085a84abb2
5 changed files with 519 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Kanban/UI/Board/CardFaceView.swift
+151 -42
View File
@@ -183,6 +183,63 @@ struct CardFaceView: View, Equatable {
/// face, and one is it.
let selectedCount: Int
/// Whether this face's own lane has a live neighbour one step to the **left** — Navigation ▸ Move
/// Left's render-safe enablement input (2026-08-09 ▸ "Give the card context menu's Navigation rows
/// real card-move behavior", card 06322636), `isSelected`'s exact pattern one row over: the parent
/// (`LaneView`) reads `store.snapshot` **once per lane** (`CardMoveTarget.hasNeighbor`) and hands
/// every face in it the answer, so a click never costs a per-face board walk. Defaults `false` —
/// the trash side never constructs Navigation rows at all, so its faces never need a real answer
/// here (`CardFaceRole`).
let hasLeftNeighbor: Bool
/// `hasLeftNeighbor`'s mirror, for Move Right.
let hasRightNeighbor: Bool
/// Whether the **live board selection** reaches outside this face's own lane — the "no coherent
/// left for a spread" refusal (owner ruling, card 06322636): a selection spanning more than one
/// lane disables Navigation for every one of its own selected members, because the widened target
/// (`targetIDs`) would name cards `CardMoveTarget.destination` can never place in one direction.
/// Only read together with `isSelected` — an unselected face's target is always itself alone,
/// trivially confined to its own lane regardless of what this says (`moveLeftEnabled`).
///
/// Lane-hoisted like the two neighbour flags above (`CardMoveTarget.selectionSpansOtherLanes`):
/// the parent already reads the selection once for `isSelected`/`selectedCount`, so this costs one
/// more `Set` comparison over cards it already has in hand, never a second `store.selection` read.
let selectionSpansLanes: Bool
/// Explicit only for `hasLeftNeighbor`/`hasRightNeighbor`/`selectionSpansLanes` — the three
/// Navigation inputs, all defaulted to `false` (every row reads disabled) so the trash side's one
/// call site (`TrashLaneView.cardRow`, whose faces never draw Navigation at all) and the pre-move
/// equatable-gate tests need no opinion about a feature their construction never exercises. Every
/// other parameter stays required — the memberwise init's own shape — because the board side
/// (`LaneView.scrollableCards`) always has a real answer for all eight and a silently-defaulted
/// `card` or `isSelected` would be the wrong kind of convenience.
init(
store: BoardStore,
card: Card,
role: CardFaceRole,
marquee: MarqueeControl,
drops: BoardDropContext,
hero: URL?,
isSelected: Bool,
selectedCount: Int,
hasLeftNeighbor: Bool = false,
hasRightNeighbor: Bool = false,
selectionSpansLanes: Bool = false
) {
self.store = store
self.card = card
self.role = role
self.marquee = marquee
self.drops = drops
self.hero = hero
self.isSelected = isSelected
self.selectedCount = selectedCount
self.hasLeftNeighbor = hasLeftNeighbor
self.hasRightNeighbor = hasRightNeighbor
self.selectionSpansLanes = selectionSpansLanes
}
/// The app-wide quick-style recents — see `LaneView`'s own note.
@Environment(AppModel.self) private var appModel
@@ -253,6 +310,14 @@ struct CardFaceView: View, Equatable {
/// the deferred `styleTarget` read the selection the same way, inside actions, which is why they
/// stayed as they were.
///
/// **The three Navigation inputs joined 2026-08-09** (card 06322636), `isSelected`'s exact
/// reason one more time: `hasLeftNeighbor`/`hasRightNeighbor`/`selectionSpansLanes` are what let
/// `moveLeftEnabled`/`moveRightEnabled` answer without a `store.snapshot`/`store.selection` read
/// inside this face's own `.disabled` — so a gate that swallowed them would leave a face's
/// Navigation rows wearing a stale enabled state after a neighbouring lane folded, a wall lane's
/// board-edge shifted, or the live selection grew across lanes, none of which touch `card`, `hero`
/// or the other selection figures.
///
/// Deliberately NOT compared: `@State` (per-identity, preserved across updates anyway),
/// `@Environment` values (SwiftUI invalidates on those itself), and the `.board` role's
/// `openCard` closure (see `CardFaceRole.isEquivalent(to:)`).
@@ -262,6 +327,9 @@ struct CardFaceView: View, Equatable {
&& lhs.hero == rhs.hero
&& lhs.isSelected == rhs.isSelected
&& lhs.selectedCount == rhs.selectedCount
&& lhs.hasLeftNeighbor == rhs.hasLeftNeighbor
&& lhs.hasRightNeighbor == rhs.hasRightNeighbor
&& lhs.selectionSpansLanes == rhs.selectionSpansLanes
&& lhs.store === rhs.store
&& lhs.marquee.isEquivalent(to: rhs.marquee)
&& lhs.drops.isEquivalent(to: rhs.drops)
@@ -688,10 +756,14 @@ struct CardFaceView: View, Equatable {
/// 3. Navigation ▸ (Move Left, Move Right)
/// 4. Send to Trash
///
/// Every row below routes through the *existing* command it twins — nothing here is a new
/// capability, only a new arrangement of ones the app already has (`OpenCardCommand`,
/// `BoardRenameCommand`, `StyleCommand`/`StyleEditorSession`, `ClipboardStore`,
/// `LaneMoveTarget`/`MoveLaneCommands`, `store.delete`/`TrashCommands`).
/// Every row below routes through an *existing* write primitive it twins — nothing here invents a
/// new way to touch disk, only new arrangements and, for one group, a new targeting rule over ones
/// the app already has (`OpenCardCommand`, `BoardRenameCommand`, `StyleCommand`/`StyleEditorSession`,
/// `ClipboardStore`, `store.delete`/`TrashCommands`). **Navigation is the one exception to "new
/// arrangement, not new capability"**: this group shipped structurally in fe66c461 (wired but
/// unconditionally disabled — no per-card cross-lane move predicate existed yet) and gained the
/// real one in 06322636, `CardMoveTarget`, over the same `BoardStore.moveCards(_:toLane:at:)` a
/// released drag already calls — see that section below for the full story.
///
/// ### Two deviations from the card's literal list, both kept and both journaled
///
@@ -724,29 +796,39 @@ struct CardFaceView: View, Equatable {
/// which only asked for the *card* menu). Flagged for owner review: easy to bring back as a third
/// row under Style if the drop was not intended.
///
/// ### Navigation ▸ Move Left / Move Right — wired, but unconditionally disabled (flagged)
/// ### Navigation ▸ Move Left / Move Right — a real per-card cross-lane move (2026-08-09, card
/// 06322636)
///
/// `LaneMoveTarget.destination` — Move Left/Right's one predicate, shared with the menu-bar row —
/// reads the **board's live selection**, not the clicked card, and answers `nil` for any card id
/// unconditionally ("a card id is in no lane order" — see its own doc comment). A card's own
/// context menu can therefore never itself be a case where this predicate answers `true` for
/// *this* card; the only way the row could ever be live is if some unrelated lane happened to be
/// the board's live selection at the same time — a coincidence with nothing to do with the card
/// that was right-clicked, and confusing to expose ("Move Left" on a card silently moving some
/// other lane).
/// **Not `LaneMoveTarget.destination`** — that predicate reads the **board's live selection** and
/// answers `nil` for any card id unconditionally ("a card id is in no lane order" — its own doc
/// comment), because it is Board ▸ Move Left/Right's *lane*-selection predicate, shared with the
/// menu-bar row. These two rows validate against `CardMoveTarget.destination` instead — the
/// cousin built for exactly this card, over this face's own `targetIDs` (Copy/Cut's widening:
/// the clicked card, or the live selection when the clicked card is a member of it) rather than
/// the live selection's lane membership.
///
/// Reading the live selection to chase that coincidence would also cost real render performance:
/// unlike every other row here, `LaneMoveTarget.destination` needs `store.selection` and
/// `store.snapshot`, and `.contextMenu`'s builder is **not lazy** — SwiftUI evaluates `boardMenu`
/// (and every `.disabled` inside it) on every ordinary body pass, the exact regression
/// `isSelected`/`selectedCount` exist to prevent (this struct's own top-of-file note;
/// `BoardRenderPerformanceTests.selectionStillRepaints`). So the two rows are wired to the real
/// store call (`moveLane`, via `LaneMoveTarget.destination`) for when this is revisited, but
/// `.disabled(true)` unconditionally rather than paying an O(board) selection read for a row that
/// is a structural mismatch for a card menu in the first place. Flagged for owner review: a
/// genuine per-card "move this card's lane" affordance would be new targeting behavior — out of this
/// card's "menu structure, not new behavior" scope — and the owner's card body may simply not have
/// anticipated the sole-lane restriction Move Left/Right already carries (11-command-nexus.md).
/// **Destination is the adjacent live lane**, skipping nothing (owner ruling): the trash is never
/// a candidate (it is not a `Lane`) and a collapsed lane is a perfectly good landing — a fold
/// hides cards, it does not close the lane. **Position is index-preserving** — the clicked card's
/// own index in its lane's display order, carried over to the destination and clamped to its card
/// count, with the rest of a widened group riding along in their existing relative order
/// (`CardMoveTarget`'s own type comment has the full reasoning). The write is the ordinary
/// `BoardStore.moveCards(_:toLane:at:)` — the exact call a released drag makes — so rank-minting,
/// the undo step, the watcher echo and the banner all come free from that one path.
///
/// **Enablement stays render-safe the same way every other row here does.** `CardMoveTarget`
/// needs `store.snapshot` (for the neighbouring lane) and, when the clicked card is itself
/// selected, `store.selection` (for the multi-lane-spread refusal) — both of which are forbidden
/// inside this face's own `.disabled` (this struct's top-of-file note; `.contextMenu`'s builder is
/// not lazy). So neither is read here: `hasLeftNeighbor`/`hasRightNeighbor` and
/// `selectionSpansLanes` are lane-hoisted compared parameters, `isSelected`'s exact pattern —
/// `LaneView.scrollableCards` reads `store.snapshot`/`store.selection` **once per lane** and hands
/// every face in it the answer, so a click costs `O(lane count)`, never `O(board cards)`
/// (`moveLeftEnabled`/`moveRightEnabled`; `BoardRenderPerformanceTests` stays green throughout).
///
/// **A selection spanning more than one lane disables both rows** (owner ruling: "no coherent
/// left for a spread") — `selectionSpansLanes`, consulted only when `isSelected` is true, since an
/// unselected clicked card's own target is always itself alone.
///
/// ### Copy / Cut / Paste / Paste Special — targeting
///
@@ -799,6 +881,14 @@ struct CardFaceView: View, Equatable {
/// kind-checks are always satisfied and the rows reduce to `isEditingInline`/`acceptsBoardMutations`
/// (`copyEnabled`/`store.acceptsBoardMutations`) without reading the target at all for `.disabled`
/// — the target is still read, correctly, inside each action (`clipboardTarget`).
///
/// **Navigation ▸ Move Left / Move Right take the third route**: unlike Paste and Copy/Cut, there
/// is no algebraic reduction that makes `CardMoveTarget.destination` free of `store.snapshot`/
/// `store.selection` — a neighbouring lane and a cross-lane selection spread are both genuinely
/// board-wide facts. So the parent resolves them instead, **once per lane**, and hands this face
/// the answer as three more compared parameters (`hasLeftNeighbor`, `hasRightNeighbor`,
/// `selectionSpansLanes`) — `isSelected`/`selectedCount`'s own precedent, not a new one
/// (`moveLeftEnabled`/`moveRightEnabled`).
@ViewBuilder
private func boardMenu(openCard: @escaping (ItemID) -> Void) -> some View {
// Group 1 — Open, Copy Link, Rename, Style ▸ (Symbol, Color). One run, no divider inside it:
@@ -840,17 +930,17 @@ struct CardFaceView: View, Equatable {
Divider()
// Group 3 — Navigation ▸ Move Left / Move Right. Wired, unconditionally disabled — see the
// type comment's own section on why.
// Group 3 — Navigation ▸ Move Left / Move Right, a real per-card cross-lane move — see the
// type comment's own section for the destination/position rules and the render-safety proof.
Menu("Navigation") {
Button("Move Left") { moveLane(by: -1) }
.disabled(true)
Button("Move Right") { moveLane(by: 1) }
.disabled(true)
Button("Move Left") { moveCardAcrossLane(by: -1) }
.disabled(!moveLeftEnabled)
Button("Move Right") { moveCardAcrossLane(by: 1) }
.disabled(!moveRightEnabled)
}
// Both rows are unconditionally disabled (see above), so the submenu itself greys out too
// rather than opening onto two dead rows.
.disabled(true)
// The submenu greys out only when *both* rows would — a card in the leftmost lane can still
// move right, so the submenu itself has to stay open to it.
.disabled(!moveLeftEnabled && !moveRightEnabled)
Divider()
@@ -1043,16 +1133,35 @@ struct CardFaceView: View, Equatable {
// MARK: - Navigation (Move Left / Move Right)
/// Navigation ▸ Move Left/Right's action — `MoveLaneCommands.move(by:)`'s own shape, over
/// `LaneMoveTarget.destination`, the exact predicate the menu-bar row validates against. The rows
/// calling this are `.disabled(true)` unconditionally (see the type comment's own section on why),
/// so in practice this never fires from the UI; it stays real rather than a stub so a future pass
/// that relaxes the disable has the correct call already in place.
private func moveLane(by delta: Int) {
/// Navigation ▸ Move Left's render-safe enablement — `hasLeftNeighbor` (this face's own lane
/// against the live board order, lane-hoisted the way `isSelected` is), `acceptsBoardMutations`
/// (a narrow, rarely-changing `BoardStore` flag — every other row here reads it directly, the same
/// proof applies), and the multi-lane-spread refusal, consulted only when this card is itself
/// selected (an unselected clicked card's target is always itself alone, trivially confined to
/// this lane whatever `selectionSpansLanes` says — see the type comment's own section).
private var moveLeftEnabled: Bool {
store.acceptsBoardMutations && hasLeftNeighbor && !(isSelected && selectionSpansLanes)
}
/// `moveLeftEnabled`'s mirror, for Move Right.
private var moveRightEnabled: Bool {
store.acceptsBoardMutations && hasRightNeighbor && !(isSelected && selectionSpansLanes)
}
/// Navigation ▸ Move Left/Right's action — `CardMoveTarget.destination`, the per-card cousin of
/// `LaneMoveTarget.destination` (`MoveLaneCommands.move(by:)`'s own shape one type over), over
/// this face's own widened target (`targetIDs`, Copy/Cut's precedent). `destination` re-derives
/// exactly what `moveLeftEnabled`/`moveRightEnabled` already checked — a card menu names its
/// target by where it was invoked, never by a lingering `.disabled` read, `copyLink()`'s reason —
/// so a stale enablement can only ever make this a no-op, never a wrong move. The write is the
/// ordinary `moveCards`: rank-minting, the undo step, the watcher echo and the banner all come
/// free from that one call, exactly as a released drag's do.
private func moveCardAcrossLane(by delta: Int) {
guard store.acceptsBoardMutations,
let target = LaneMoveTarget.destination(selection: store.selection, snapshot: store.snapshot, delta: delta)
let target = CardMoveTarget.destination(
clicked: card.id, targeting: targetIDs, snapshot: store.snapshot, delta: delta)
else { return }
store.moveLane(target.lane, toIndex: target.index)
store.moveCards(targetIDs, toLane: target.laneID, at: target.index)
}
// MARK: - Title row