diff --git a/CHANGELOG.md b/CHANGELOG.md index 5eac174..d58071c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ **August 2026** +Edit ▸ Undo, ⌘Z, and the toolbar's Undo and Redo buttons now work on board changes — they had been silently disabled. + The board's symbol now appears in the title bar beside the board's name, in its chosen tint. The board's symbol can now wear a color: the symbol picker carries a row of tints below the glyphs, with None to clear it. diff --git a/Kanban/App/BoardWindowHost.swift b/Kanban/App/BoardWindowHost.swift index dc4c0c9..1e36ebc 100644 --- a/Kanban/App/BoardWindowHost.swift +++ b/Kanban/App/BoardWindowHost.swift @@ -236,6 +236,12 @@ struct BoardWindowHost: View { // purge-alert host, which the trash's two confirmed commands raise, and its search // field, which Edit ▸ Find focuses and the caret-chord commands yield to. .focusedSceneValue(\.boardStore, store) + // **What Edit ▸ Undo/Redo cross with this window in front** — the session's stack, which + // those rows now read for themselves (13-native-undo.md ▸ Rules ▸ the command-surface + // bullet, re-ruled 2026-08-08; `UndoCommands.swift`). Read afresh from the model rather + // than stored, for `windowUndoManager`'s reason exactly: a torn-down board answers + // nothing rather than a stack with no board behind it. + .focusedSceneValue(\.undoStack, appModel.session(for: ref)?.undoManager) .focusedSceneValue(\.boardSearch, boardSearch) // The window's identity beside its store — File ▸ Duplicate flushes a *session*, which // is keyed on the window rather than on the board it is showing. @@ -709,11 +715,20 @@ struct BoardWindowHost: View { } } - // This window's answer to "what does ⌘Z act on" (13-native-undo.md ▸ Rules; 06 ▸ Undo - // routing) — the *session's* stack, read afresh on every ask so a torn-down board answers - // nothing rather than a stack with no board behind it. The Edit menu's Undo/Redo rows and - // the toolbar's pair are nil-target `undo:`/`redo:`, so this one line is what lights them - // up: `NSWindow` validates and crosses them against exactly this manager. + // This window's answer to "what does ⌘Z act on" (13-native-undo.md ▸ Rules; ▸ Undo routing) + // — the *session's* stack, read afresh on every ask so a torn-down board answers nothing + // rather than a stack with no board behind it. + // + // **It no longer lights the command surface**, and that sentence used to be this comment's + // whole point (13 ▸ Rules ▸ the command-surface bullet, re-ruled 2026-08-08): the Edit + // menu's rows and the toolbar's pair were nil-target `undo:`/`redo:` resolving through + // `NSWindow.undoManager`, which a SwiftUI window latches empty during creation, before this + // controller's delegate can install — so `windowWillReturnUndoManager` is never consulted + // and the surface validated against a stack nothing registers into. Both faces read the + // session's manager directly now: the rows through `FocusedValues.undoStack` published a few + // lines up in `body`, the toolbar pair through the explicit target `BoardToolbar` is handed + // below. This closure stays wired because it is still the right answer wherever *AppKit* + // asks a delegate for a manager. windowController.windowUndoManager = { appModel.session(for: ref)?.undoManager } // The window-title widget (03-board-ui.md § Board popover) — **board windows only**, which @@ -750,7 +765,11 @@ struct BoardWindowHost: View { search: boardSearch, zoom: appModel.zoom, appearance: appModel.appearance, - session: appModel.dragSession + session: appModel.dragSession, + // The same manager the Edit menu's rows reach through the focus system — one object, two + // faces, which is what "the toolbar mirrors the menu" means for this pair now that + // neither of them goes through the responder chain (13 ▸ Rules, re-ruled 2026-08-08). + undo: appModel.session(for: ref)?.undoManager )) } diff --git a/Kanban/App/CardWindowHost.swift b/Kanban/App/CardWindowHost.swift index 47a6c47..aea71d5 100644 --- a/Kanban/App/CardWindowHost.swift +++ b/Kanban/App/CardWindowHost.swift @@ -362,6 +362,13 @@ struct CardWindowHost: View { // item reaches the frontmost one's body surface through this, exactly as board-window // items reach their window's store (`FocusedBoardStoreKey`). .focusedSceneValue(\.cardBody, bodyPresentation) + // **This window's own stack, published for Edit ▸ Undo/Redo** — the second of 13's two + // levels reaching the menu (13-native-undo.md ▸ Rules ▸ two levels; ▸ the + // command-surface bullet, re-ruled 2026-08-08). One key serves both levels because a + // card window's manager is the same face type as a board session's, so which of them a + // row crosses is decided by which window is in front and by nothing else — no + // fall-through, as a property of the value rather than a rule (`UndoCommands.swift`). + .focusedSceneValue(\.undoStack, session.undo.manager) // View ▸ Raw Source (⌥⌘E) reaches the frontmost card window the same way, and Edit Body // reads it too — "View ▸ Edit Body (⌘E) disables while source mode is active" // (05-card-window.md ▸ Raw source outlet). @@ -878,15 +885,22 @@ struct CardWindowHost: View { // **This window's own stack** (13-native-undo.md ▸ Rules ▸ two levels, re-ruled 2026-07-31 — // superseding the shared-stack wiring): "a card window owns its own stack for the session it - // represents ... and `window.undoManager` answers with it (standard per-window AppKit - // scoping)". ⌘Z with this window in front walks the gestures made *here*, newest first, and - // when they run out it beeps — "no fall-through: exhausting the window's stack ... never - // reaches board history" (06-history-undo.md ▸ Undo routing). What board history gets is the - // one coarse step this session registers when the window closes. + // represents". ⌘Z with this window in front walks the gestures made *here*, newest first, + // and when they run out it beeps — "no fall-through: exhausting the window's stack ... never + // reaches board history" (13 ▸ Undo routing). What board history gets is the one coarse step + // this session registers when the window closes. // - // The card's *text* surfaces are untouched by this — the body editor and the raw-source - // editor each vend their own manager to the responder chain, which is what keeps typing undo - // above either stack (06 ▸ Undo routing, unchanged). + // **The Edit menu does not arrive through here**, which is what the same rule's 2026-08-08 + // re-ruling changed: `window.undoManager` is latched empty by SwiftUI before this + // controller's delegate installs, so the rows read this same manager through + // `FocusedValues.undoStack` instead (published in `body` above; `UndoCommands.swift`). The + // closure stays wired for AppKit's own asks, which is the one caller it always had a right + // to. + // + // The card's *text* surfaces are untouched by either wiring — the body editor and the + // raw-source editor each vend their own manager to the responder chain, and the rows route + // to the first responder's manager while one of them holds the keyboard, which is what keeps + // typing undo above either stack (13 ▸ Undo routing, unchanged). windowController.windowUndoManager = { [session] in session.undo.manager } windowController.onAttach = { window in diff --git a/Kanban/App/UndoCommands.swift b/Kanban/App/UndoCommands.swift new file mode 100644 index 0000000..586bef4 --- /dev/null +++ b/Kanban/App/UndoCommands.swift @@ -0,0 +1,294 @@ +import AppKit +import Observation +import SwiftUI + +// MARK: - The focused stack + +/// **The stack the frontmost window's ⌘Z crosses**, published into the focus system by whichever +/// host owns it: `BoardWindowHost` publishes its session's manager, `CardWindowHost` the window's own +/// (`CardWindowUndo.manager`). +/// +/// One key serves both levels, and that is the two-level model's own shape rather than a shortcut +/// (13-native-undo.md ▸ Rules ▸ two levels, re-ruled 2026-07-31): a card window's stack is the *same* +/// face type over the same seam, so a second key would only ask the rows to decide which of two +/// answers is in front — which is exactly the question the focus system already answers. Crossing +/// between the two levels is therefore impossible by construction here, which is 13 ▸ Undo routing's +/// "no fall-through" holding as a fact about the value rather than as a rule someone applies. +/// +/// `FocusedBoardStoreKey` is the precedent and its note carries over verbatim: `focusedSceneValue` +/// rather than `focusedValue`, because the value is the *window's* and not any particular control's, +/// so it stays available whatever inside the window holds the keyboard — which is precisely what a +/// row that then routes on the first responder needs to read. +struct FocusedUndoStackKey: FocusedValueKey { + typealias Value = BoardUndoManager +} + +extension FocusedValues { + var undoStack: BoardUndoManager? { + get { self[FocusedUndoStackKey.self] } + set { self[FocusedUndoStackKey.self] = newValue } + } +} + +// MARK: - Routing + +/// **What an Undo/Redo row answers with**, given the focused stack and what holds the keyboard — +/// 13-native-undo.md ▸ Undo routing's predicate, relocated from the window delegate to the command +/// layer (13 ▸ Rules ▸ the command-surface bullet, re-ruled 2026-08-08; the rule itself is +/// unchanged, only where it is enacted). +/// +/// ### Why the rule had to move +/// +/// The predicate used to be the platform's to apply: the system's nil-target `undo:`/`redo:` rows +/// resolve to `NSWindow`, which reads the manager its delegate hands back +/// (`windowWillReturnUndoManager`, still wired in `HostedWindowController` and still the right answer +/// wherever AppKit itself asks). On a SwiftUI window that hook is never consulted — `NSWindow` reads +/// and **permanently latches** an undo manager of its own during window creation, before any app code +/// can install a delegate (diagnosed 2026-08-07 by live probe) — so the whole command surface became +/// the app's own, and the routing came with it. +/// +/// ### Pure, and free of both windows and SwiftUI +/// +/// Everything a row needs is two references it is handed. The first responder is the one part a test +/// cannot conjure from the focus system, so it is a parameter rather than a read of `NSApp` — the +/// same posture `BoardUndoRouting` takes one layer down, and the reason that enum's decision is +/// reusable here verbatim rather than restated. +@MainActor +enum UndoCommandRouting { + + /// The manager a row answers with — the focused stack, or, while a text surface holds the + /// keyboard, that responder's own manager. + /// + /// The text case is where its typing undo actually registered: an editor that vends a manager + /// through its delegate (`CardBodySurface`, `CardRawSourceView`) hands back that one, and a field + /// editor — which vends none — resolves up the responder chain to the window's latched manager, + /// which is exactly where AppKit put its typing undo. Either way the answer is the responder's, + /// which is what "a reflexive undo over a typo must never become a board-level restore" + /// (13 ▸ Undo routing) means once the app is the one choosing. + /// + /// `nil` where there is nothing to cross at all: no stack in front and no text surface focused, or + /// a text surface whose own manager is `nil` (a view in no window, with no delegate to ask). A row + /// over `nil` disables and keeps the bare verb. + static func routedManager(stack: BoardUndoManager?, firstResponder: NSResponder?) -> UndoManager? { + guard BoardUndoRouting.isTextEditing(firstResponder) else { return stack } + guard let text = firstResponder?.undoManager else { return nil } + return BoardUndoRouting.undoManager(isTextEditing: true, board: stack, textFallback: text) + } + + // MARK: Titles + + /// "Undo Move 3 Cards" — composed and localized by the platform over the bare 13-vocabulary + /// phrase, so the app never spells the verb (`BoardUndoManager.undoMenuItemTitle`). + /// + /// The trim is for the nameless case on a manager that is *not* this app's adapter: a plain + /// `NSUndoManager` — a text view's, a field editor's — composes `Undo %@` over an empty name and + /// leaves the trailing space standing. The bare verb is also the answer for no manager at all, + /// which is a row with nothing in front of it rather than a row that lost its name. + static func undoTitle(of manager: UndoManager?) -> String { + title(manager?.undoMenuItemTitle, bare: "Undo") + } + + static func redoTitle(of manager: UndoManager?) -> String { + title(manager?.redoMenuItemTitle, bare: "Redo") + } + + private static func title(_ composed: String?, bare: String) -> String { + let trimmed = composed?.trimmingCharacters(in: .whitespaces) ?? "" + return trimmed.isEmpty ? bare : trimmed + } + + // MARK: Enablement + + /// Whether the row is live — the routed manager's own answer, which for this app's adapter is + /// "steps, and no read-only lock" (`BoardUndoManager.canUndo`, where 13 ▸ Rules' lock clause + /// lives) and for a text manager is the editor's session. `false` with no manager, which is the + /// same disablement an empty stack gets: exhausting what the row reaches never reaches anything + /// else (13 ▸ Undo routing, no fall-through). + static func canUndo(_ manager: UndoManager?) -> Bool { + manager?.canUndo == true + } + + static func canRedo(_ manager: UndoManager?) -> Bool { + manager?.canRedo == true + } +} + +// MARK: - The revalidation ticker + +/// **What makes a rendered Undo/Redo row notice the world moved.** +/// +/// The rows read three things that are not SwiftUI state: the routed manager's enablement, its +/// composed title, and — through the first responder — which of the two it is routing to at all. +/// `BoardUndoManager` re-derives from an `@Observable` provider, so a board or card step lands on the +/// row for free; a *text* manager is a plain `NSUndoManager` with no observation in it, and neither +/// is `NSApp.keyWindow?.firstResponder`. This is the subscription those two get. +/// +/// A bumped counter rather than published state: nothing here knows what any row's answer is, only +/// that it may have changed, and the rows are cheap to re-derive. Reading `revision` inside a row's +/// `body` is what enrolls it. +/// +/// ### The notifications, and why each one +/// +/// `NSUndoManagerCheckpoint` with `object: nil` is the load-bearing one: it catches **every** +/// manager's stack changes, which is how typing into a field editor retitles the row that is routing +/// to it. The did-undo/did-redo pair covers a crossing that changes which direction is live, the +/// `NSText` editing pair covers the field editor arriving and leaving, and `NSMenu`'s did-begin- +/// tracking is a re-derive a moment before the Edit menu draws itself. +/// +/// ### The known residual, which is cosmetic +/// +/// Focus moving in or out of a text surface *without any typing* — clicking into an empty search +/// field, tabbing away from an untouched one — bumps nothing, because AppKit posts no editing +/// notification until an edit begins. A row rendered across that moment can therefore be titled or +/// enabled off the stack it is no longer routing to. Behaviour stays correct regardless: the action +/// re-derives the routed manager at fire time and beeps rather than crossing the wrong stack, which +/// is 13 ▸ Undo routing's own answer for an exhausted focus. +@MainActor +@Observable +final class UndoCommandTicker { + + /// One instance, read by both rows: the rows differ in direction, never in when they are stale. + static let shared = UndoCommandTicker() + + /// Bumped, never read for its value — a row reads it to subscribe, and the number itself means + /// nothing. + private(set) var revision = 0 + + private var observers: [any NSObjectProtocol] = [] + + private init() { + let names: [Notification.Name] = [ + .NSUndoManagerCheckpoint, + .NSUndoManagerDidUndoChange, + .NSUndoManagerDidRedoChange, + NSText.didBeginEditingNotification, + NSText.didEndEditingNotification, + NSMenu.didBeginTrackingNotification, + ] + observers = names.map { name in + NotificationCenter.default.addObserver(forName: name, object: nil, queue: nil) { [weak self] _ in + // All six are posted on the main thread, and the synchronous path is the one that + // matters: a menu that has begun tracking is about to draw, so a hop onto the next + // turn would re-derive the row after the user is already looking at it. The + // asynchronous branch is the promise-keeping half, never the expected one. + if Thread.isMainThread { + MainActor.assumeIsolated { self?.bump() } + } else { + Task { @MainActor in self?.bump() } + } + } + } + } + + private func bump() { + revision &+= 1 + } +} + +// MARK: - The rows + +/// **Edit ▸ Undo and Edit ▸ Redo, as the app's own rows** (13-native-undo.md ▸ Rules ▸ the +/// command-surface bullet, ruled 2026-08-08). +/// +/// `CommandGroup(replacing: .undoRedo)` takes the system's nil-target pair out of the menu, because +/// on a SwiftUI window they are unreachable: `NSWindow` latches an empty undo manager of its own +/// during window creation, before `HostedWindowController` can install the delegate that would have +/// vended the board's, so `windowWillReturnUndoManager` is never consulted and the rows validate +/// against a stack nothing ever registers into. Everything the pair used to get from the platform — +/// enablement, the dynamic title, the crossing — the rows below now ask for by name, off the same +/// `BoardUndoManager` that surface always meant to be reading. +/// +/// The alternatives are recorded in 13 and both re-open decisions this app already made: registering +/// steps into the latched manager needs an `NSUndoManager` substrate, which `HistoryStepOutcome +/// .failed` rules out (`NativeHistoryProvider` ▸ two arrays), and intercepting nil-target `undo:` +/// from the responder chain is preempted by `NSWindow` handling the action itself, ahead of its own +/// delegate. +struct UndoRedoCommands: Commands { + + var body: some Commands { + CommandGroup(replacing: .undoRedo) { + UndoMenuRow() + RedoMenuRow() + } + } +} + +/// Edit ▸ Undo (⌘Z). +/// +/// **The whole row routes, not just its action** (13 ▸ Rules ▸ the command-surface bullet, re-ruled +/// 2026-08-08). A row left titled and enabled off the board stack while a text field held the +/// keyboard would fail in one of two ways, and both are worse than a re-derived title: enabled, it +/// advertises a board step it will not cross once the action routes elsewhere; disabled — because +/// the board stack happened to be empty — it *swallows* ⌘Z outright, since a menu item owns its key +/// equivalent whether or not it is live and nothing downstream ever sees the chord. Routing title, +/// enablement and action together is what keeps 13 ▸ Undo routing's no-fall-through honest from the +/// user's side of the menu. +private struct UndoMenuRow: View { + + @FocusedValue(\.undoStack) private var stack + + var body: some View { + // Read for the subscription, never for the value — see `UndoCommandTicker`, which is what + // makes a text manager's stack changes and a focus move reach a row that has already + // rendered. It has to be read *in* `body`, which is the only scope SwiftUI tracks. + let _ = UndoCommandTicker.shared.revision + let manager = UndoCommandRouting.routedManager( + stack: stack, + firstResponder: NSApp.keyWindow?.firstResponder + ) + Button(UndoCommandRouting.undoTitle(of: manager)) { + cross() + } + .keyboardShortcut("z", modifiers: .command) + .disabled(!UndoCommandRouting.canUndo(manager)) + } + + /// **Routed again here**, rather than closing over what the row rendered with: focus and stacks + /// both move between a menu's display and its click, and the manager the crossing reaches must be + /// the one that holds the keyboard *now*. The beep is 13 ▸ Undo routing's "exhausting a focused + /// editor's stack beeps; it never reaches board history", answering the window where the row's + /// rendered enablement was a moment stale. + private func cross() { + let manager = UndoCommandRouting.routedManager( + stack: stack, + firstResponder: NSApp.keyWindow?.firstResponder + ) + guard let manager, manager.canUndo else { + NSSound.beep() + return + } + manager.undo() + } +} + +/// Edit ▸ Redo (⇧⌘Z) — `UndoMenuRow`'s twin in every respect, which is why its reasoning is not +/// repeated here. +private struct RedoMenuRow: View { + + @FocusedValue(\.undoStack) private var stack + + var body: some View { + let _ = UndoCommandTicker.shared.revision + let manager = UndoCommandRouting.routedManager( + stack: stack, + firstResponder: NSApp.keyWindow?.firstResponder + ) + Button(UndoCommandRouting.redoTitle(of: manager)) { + cross() + } + .keyboardShortcut("z", modifiers: [.command, .shift]) + .disabled(!UndoCommandRouting.canRedo(manager)) + } + + private func cross() { + let manager = UndoCommandRouting.routedManager( + stack: stack, + firstResponder: NSApp.keyWindow?.firstResponder + ) + guard let manager, manager.canRedo else { + NSSound.beep() + return + } + manager.redo() + } +} diff --git a/Kanban/App/WindowToolbar.swift b/Kanban/App/WindowToolbar.swift index 9cf2001..2da47bf 100644 --- a/Kanban/App/WindowToolbar.swift +++ b/Kanban/App/WindowToolbar.swift @@ -64,10 +64,6 @@ struct ToolbarItemSpec { case button(isEnabled: () -> Bool, perform: () -> Void) /// A toggle showing on-state — Show Trash, Edit Body, Raw Source (03 ▸ Toolbar). case toggle(isEnabled: () -> Bool, isOn: () -> Bool, setOn: (Bool) -> Void) - /// An action sent up the responder chain with no target of our own — **Undo and Redo**, which - /// is how their menu rows work too, so "matching their menu items" is one mechanism rather - /// than two (03 ▸ Toolbar; 06-history-undo.md). - case responderAction(Selector) /// A search field in AppKit's own `NSSearchToolbarItem` — the board's search /// (03-board-ui.md ▸ Toolbar). The item owns the field's layout, so `focusedWidth` is a /// preference rather than a constraint: it is the width the field takes *when it has the @@ -88,8 +84,8 @@ struct ToolbarItemSpec { /// `selected()` names the option index carrying the checkmark, read fresh whenever AppKit /// opens the menu rather than polled — the same freshness every other menu row in the app /// gets (`validateMenuItem(_:)`) — and `select(_:)` is a chosen row's whole action. The one - /// behavior with no `activate()` of its own: firing lives in the dropdown's rows, not in the - /// item itself, the way `responderAction`'s lives in the responder chain rather than here. + /// behavior with no `activate()` of its own: firing lives in the dropdown's rows rather than + /// in the item's primary action. case picker( options: [(title: String, symbol: String?)], selected: () -> Int?, @@ -126,14 +122,14 @@ struct ToolbarItemSpec { // MARK: State - /// The item's live enablement. Responder-chain items and the search item answer `true`: the - /// first is validated by the chain itself (which is the point of it), and the second has no - /// enablement of its own. + /// The item's live enablement. The search item and the picker answer `true`: neither has an + /// enablement of its own — the field is a control rather than a command, and the picker's + /// choices are always available. var isEnabled: Bool { switch behavior { case let .button(isEnabled, _): isEnabled() case let .toggle(isEnabled, _, _): isEnabled() - case .responderAction, .searchField, .picker: true + case .searchField, .picker: true } } @@ -141,7 +137,7 @@ struct ToolbarItemSpec { var isOn: Bool? { switch behavior { case let .toggle(_, isOn, _): isOn() - case .button, .responderAction, .searchField, .picker: nil + case .button, .searchField, .picker: nil } } @@ -152,7 +148,7 @@ struct ToolbarItemSpec { switch behavior { case let .button(_, perform): perform() case let .toggle(_, isOn, setOn): setOn(!isOn()) - case .responderAction, .searchField, .picker: break + case .searchField, .picker: break } } } @@ -165,7 +161,9 @@ struct ToolbarItemSpec { /// ### Why `NSToolbar` rather than SwiftUI's `.toolbar(id:)` /// /// SwiftUI's customizable toolbar would answer most of 03's clauses, and it was the first choice. -/// Four requirements sent this to AppKit instead, and each is normative rather than aesthetic: +/// Four requirements sent this to AppKit instead, and each was normative rather than aesthetic. Three +/// of them still hold; the fourth is kept below because a retired reason is worth more written down +/// than deleted, and because the three that remain are what the answer now rests on: /// /// - **⌘F has to know whether the search item is installed.** "With the field removed from the /// toolbar, invoking it surfaces the field transiently" (03) — a decision that needs to read the @@ -173,12 +171,16 @@ struct ToolbarItemSpec { /// delegate callbacks); SwiftUI's toolbar API has no such query, and reaching around it into the /// `NSToolbar` it happens to own means matching identifiers SwiftUI derives rather than ones this /// app spells. -/// - **Undo and Redo have to reach the responder chain.** Their menu rows are the system's own — -/// nil-target `undo:`/`redo:` — and "matching their menu items" (03) is literal here: a toolbar -/// item with the same nil-target action validates and fires through exactly the same lookup, so -/// the pair enables and disables with the menu rows by construction rather than by agreement, -/// reading the board window's `BoardUndoManager` through `NSWindow`'s own validation -/// (13-native-undo.md). A SwiftUI `Button` cannot express that. +/// - ~~**Undo and Redo have to reach the responder chain.**~~ **Retired 2026-08-08** with the +/// mechanism it named (13-native-undo.md ▸ Rules ▸ the command-surface bullet, re-ruled that day): +/// the pair's nil-target `undo:`/`redo:` resolved to `NSWindow`, which reads the manager its +/// delegate vends — and a SwiftUI window latches an empty manager of its own during creation, +/// before `HostedWindowController` installs, so that route reached a stack nothing registers into. +/// The pair now carries an **explicit target** over the focused session's `BoardUndoManager` +/// (`BoardToolbar`), which is the same object the app's own Edit ▸ Undo/Redo rows read through the +/// focus system — "matching their menu items" (03) by sharing the manager rather than by sharing a +/// lookup. Nothing about that needs AppKit; the other three bullets are why this is still an +/// `NSToolbar`. /// - **The search item is AppKit's own `NSSearchToolbarItem`**, hosting a real `NSSearchField` with /// explicit first-responder control, settled in m5 for reasons `BoardSearchFieldController` /// records (⌘F must focus it from a menu item; Escape in an empty field must hand the keyboard @@ -304,8 +306,6 @@ final class WindowToolbarController: NSObject, NSToolbarDelegate { return makeButtonItem(spec) case .toggle: return makeToggleItem(spec) - case let .responderAction(selector): - return makeResponderItem(spec, selector: selector) case let .searchField(focusedWidth, make, install): return makeSearchItem( spec, @@ -333,19 +333,6 @@ final class WindowToolbarController: NSObject, NSToolbarDelegate { return item } - /// **Undo and Redo**: no target, so AppKit resolves and validates the action up the responder - /// chain — the same lookup their menu rows use, which is the whole of "matching their menu - /// items" (03 ▸ Toolbar). Deliberately *not* a `ValidatingToolbarItem`: the default validation - /// is precisely the behaviour wanted here. - private func makeResponderItem(_ spec: ToolbarItemSpec, selector: Selector) -> NSToolbarItem { - let item = NSToolbarItem(itemIdentifier: spec.identifier) - decorate(item, with: spec) - item.isBordered = true - item.target = nil - item.action = selector - return item - } - /// A toggle button showing on-state. A hosted `NSButton` rather than a plain item because /// `NSToolbarItem` has no state of its own, and 03 asks for one explicitly ("Show Trash (toggle /// state matching the View menu checkmark)", "Edit Body is a single toggle button (on-state in diff --git a/Kanban/History/BoardUndoManager.swift b/Kanban/History/BoardUndoManager.swift index 23e4387..aedab39 100644 --- a/Kanban/History/BoardUndoManager.swift +++ b/Kanban/History/BoardUndoManager.swift @@ -2,16 +2,25 @@ import AppKit // MARK: - BoardUndoManager -/// What AppKit is handed for a board's windows: an `UndoManager` that owns no stack and answers -/// every question from the session's `HistoryProviding`. +/// The face a board's undo wears everywhere it is asked about: an `UndoManager` that owns no stack +/// and answers every question from the session's `HistoryProviding`. /// /// ### Why an adapter exists at all /// -/// The responder chain speaks one currency. `NSWindow` implements `undo:`/`redo:` and validates -/// those menu items itself, reading `canUndo`, `canRedo` and `undoMenuItemTitle` off whatever -/// `UndoManager` the window's delegate hands back (`windowWillReturnUndoManager(_:)`) — which is -/// exactly how the system's Edit ▸ Undo row and the toolbar's nil-target pair (`BoardToolbar`) light -/// up, disable and retitle with no code of the app's own. +/// The command surface speaks one currency. Enablement, the composed menu title, and the crossing +/// itself are all `UndoManager` questions — `canUndo`, `undoMenuItemTitle`, `undo()` — and every +/// surface that offers ⌘Z asks them of *something*: the app's own Edit ▸ Undo/Redo rows read this +/// object through `FocusedValues.undoStack` (`UndoCommands.swift`), the board toolbar's pair carries +/// it as an explicit target (`BoardToolbar`), and AppKit itself still asks the window's delegate for +/// it (`HostedWindowController.windowWillReturnUndoManager`, wired and correct wherever the platform +/// is the one asking). One object answers all of them, which is what keeps them from disagreeing. +/// +/// (Those first two used to be the same fact as the third: the rows and the toolbar pair were the +/// system's nil-target `undo:`/`redo:`, validated by `NSWindow` against whatever its delegate vended. +/// That route is unreachable on a SwiftUI window — the window latches an empty manager of its own +/// during creation, before any delegate of ours installs — so the surface became the app's own, +/// reading this object directly: 13-native-undo.md ▸ Rules ▸ the command-surface bullet, re-ruled +/// 2026-08-08. What it reads did not change, only how it gets here.) /// /// The seam, though, must not be an `NSUndoManager`: a protocol that vended one could only ever have /// had a single implementation, and the substrate stays behind `HistoryProviding` so a future @@ -50,9 +59,10 @@ import AppKit /// "Every read-only lock (vanished root, failed reload after wholesale ops, unwritable location) /// disables Undo/Redo with the other mutating commands; **the stack itself survives the lock and /// resumes when it clears**" (13 ▸ Rules). This is the right place for it and the only one: every -/// surface that offers ⌘Z — the Edit menu's nil-target row, the toolbar pair, a card window's -/// responder chain — validates through this object, so answering `false` here disables all of them -/// at once, exactly as the lock's other victims disable through menu validation (02-architecture.md +/// surface that offers ⌘Z — the Edit menu's own rows over the focused stack, the board toolbar's +/// pair over the session's, a card window's over its own — validates through this object, so +/// answering `false` here disables all of them at once, exactly as the lock's other victims disable +/// through menu validation (02-architecture.md /// § "The lock's scope"). Putting it in the *provider* would have been the same answer in the wrong /// place: the stack is not the thing that is locked, the board is, and whatever provider a session /// binds — a future one included — must inherit the rule without reimplementing it. diff --git a/Kanban/History/HistoryProviding.swift b/Kanban/History/HistoryProviding.swift index 46b796e..739ce89 100644 --- a/Kanban/History/HistoryProviding.swift +++ b/Kanban/History/HistoryProviding.swift @@ -279,8 +279,10 @@ public struct HistoryStep { /// the seam is real", 12 — written when native undo was the free tier's; the proof it named was /// two working substrates, which the pivot left standing and the 2026-08-08 excision then narrowed /// back to one, `strategy/01-git-excision.md` — the seam itself is what stays proved either way). -/// AppKit still needs an `UndoManager` to hand the responder chain; that adapter is -/// `BoardUndoManager`, which sits *over* this protocol rather than inside it. +/// The command surface still speaks `UndoManager` — the menu rows, the toolbar pair and AppKit's +/// own asks all read one (13-native-undo.md ▸ Rules ▸ the command-surface bullet, re-ruled +/// 2026-08-08); that adapter is `BoardUndoManager`, which sits *over* this protocol rather than +/// inside it. /// - **No persistence promise.** The native stack dies with the session (13); a git provider's once /// survived relaunch because git does, before app-managed git was excised entirely (2026-08-08, /// `strategy/01-git-excision.md`). Both were honest implementations of these seven members; only @@ -326,7 +328,9 @@ public protocol HistoryProviding: AnyObject { func register(_ step: HistoryStep) /// Whether there is a step to cross. What the Edit menu's Undo row and the toolbar's Undo item - /// enable on, through the same responder-chain answer. + /// enable on — both of them through one `BoardUndoManager` over this seam, which is what keeps + /// the two surfaces from being two answers (13-native-undo.md ▸ Rules ▸ the command-surface + /// bullet, re-ruled 2026-08-08). var canUndo: Bool { get } var canRedo: Bool { get } diff --git a/Kanban/History/NativeHistoryProvider.swift b/Kanban/History/NativeHistoryProvider.swift index d0a0715..1ee5e3d 100644 --- a/Kanban/History/NativeHistoryProvider.swift +++ b/Kanban/History/NativeHistoryProvider.swift @@ -1,4 +1,5 @@ import Foundation +import Observation // MARK: - NativeHistoryProvider @@ -13,13 +14,16 @@ import Foundation /// ### Two arrays, and why not `NSUndoManager` /// /// This provider was an `NSUndoManager` for exactly one milestone, on the argument that the *command -/// surface* is the platform's — Edit ▸ Undo and Edit ▸ Redo are the system's own nil-target -/// `undo:`/`redo:` rows (11-command-nexus.md), the toolbar pair carries the same actions -/// (`BoardToolbar`), and both light up, disable and **retitle** from whatever `UndoManager` the -/// focused window hands back. All of that is still true, and none of it lives here: the retitling is -/// `BoardUndoManager.undoMenuItemTitle` composing `undoMenuTitle(forUndoActionName:)` over the bare -/// phrase this seam vends as a `String?`. The adapter is the `UndoManager`; the substrate never -/// needed to be one. +/// surface* is the platform's — Edit ▸ Undo and Edit ▸ Redo were the system's own nil-target +/// `undo:`/`redo:` rows (11-command-nexus.md), the toolbar pair carried the same actions +/// (`BoardToolbar`), and both lit up, disabled and **retitled** from whatever `UndoManager` the +/// focused window handed back. The surface is the app's own now — replaced rows and explicit toolbar +/// targets, reading the focused `BoardUndoManager` through `FocusedValues` (13-native-undo.md ▸ Rules +/// ▸ the command-surface bullet, re-ruled 2026-08-08, the SwiftUI latch) — and the part of that +/// argument this file rests on is untouched either way: none of it lives here, and the *vocabulary* +/// is still the platform's, `BoardUndoManager.undoMenuItemTitle` composing +/// `undoMenuTitle(forUndoActionName:)` over the bare phrase this seam vends as a `String?`. The +/// adapter is the `UndoManager`; the substrate never needed to be one. /// /// What forced the change is the staleness milestone's third outcome. `HistoryStepOutcome.failed` /// means **the step stays put** — a disk error is retryable, so ⌘Z must still be able to reach the @@ -51,7 +55,21 @@ import Foundation /// it until the session ends (`strandedSteps`; 13 ▸ Interaction with the trash, ruled 2026-07-31 — /// the skip-purge decoupling). Nothing can cross a stranded step and nothing can see one in the Edit /// menu; it is a hold outliving the history it belonged to. +/// +/// ### Observable, because the command surface is the app's own now +/// +/// The two stacks below back every `canUndo`, `canRedo` and `undoActionName` any surface reads, and +/// since 2026-08-08 those surfaces are SwiftUI's and the toolbar's rather than `NSWindow`'s own +/// validation (13-native-undo.md ▸ Rules ▸ the command-surface bullet, re-ruled that day: the Edit +/// menu's rows read the focused `BoardUndoManager` through `FocusedValues`, and the toolbar pair +/// carries explicit targets over the same object). Both want +/// to re-derive when a step lands rather than when the user next moves the mouse, and both already +/// know how: the menu rows are views, and `WindowToolbarController.trackValidationState` re-arms +/// `withObservationTracking` over every predicate it was given. Making the *stacks* observed is what +/// lets that machinery reach through the adapter — one macro here, and no notification of anyone's +/// own anywhere above. @MainActor +@Observable public final class NativeHistoryProvider: HistoryProviding { /// The two stacks, top last. Both hold steps oriented for crossing — see the type's note. diff --git a/Kanban/KanbanApp.swift b/Kanban/KanbanApp.swift index 8549f1a..cb7f488 100644 --- a/Kanban/KanbanApp.swift +++ b/Kanban/KanbanApp.swift @@ -221,11 +221,18 @@ struct KanbanApp: App { TrashCommands() } + // Edit ▸ Undo/Redo, replacing the system's own pair — **the command surface is the app's** + // (13-native-undo.md ▸ Rules, re-ruled 2026-08-08): the platform's nil-target rows resolve + // through `NSWindow.undoManager`, which a SwiftUI window latches empty before any delegate of + // ours can vend the board's, so the rows below read the focused session's stack themselves + // (`UndoCommands.swift`). + UndoRedoCommands() + // The Edit menu: Find (⌘F), then its card-window stepping twins, placed after the standard - // Cut/Copy/Paste/Select All group, which is where macOS puts Find. Undo/Redo and the - // clipboard items are the system's and the board answers them as a responder - // (`ClipboardCommands.swift`) — a second item sharing one of those titles is what - // titles-are-API forbids. + // Cut/Copy/Paste/Select All group, which is where macOS puts Find. The clipboard items above + // them stay the system's, answered by the board as a responder (`ClipboardCommands.swift`) — + // a second item sharing one of those titles is what titles-are-API forbids. Undo and Redo + // were the system's too until the latch (the group just above). CommandGroup(after: .pasteboard) { FindCommand() FindSteppingCommands() diff --git a/Kanban/UI/Board/BoardToolbar.swift b/Kanban/UI/Board/BoardToolbar.swift index 2ac836d..ed4c7f9 100644 --- a/Kanban/UI/Board/BoardToolbar.swift +++ b/Kanban/UI/Board/BoardToolbar.swift @@ -34,23 +34,37 @@ extension NSToolbarItem.Identifier { /// "The board popover deliberately has **no toolbar item** — the window-title widget is its /// committed home" — so there is no Board Info entry here, and its absence is pinned by a test. /// -/// ### Undo and Redo are the responder chain's, exactly as the menu's are +/// ### Undo and Redo carry the session's own manager, exactly as the menu's rows do /// -/// The app ships no Undo/Redo rows of its own: those are the standard Edit-menu items, nil-target -/// `undo:`/`redo:` resolved up the responder chain (`KanbanApp.menuCommands`). The toolbar items -/// carry the same actions with the same nil target, so "matching their menu items" (03) is not a -/// predicate written here — it is literally the same validation. Both reach the board window, whose -/// `windowWillReturnUndoManager` hands back the session's `BoardUndoManager`, and both therefore -/// enable exactly when that board has a step to cross and no read-only lock stands -/// (13-native-undo.md ▸ Rules). **Every board has undo**, so there is no substrate-shaped -/// disablement to write: 03's parenthetical about boards without undo was 06's *git* substrate, -/// which a board once bound by being in git mode — on any tier since 12-editions.md ▸ PIVOT -/// 2026-08-07, and under a Pro subscription before it, until app-managed git left the app entirely -/// (`strategy/01-git-excision.md`, 2026-08-08) and there stopped being a substrate to bind. +/// **The command surface is the app's** (13-native-undo.md ▸ Rules ▸ the command-surface bullet, +/// re-ruled 2026-08-08): Edit ▸ Undo/Redo are the app's own rows reading the focused session's +/// `BoardUndoManager` through the focus system (`UndoCommands.swift`), and the two items below carry +/// an explicit target over **that same manager** — the session's, handed in by the window's host +/// (`BoardWindowHost.configureWindow`). "Matching their menu items" (03) is therefore one *object* +/// rather than one lookup: enablement is `canUndo`/`canRedo` on both surfaces, so a step landing, an +/// empty stack, and the read-only lock reach the pair and the rows together and cannot disagree +/// (`BoardUndoManager` answers all three, and is the only place any of them is decided). /// -/// Their labels are the design's one exception to the menu-title rule: `NSUndoManager` rewrites the -/// *menu* titles as the stack changes ("Undo Move Card"), which a toolbar label does not track, so -/// these two are built from static labels (`ToolbarItemSpec.staticLabel`). +/// The pair was nil-target `undo:`/`redo:` until that re-ruling, resolving up the responder chain to +/// the board window's `windowWillReturnUndoManager` — a route a SwiftUI window makes unreachable by +/// latching an empty undo manager of its own before any delegate of ours installs (diagnosed +/// 2026-08-07). Nothing about the *design* changed: the same manager, the same predicates, reached +/// by being handed it instead of by looking it up. +/// +/// **Every board has undo**, so there is no substrate-shaped disablement to write: 03's +/// parenthetical about boards without undo was the *git* substrate, which a board once bound by being +/// in git mode — on any tier since 12-editions.md ▸ PIVOT 2026-08-07, and under a Pro subscription +/// before it, until app-managed git left the app entirely (`strategy/01-git-excision.md`, +/// 2026-08-08) and there stopped being a substrate to bind. +/// +/// **Toolbar clicks are never text-routed.** The menu rows route ⌘Z to the first responder's own +/// manager while a text surface holds the keyboard (13 ▸ Undo routing); clicking a toolbar button is +/// not a keystroke aimed at an editor, so these two act on the board stack unconditionally — which is +/// also the only stack the item can see. +/// +/// Their labels are the design's one exception to the menu-title rule: the *menu* titles are rewritten +/// as the stack changes ("Undo Move Card"), which a toolbar label does not track, so these two are +/// built from static labels (`ToolbarItemSpec.staticLabel`). @MainActor enum BoardToolbar { @@ -78,12 +92,17 @@ enum BoardToolbar { /// and `@Observable` so the picker's checkmark, read when its menu opens, is never stale. /// - session: the app's drag session, for the same guard the menu rows carry /// (`ZoomCommands.isEnabled`). + /// - undo: **the board session's stack**, for the Undo/Redo pair — the same `BoardUndoManager` + /// the Edit menu's rows read through the focus system (13-native-undo.md ▸ Rules ▸ the + /// command-surface bullet, re-ruled 2026-08-08). `nil` for a window whose session has gone, + /// which the pair reads as an empty stack: disabled, and crossing nothing. static func specs( store: BoardStore, search: BoardSearchPresentation, zoom: BoardZoomStore, appearance: AppearanceStore, - session: DragSession + session: DragSession, + undo: BoardUndoManager? ) -> [ToolbarItemSpec] { [ .mirroring( @@ -132,17 +151,28 @@ enum BoardToolbar { perform: { [weak zoom] in zoom?.step(.out) } ) ), + // The one pair whose predicate is not a menu row's *expression* but the menu row's own + // object: `canUndo`/`canRedo` on the session's manager, which is where the empty stack + // and the read-only lock are both already decided (`BoardUndoManager`). Weak like every + // other capture here — production hands these app-lived objects, and a spec must not be + // what keeps a torn-down session's stack alive. .staticLabel( "Undo", identifier: .boardUndo, symbol: "arrow.uturn.backward", - behavior: .responderAction(NSSelectorFromString("undo:")) + behavior: .button( + isEnabled: { [weak undo] in undo?.canUndo == true }, + perform: { [weak undo] in undo?.undo() } + ) ), .staticLabel( "Redo", identifier: .boardRedo, symbol: "arrow.uturn.forward", - behavior: .responderAction(NSSelectorFromString("redo:")) + behavior: .button( + isEnabled: { [weak undo] in undo?.canRedo == true }, + perform: { [weak undo] in undo?.redo() } + ) ), .mirroring( menuTitle: "Show Trash", @@ -219,11 +249,19 @@ enum BoardToolbar { search: BoardSearchPresentation, zoom: BoardZoomStore, appearance: AppearanceStore, - session: DragSession + session: DragSession, + undo: BoardUndoManager? ) -> WindowToolbarController { let controller = WindowToolbarController( identifier: identifier, - specs: specs(store: store, search: search, zoom: zoom, appearance: appearance, session: session), + specs: specs( + store: store, + search: search, + zoom: zoom, + appearance: appearance, + session: session, + undo: undo + ), defaults: defaultItems ) // Centered against the window, not a flexible-space sandwich (03 ▸ Toolbar's placement diff --git a/KanbanTests/HistoryProviderTests.swift b/KanbanTests/HistoryProviderTests.swift index c2832ab..d4a59c3 100644 --- a/KanbanTests/HistoryProviderTests.swift +++ b/KanbanTests/HistoryProviderTests.swift @@ -600,28 +600,32 @@ struct BoardSessionHistoryTests { // MARK: - The command surface -/// **What the Edit menu's Undo/Redo rows and the toolbar's pair actually do** — driven through the -/// platform machinery they ride on rather than described (11-command-nexus.md ▸ Menu commands, the -/// M− row; 03-board-ui.md ▸ Toolbar; 13-native-undo.md ▸ Rules). +/// **What a window hands AppKit, and what the board toolbar's Undo/Redo pair reaches** — the two +/// halves of the command surface that are still platform-shaped, driven through the machinery they +/// ride on rather than described (03-board-ui.md ▸ Toolbar; 13-native-undo.md ▸ Rules). /// -/// ### The app writes none of this, which is exactly why it is tested +/// ### What this suite covers since the surface became the app's own /// -/// There is no custom Undo/Redo menu code anywhere: the rows are the system's own nil-target -/// `undo:`/`redo:`, and the toolbar's two items carry the same selectors with the same nil target -/// (`BoardToolbar`). Every claim the design makes about them — they enable on a stack with steps, -/// they dim under the read-only lock, the *menu* rows retitle themselves to "Undo Move 3 Cards" -/// while the *toolbar* labels stay static — is therefore a claim about `NSWindow`'s own validation -/// reading the manager this app's window delegate hands back. Nothing here would fail loudly if the -/// wiring came undone; it would just quietly stop working, which is what these tests are for. +/// **The rows are no longer here.** Edit ▸ Undo/Redo are `CommandGroup(replacing: .undoRedo)` rows +/// the app writes and routes itself (13 ▸ Rules ▸ the command-surface bullet, re-ruled 2026-08-08; +/// `UndoCommands.swift`, pinned by `UndoCommandsTests.swift`), because the nil-target route they used +/// to ride is unreachable on a SwiftUI window: `NSWindow` latches an empty undo manager during +/// creation, before `HostedWindowController` installs, so `windowWillReturnUndoManager` is never +/// consulted for them. These tests never reproduced that — they attach the delegate before the +/// window's first read, which is exactly the ordering a real SwiftUI window denies — and that is the +/// diagnosis, not a gap to close: the hook works when it is asked, and the app stopped depending on +/// it being asked. /// -/// ### What a headless run can and cannot reach +/// What is pinned below is therefore what remains true and load-bearing: /// -/// `NSWindow.validateMenuItem(_:)` and `NSWindow.validateUserInterfaceItem(_:)` are the two methods -/// AppKit calls once a nil-target lookup has resolved to the window, and both answer fully in a test -/// process — which is the half this app owns and the half that can break. The lookup *itself* -/// (`NSApp.target(forAction:to:from:)`) needs a **key window**, and a unit-test host has none, so -/// "the board window is what the chain resolves to when it is key" is the one link these tests -/// cannot close; it is standard responder-chain behaviour with no code of this app's in it. +/// - **The delegate hook itself**, which stays wired because it is the right answer wherever *AppKit* +/// asks a window's delegate for a manager (`HostedWindowController.windowWillReturnUndoManager`). +/// `NSWindow.validateMenuItem(_:)` is the sharpest instrument a headless run has for reading what +/// that hook returned — enablement, the lock, the composed title, the two levels — so the menu +/// rows still appear here as the *probe*, not as the subject. +/// - **The toolbar pair's explicit target** (`BoardToolbar`), which validates and fires against the +/// session's `BoardUndoManager` directly. That path is the app's own end to end, and it is the one +/// a headless run can close completely. @MainActor @Suite("History ▸ the command surface") struct UndoCommandSurfaceTests { @@ -641,13 +645,17 @@ struct UndoCommandSurfaceTests { return (window, controller) } + /// A row carrying the platform's own `undo:`/`redo:`, used here as a **probe** rather than as a + /// shipped surface: validating one against the window is how a test reads back the manager + /// `windowWillReturnUndoManager` returned, title composition and all. The app's own rows carry no + /// selector at all (`UndoCommands.swift`). private func menuItem(_ selector: String) -> NSMenuItem { NSMenuItem(title: selector == "undo:" ? "Undo" : "Redo", action: NSSelectorFromString(selector), keyEquivalent: "") } - // MARK: The menu rows + // MARK: What the delegate hands back - @Test("The Edit menu's rows read the board's stack, and retitle themselves from its step names") + @Test("A window's delegate hands back the board's stack, titles composing from its step names") func theMenuRowsReadTheBoardsStack() { let provider = FakeHistoryProvider() let manager = BoardUndoManager(history: provider) @@ -667,9 +675,10 @@ struct UndoCommandSurfaceTests { provider.canUndo = true provider.undoActionName = "Move 3 Cards" - // 13's "the 06 vocabulary supplies menu titles ('Undo Move 3 Cards'), via NSUndoManager's - // dynamic retitling": the app never writes that string — the platform composes it from the - // bare phrase the seam vends, and validation is when it lands on the row. + // 13's "the vocabulary supplies menu titles ('Undo Move 3 Cards'), via dynamic retitling": + // the app never writes that string — the platform composes it from the bare phrase the seam + // vends (`BoardUndoManager.undoMenuItemTitle`), and this is where it lands. The app's own + // rows read the very same property, one step further out (`UndoCommandRouting.undoTitle`). #expect(window.validateMenuItem(undoRow)) #expect(undoRow.title == "Undo Move 3 Cards") #expect(window.validateMenuItem(redoRow) == false) @@ -752,8 +761,10 @@ struct UndoCommandSurfaceTests { #expect(boardWindow.validateMenuItem(boardRow)) #expect(boardRow.title == "Undo Move 3 Cards") - // **No fall-through** (06-history-undo.md ▸ Undo routing): the card window's own stack is - // empty, so its row is disabled and ⌘Z beeps — it never reaches the board's step. + // **No fall-through** (13-native-undo.md ▸ Undo routing): the card window's own stack is + // empty, so its row is disabled and ⌘Z beeps — it never reaches the board's step. The rows + // enforce it the same way now, by reading one focused value that is one window's or the + // other's (`FocusedValues.undoStack`) and never both. #expect(cardWindow.validateMenuItem(cardRow) == false) #expect(cardRow.title == "Undo") @@ -765,92 +776,93 @@ struct UndoCommandSurfaceTests { // MARK: The toolbar twins - @Test("The toolbar pair validates identically to the menu rows — and keeps its static labels") - func theToolbarPairMatchesTheMenuRows() throws { + /// A board toolbar wired the way `BoardWindowHost` wires one, over `undo`. + private func boardToolbar(store: BoardStore, undo: BoardUndoManager?) -> WindowToolbarController { + let domain = "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)" + return BoardToolbar.controller( + store: store, + search: BoardSearchPresentation(), + zoom: BoardZoomStore(defaults: UserDefaults(suiteName: domain)!), + appearance: AppearanceStore(defaults: UserDefaults(suiteName: domain + ".appearance")!, apply: { _ in }), + session: DragSession(), + undo: undo + ) + } + + /// One real item, built by the real delegate. + private func toolbarItem( + _ controller: WindowToolbarController, + _ identifier: NSToolbarItem.Identifier + ) throws -> NSToolbarItem { + try #require(controller.toolbar( + controller.toolbar, + itemForItemIdentifier: identifier, + willBeInsertedIntoToolbar: true + )) + } + + /// **The pair carries an explicit target now** (13-native-undo.md ▸ Rules ▸ the command-surface + /// bullet, re-ruled 2026-08-08) — the toolbar controller, over the session's `BoardUndoManager`, + /// where until that ruling both items carried nil targets and `undo:`/`redo:` selectors for the + /// responder chain to resolve. + /// + /// This replaces the pin that read the pair's validation *through the window* against the menu + /// rows'. The claim it was making — one answer on both surfaces — is unchanged and now stronger: + /// they are not two validations that agree, they are one object both of them read + /// (`bothSurfacesReadOneManager`). + @Test("The toolbar pair targets the session's stack — and keeps its static labels") + func theToolbarPairCarriesAnExplicitTarget() throws { let fixture = try makeBoard() defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) - let controller = BoardToolbar.controller( - store: store, - search: BoardSearchPresentation(), - zoom: BoardZoomStore(defaults: UserDefaults(suiteName: "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)")!), - appearance: AppearanceStore(defaults: UserDefaults(suiteName: "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)")!, apply: { _ in }), - session: DragSession() - ) let provider = FakeHistoryProvider() let manager = BoardUndoManager(history: provider) - let (window, hosted) = hostedWindow(manager) - defer { hosted.detach() } + let controller = boardToolbar(store: store, undo: manager) - /// The real items, built by the real delegate — nil target, `undo:`/`redo:` actions. - func item(_ identifier: NSToolbarItem.Identifier) throws -> NSToolbarItem { - try #require(controller.toolbar( - controller.toolbar, - itemForItemIdentifier: identifier, - willBeInsertedIntoToolbar: true - )) - } - let undoItem = try item(.boardUndo) - let redoItem = try item(.boardRedo) - #expect(undoItem.target == nil, "nil target: the chain resolves it, exactly as the menu row's is") - #expect(redoItem.target == nil) + let undoItem = try toolbarItem(controller, .boardUndo) + let redoItem = try toolbarItem(controller, .boardRedo) + #expect(undoItem.target === controller, "the app's own target, not the responder chain's lookup") + #expect(redoItem.target === controller) - // `validateUserInterfaceItem` is what `NSToolbarItem.validate()` asks its resolved target, - // and `validateMenuItem` is what a menu row's asks — one predicate, two doors. - #expect(window.validateUserInterfaceItem(undoItem) == false) - #expect(window.validateUserInterfaceItem(redoItem) == false) + undoItem.validate() + redoItem.validate() + #expect(undoItem.isEnabled == false, "an empty stack dims it") + #expect(redoItem.isEnabled == false) provider.canUndo = true provider.undoActionName = "Move 3 Cards" provider.canRedo = true provider.redoActionName = "Rename Lane" - let undoRow = menuItem("undo:") - let redoRow = menuItem("redo:") - #expect(window.validateUserInterfaceItem(undoItem) == window.validateMenuItem(undoRow)) - #expect(window.validateUserInterfaceItem(redoItem) == window.validateMenuItem(redoRow)) - #expect(window.validateUserInterfaceItem(undoItem)) - #expect(window.validateUserInterfaceItem(redoItem)) + undoItem.validate() + redoItem.validate() + #expect(undoItem.isEnabled) + #expect(redoItem.isEnabled) - // 03's one exception to the label rule, proven rather than asserted: validation rewrote the - // *menu* row's title and left the toolbar item's label exactly where it was. - #expect(undoRow.title == "Undo Move 3 Cards") + // 03's one exception to the label rule, proven rather than asserted: the phrase the *menu* + // composes ("Undo Move 3 Cards") never reaches a toolbar label, whatever validation does. + #expect(manager.undoMenuItemTitle == "Undo Move 3 Cards") #expect(undoItem.label == "Undo") #expect(redoItem.label == "Redo") #expect(undoItem.paletteLabel == "Undo", "the customize palette shows the static label too") } @Test("A toolbar item's own validation lands on the board's answer, lock included") - func theToolbarItemValidatesThroughTheWindow() throws { + func theToolbarItemValidatesThroughTheManager() throws { final class Lock { var isOn = false } let lock = Lock() let fixture = try makeBoard() defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) - let zoomDomain = "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)" - defer { UserDefaults.standard.removePersistentDomain(forName: zoomDomain) } - let toolbar = BoardToolbar.controller( - store: store, - search: BoardSearchPresentation(), - zoom: BoardZoomStore(defaults: UserDefaults(suiteName: zoomDomain)!), - appearance: AppearanceStore(defaults: UserDefaults(suiteName: zoomDomain + ".appearance")!, apply: { _ in }), - session: DragSession() - ) let provider = FakeHistoryProvider() let manager = BoardUndoManager(history: provider, isReadOnly: { lock.isOn }) - let (window, hosted) = hostedWindow(manager) - defer { hosted.detach() } + let controller = boardToolbar(store: store, undo: manager) - let item = try #require(toolbar.toolbar( - toolbar.toolbar, - itemForItemIdentifier: .boardUndo, - willBeInsertedIntoToolbar: true - )) - // The one link a headless run cannot make: `NSToolbarItem.validate()` resolves its target - // through the key window, and a test host has none. Standing the window in as the target is - // that lookup's *answer* — which is what nil-target means when a board window is key — so - // what this asserts is the item's own validation path, end to end from `validate()`. - item.target = window + let item = try toolbarItem(controller, .boardUndo) + // The link a headless run no longer has to fake: validation used to resolve a nil target + // through the key window, which a test host does not have, so the window stood in as the + // answer. The item is handed its target at construction now, so this is the shipped path + // end to end from `validate()`. #expect(item.autovalidates, "AppKit revalidates it on user events; the observation covers the rest") item.validate() @@ -870,27 +882,67 @@ struct UndoCommandSurfaceTests { #expect(item.label == "Undo", "no crossing of validation ever moves the label") } - @Test("The pair's enablement is deliberately not a predicate of the toolbar's own") - func theSpecsAbstainFromEnablement() throws { + @Test("Clicking the item crosses the board's stack, through the target it was given") + func theToolbarItemCrossesTheStack() throws { let fixture = try makeBoard() defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) + let provider = FakeHistoryProvider() + provider.canUndo = true + provider.canRedo = true + let manager = BoardUndoManager(history: provider) + let controller = boardToolbar(store: store, undo: manager) + + let undoItem = try toolbarItem(controller, .boardUndo) + let redoItem = try toolbarItem(controller, .boardRedo) + let target = try #require(undoItem.target as? NSObject) + let action = try #require(undoItem.action) + + target.perform(action, with: undoItem) + #expect(provider.undoCount == 1, "the click reaches the session's stack, not a responder's") + + let redoTarget = try #require(redoItem.target as? NSObject) + redoTarget.perform(try #require(redoItem.action), with: redoItem) + #expect(provider.redoCount == 1) + } + + /// **One manager, two faces** — what "the toolbar mirrors the menu" means for this pair now that + /// neither of them goes through the responder chain (13-native-undo.md ▸ Rules ▸ the + /// command-surface bullet, re-ruled 2026-08-08; `BoardToolbar`'s header). + /// + /// This is where the retired `theSpecsAbstainFromEnablement` pin went. Its claim — that the pair + /// must never grow a second answer able to disagree with the menu's — is the same claim, made + /// the only way it can be now that the items *do* carry a predicate: the predicate and the row + /// are reading one object, so they cannot come apart. + @Test("The toolbar pair and the Edit menu's rows read one and the same manager") + func bothSurfacesReadOneManager() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let provider = FakeHistoryProvider() + let manager = BoardUndoManager(history: provider) let specs = BoardToolbar.specs( store: store, search: BoardSearchPresentation(), zoom: BoardZoomStore(defaults: UserDefaults(suiteName: "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)")!), appearance: AppearanceStore(defaults: UserDefaults(suiteName: "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)")!, apply: { _ in }), - session: DragSession() + session: DragSession(), + undo: manager ) - // Every other item mirrors its menu row's predicate; these two mirror the *mechanism*. A - // spec-level `isEnabled` here would be a second answer able to disagree with the responder - // chain's — and it would have to read a stack the toolbar has no route to, since the board's - // window is what owns that answer. - for identifier in [NSToolbarItem.Identifier.boardUndo, .boardRedo] { - let spec = try #require(specs.first { $0.identifier == identifier }) - #expect(spec.isEnabled, "abstention, not enablement: AppKit's own validation decides") - #expect(spec.isOn == nil) + // The row's side of it, derived exactly as `UndoMenuRow` derives it: the focused stack, with + // nothing text-shaped holding the keyboard. + func rowIsEnabled() -> Bool { + UndoCommandRouting.canUndo( + UndoCommandRouting.routedManager(stack: manager, firstResponder: nil) + ) + } + + for state in [false, true, false] { + provider.canUndo = state + let spec = try #require(specs.first { $0.identifier == .boardUndo }) + #expect(spec.isEnabled == state) + #expect(spec.isEnabled == rowIsEnabled(), "one object answers both surfaces") } } } diff --git a/KanbanTests/ToolbarTests.swift b/KanbanTests/ToolbarTests.swift index 6b02e17..9e4b12b 100644 --- a/KanbanTests/ToolbarTests.swift +++ b/KanbanTests/ToolbarTests.swift @@ -50,22 +50,46 @@ private func makeAppearance() -> AppearanceStore { return AppearanceStore(defaults: UserDefaults(suiteName: name)!, apply: { _ in }) } -/// The board catalog, with the three collaborators every test here supplies the same way: a fresh -/// zoom store, a fresh appearance store, and a drag session with nothing in flight. +/// The board catalog, with the four collaborators every test here supplies the same way: a fresh +/// zoom store, a fresh appearance store, a drag session with nothing in flight, and no undo stack — +/// which is the honest default for a suite whose subject is the *catalog*, and the Undo/Redo pair's +/// own tests pass one (`undoPairCrossesTheSessionsStack`). @MainActor private func boardSpecs( store: BoardStore, search: BoardSearchPresentation = BoardSearchPresentation(), zoom: BoardZoomStore? = nil, appearance: AppearanceStore? = nil, - session: DragSession = DragSession() + session: DragSession = DragSession(), + undo: BoardUndoManager? = nil ) -> [ToolbarItemSpec] { BoardToolbar.specs( store: store, search: search, zoom: zoom ?? makeZoom(), appearance: appearance ?? makeAppearance(), - session: session + session: session, + undo: undo + ) +} + +/// A board stack with a real substrate behind it — the shape `BoardWindowHost` hands the toolbar +/// (`AppModel`'s session composes exactly this pair). +@MainActor +private func makeUndo(isReadOnly: @escaping @MainActor () -> Bool = { false }) -> (BoardUndoManager, NativeHistoryProvider) { + let provider = NativeHistoryProvider() + return (BoardUndoManager(history: provider, isReadOnly: isReadOnly), provider) +} + +/// One step that records its crossings — `HistoryProviderTests`' synthetic fixture, in the one shape +/// this suite needs: what the toolbar pair must prove is that firing it *reaches* the stack, not what +/// the stack then does to disk. +@MainActor +private func countingStep(_ name: String, crossings: @escaping @MainActor () -> Void) -> HistoryStep { + HistoryStep( + name: name, + undo: { _ in crossings(); return .applied }, + redo: { _ in crossings(); return .applied } ) } @@ -240,17 +264,104 @@ struct BoardToolbarTests { #expect(specs.map(\.label) == [ "New Card", "New Lane", "Zoom In", "Zoom Out", "Undo", "Redo", "Show Trash", "Appearance", "Search", ]) - // The one exception 03 names: "the Undo/Redo toolbar items keep static labels — - // NSUndoManager rewrites their menu titles dynamically ('Undo Move Card…'), which a toolbar - // label doesn't track". They are also the two items with no action of their own: nil target, - // responder-chain selectors, "matching their menu items" by using the same lookup. + // The one exception 03 names: "the Undo/Redo toolbar items keep static labels — the menu + // titles are rewritten dynamically ('Undo Move Card…'), which a toolbar label doesn't + // track". They are ordinary buttons since the command surface became the app's own + // (13-native-undo.md ▸ Rules ▸ the command-surface bullet, re-ruled 2026-08-08) — the label + // exception survived the mechanism that motivated it, because it was never about *how* the + // item fires. for identifier in [NSToolbarItem.Identifier.boardUndo, .boardRedo] { let spec = try #require(specs.spec(identifier)) - guard case let .responderAction(selector) = spec.behavior else { - Issue.record("\(identifier.rawValue) must reach the responder chain like its menu row") + guard case .button = spec.behavior else { + Issue.record("\(identifier.rawValue) must carry an explicit target over the session's stack") return } - #expect(selector == NSSelectorFromString(spec.label.lowercased() + ":")) + #expect(spec.isOn == nil, "\(spec.label) is a push button, not a toggle") + } + } + + /// **The pair's predicate is the menu rows' own object** (13-native-undo.md ▸ Rules ▸ the + /// command-surface bullet, re-ruled 2026-08-08): both surfaces read one `BoardUndoManager`, so + /// enablement here is `canUndo`/`canRedo` and nothing of the toolbar's own. + /// + /// This replaces the nil-target pin the pair carried until that re-ruling — the responder-chain + /// route it named is unreachable on a SwiftUI window, and what took its place is a predicate this + /// suite can read directly rather than one only `NSWindow` could answer. + @Test("Undo and Redo mirror the session's stack, and firing them crosses it") + func undoPairCrossesTheSessionsStack() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let (manager, provider) = makeUndo() + let specs = boardSpecs(store: store, undo: manager) + let undo = try #require(specs.spec(.boardUndo)) + let redo = try #require(specs.spec(.boardRedo)) + + #expect(!undo.isEnabled, "an empty stack dims it") + #expect(!redo.isEnabled) + + var crossings: [String] = [] + provider.register(countingStep("Move 3 Cards") { crossings.append("undo") }) + + #expect(undo.isEnabled, "a step on the stack lights it") + #expect(!redo.isEnabled, "and nothing has been crossed yet") + + undo.activate() + #expect(crossings == ["undo"], "firing the item crosses the board's own stack") + #expect(!undo.isEnabled, "the stack is empty again") + #expect(redo.isEnabled, "and the crossed step is on the other one") + + redo.activate() + #expect(crossings == ["undo", "undo"], "the synthetic step records both halves the same way") + #expect(undo.isEnabled) + } + + /// The read-only lock disables the pair with every other mutating command, and it does it in the + /// one place it is decided — `BoardUndoManager.isReadOnly` (13-native-undo.md ▸ Rules ▸ locks; + /// 02-architecture.md § "The lock's scope"). The stack is untouched, which is why the items come + /// back when it clears. + @Test("The read-only lock dims the pair, steps and all") + func theLockDimsThePair() throws { + final class Lock { var isOn = false } + let lock = Lock() + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let (manager, provider) = makeUndo(isReadOnly: { lock.isOn }) + provider.register(countingStep("Move Card") {}) + provider.register(countingStep("Rename Lane") {}) + provider.undo() + + let specs = boardSpecs(store: store, undo: manager) + let undo = try #require(specs.spec(.boardUndo)) + let redo = try #require(specs.spec(.boardRedo)) + #expect(undo.isEnabled) + #expect(redo.isEnabled) + + lock.isOn = true + #expect(!undo.isEnabled, "disabled with every other mutating command") + #expect(!redo.isEnabled) + #expect(provider.canUndo, "an enablement answer, not a clearing — the stack survives") + + lock.isOn = false + #expect(undo.isEnabled, "and resumes when the lock clears") + #expect(redo.isEnabled) + } + + /// A window whose session has gone hands the catalog `nil`, and the pair reads that as an empty + /// stack rather than as an error — the same quiet the adapter gives a board with no provider + /// (`BoardUndoManager.history`). + @Test("With no stack in front the pair is dim, and firing it does nothing") + func theUndoPairAbstainsWithNoStack() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let specs = boardSpecs(store: store, undo: nil) + + for identifier in [NSToolbarItem.Identifier.boardUndo, .boardRedo] { + let spec = try #require(specs.spec(identifier)) + #expect(!spec.isEnabled) + spec.activate() } } @@ -292,7 +403,7 @@ struct BoardToolbarTests { defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) let search = BoardSearchPresentation() - let controller = BoardToolbar.controller(store: store, search: search, zoom: makeZoom(), appearance: makeAppearance(), session: DragSession()) + let controller = BoardToolbar.controller(store: store, search: search, zoom: makeZoom(), appearance: makeAppearance(), session: DragSession(), undo: nil) // 03's three customization sentences: "right-click ▸ Customize Toolbar…, drag to rearrange, // system overflow and icon/text display options". @@ -321,7 +432,7 @@ struct BoardToolbarTests { defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) let search = BoardSearchPresentation() - let controller = BoardToolbar.controller(store: store, search: search, zoom: makeZoom(), appearance: makeAppearance(), session: DragSession()) + let controller = BoardToolbar.controller(store: store, search: search, zoom: makeZoom(), appearance: makeAppearance(), session: DragSession(), undo: nil) #expect(search.focusField == nil, "nothing to focus until the item exists") @@ -356,7 +467,7 @@ struct BoardToolbarTests { let fixture = try makeBoard() defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) - let controller = BoardToolbar.controller(store: store, search: BoardSearchPresentation(), zoom: makeZoom(), appearance: makeAppearance(), session: DragSession()) + let controller = BoardToolbar.controller(store: store, search: BoardSearchPresentation(), zoom: makeZoom(), appearance: makeAppearance(), session: DragSession(), undo: nil) let item = try #require(controller.toolbar( controller.toolbar, @@ -390,7 +501,7 @@ struct BoardToolbarTests { defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) let search = BoardSearchPresentation() - let controller = BoardToolbar.controller(store: store, search: search, zoom: makeZoom(), appearance: makeAppearance(), session: DragSession()) + let controller = BoardToolbar.controller(store: store, search: search, zoom: makeZoom(), appearance: makeAppearance(), session: DragSession(), undo: nil) _ = controller.toolbar( controller.toolbar, diff --git a/KanbanTests/UndoCommandsTests.swift b/KanbanTests/UndoCommandsTests.swift new file mode 100644 index 0000000..e24e25a --- /dev/null +++ b/KanbanTests/UndoCommandsTests.swift @@ -0,0 +1,169 @@ +import AppKit +import Testing +@testable import Kanban + +// MARK: - Fixtures + +/// A board stack with a real substrate behind it — the pair a session composes (`AppModel`). +@MainActor +private func makeStack(isReadOnly: @escaping @MainActor () -> Bool = { false }) -> (BoardUndoManager, NativeHistoryProvider) { + let provider = NativeHistoryProvider() + return (BoardUndoManager(history: provider, isReadOnly: isReadOnly), provider) +} + +/// A step that applies in both directions and does nothing else — this suite's subject is which +/// manager a row *reaches*, never what crossing it does to disk. +@MainActor +private func step(_ name: String) -> HistoryStep { + HistoryStep(name: name, undo: { _ in .applied }, redo: { _ in .applied }) +} + +/// A text view's delegate vending a manager of its own — `CardBodySurface` and `CardRawSourceView` +/// in one line, which is the shape 13-native-undo.md ▸ Undo routing calls "an editor's +/// delegate-vended manager". +@MainActor +private final class UndoVendingDelegate: NSObject, NSTextViewDelegate { + + let manager = UndoManager() + + func undoManager(for view: NSTextView) -> UndoManager? { manager } +} + +// MARK: - Routing + +/// **Which manager the app's own Undo/Redo rows answer with** (13-native-undo.md ▸ Undo routing, the +/// predicate; ▸ Rules ▸ the command-surface bullet, re-ruled 2026-08-08 — the rule unchanged, its +/// enactment moved from the window delegate to the command layer). +/// +/// The decision is three lines that are invisible until they are wrong, which is `BoardUndoRouting`'s +/// own reason for being a pure enum and this suite's for pinning every branch of the one above it. +@MainActor +@Suite("Undo commands ▸ routing") +struct UndoCommandRoutingTests { + + @Test("With nothing holding the keyboard, a row answers with the focused stack") + func noResponderAnswersTheStack() { + let (manager, _) = makeStack() + + #expect(UndoCommandRouting.routedManager(stack: manager, firstResponder: nil) === manager) + #expect(UndoCommandRouting.routedManager(stack: nil, firstResponder: nil) == nil) + } + + @Test("A responder that is not a text surface is not one — the stack answers") + func nonTextResponderAnswersTheStack() { + let (manager, _) = makeStack() + + #expect(UndoCommandRouting.routedManager(stack: manager, firstResponder: NSResponder()) === manager) + #expect(UndoCommandRouting.routedManager(stack: manager, firstResponder: NSView()) === manager) + // A field that has focus without editing is the board's, not the field editor's: AppKit + // installs the field editor only when editing begins (`BoardUndoRouting.isTextEditing`). + #expect(UndoCommandRouting.routedManager(stack: manager, firstResponder: NSTextField()) === manager) + #expect(UndoCommandRouting.routedManager(stack: nil, firstResponder: NSView()) == nil) + } + + @Test("An editor's own manager wins the keyboard, whatever the board stack holds") + func aFocusedEditorAnswersWithItsOwnManager() { + let (manager, provider) = makeStack() + provider.register(step("Move 3 Cards")) + let delegate = UndoVendingDelegate() + let editor = NSTextView() + editor.allowsUndo = true + editor.delegate = delegate + + let routed = UndoCommandRouting.routedManager(stack: manager, firstResponder: editor) + + #expect(routed === delegate.manager) + // **No fall-through** (13 ▸ Undo routing): the editor's stack is empty and the board's is + // not, and the row still answers with the editor's — "exhausting a focused editor's stack + // beeps; it never reaches board history". The row that renders over this is disabled, and + // the ⌘Z that fires against it beeps. + #expect(manager.canUndo, "the board has a step to cross, and the row will not cross it") + #expect(UndoCommandRouting.canUndo(routed) == false) + #expect(UndoCommandRouting.undoTitle(of: routed) == "Undo", "the editor's bare verb, not the board's phrase") + } + + @Test("A text surface with no manager of its own answers nothing at all") + func aTextSurfaceWithNoManagerAnswersNil() { + let (manager, provider) = makeStack() + provider.register(step("Move 3 Cards")) + let editor = NSTextView() + editor.allowsUndo = true + + // No delegate to vend one and no window to inherit one from: the responder chain ends here. + // `nil` rather than the board's stack, which is the same no-fall-through rule read from its + // other end — a text surface holding the keyboard is never a reason to reach past it. + #expect(editor.undoManager == nil) + #expect(UndoCommandRouting.routedManager(stack: manager, firstResponder: editor) == nil) + #expect(UndoCommandRouting.routedManager(stack: nil, firstResponder: editor) == nil) + } +} + +// MARK: - Titles and enablement + +/// **What a rendered row says and whether it is live** — the two halves the rows route along with the +/// action, because a row that advertised a step it would not cross, or swallowed ⌘Z while dimmed off +/// the wrong stack, is exactly how no-fall-through fails from the user's side (13-native-undo.md +/// ▸ Rules ▸ the command-surface bullet, re-ruled 2026-08-08). +@MainActor +@Suite("Undo commands ▸ titles and enablement") +struct UndoCommandTitleTests { + + @Test("No manager is the bare verb, disabled — a row with nothing in front of it") + func noManagerIsTheBareVerb() { + #expect(UndoCommandRouting.undoTitle(of: nil) == "Undo") + #expect(UndoCommandRouting.redoTitle(of: nil) == "Redo") + #expect(UndoCommandRouting.canUndo(nil) == false) + #expect(UndoCommandRouting.canRedo(nil) == false) + } + + @Test("A named step composes the platform's title, and lights the row") + func aNamedStepComposesTheTitle() { + let (manager, provider) = makeStack() + + #expect(UndoCommandRouting.undoTitle(of: manager) == "Undo", "an empty stack keeps the bare verb") + #expect(UndoCommandRouting.canUndo(manager) == false) + + provider.register(step("Move 3 Cards")) + + #expect(UndoCommandRouting.canUndo(manager)) + #expect(UndoCommandRouting.undoTitle(of: manager) == "Undo Move 3 Cards") + #expect(UndoCommandRouting.canRedo(manager) == false, "nothing crossed yet") + + provider.undo() + + #expect(UndoCommandRouting.canUndo(manager) == false) + #expect(UndoCommandRouting.canRedo(manager)) + #expect(UndoCommandRouting.redoTitle(of: manager) == "Redo Move 3 Cards") + } + + /// The trim is why these helpers exist at all rather than the rows reading `undoMenuItemTitle` + /// directly: `BoardUndoManager` trims its own nameless composition, and a plain `NSUndoManager` — + /// a text view's, a field editor's — does not, leaving "Undo " with a trailing space on the row. + @Test("A nameless plain NSUndoManager still reads as the bare verb") + func aNamelessTextManagerTrimsToTheBareVerb() { + let text = UndoManager() + + #expect(UndoCommandRouting.undoTitle(of: text) == "Undo") + #expect(UndoCommandRouting.redoTitle(of: text) == "Redo") + } + + /// The read-only lock reaches the rows through the very object they read — one answer, wherever + /// it is asked from (13 ▸ Rules ▸ locks; `BoardUndoManager.canUndo`). + @Test("The read-only lock dims the row and leaves its name standing") + func theLockDimsTheRow() { + final class Lock { var isOn = false } + let lock = Lock() + let (manager, provider) = makeStack(isReadOnly: { lock.isOn }) + provider.register(step("Move Card")) + + #expect(UndoCommandRouting.canUndo(manager)) + + lock.isOn = true + + #expect(UndoCommandRouting.canUndo(manager) == false) + #expect(UndoCommandRouting.undoTitle(of: manager) == "Undo Move Card", "the stack survives the lock") + + lock.isOn = false + #expect(UndoCommandRouting.canUndo(manager), "and resumes when it clears") + } +}