From 1ffb64913b64b98eed03ee05945946e27cd26c28 Mon Sep 17 00:00:00 2001 From: rzen Date: Mon, 3 Aug 2026 13:24:53 -0400 Subject: [PATCH] =?UTF-8?q?A=20stationary=20modifier=20flip=20answers=20at?= =?UTF-8?q?=20the=20keystroke=20=E2=80=94=20the=20third=20client=20of=20th?= =?UTF-8?q?e=20one=20shared=20retarget?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Modifiers were sampled only inside resolveOperation, reached from dropUpdated and the commit — and drop callbacks arrive only while the mouse moves, so ⌥ pressed against a still pointer changed nothing until the next twitch. DragSession now arms a local .flagsChanged watch for exactly the session's lifetime (begin arms, end stops, the watchdog guarantees end; ⇧/⌃/caps don't count, a settled hold swallows) and publishes modifierGeneration; the three retargets record which window's surface resolved the proposal (RetargetOrigin — weak registry identity, never board root, so cross-board and two-windows- one-board both answer correctly), and the hovered window replays that same retarget with the operation re-resolved FIRST, since the index space is a function of it. The monitor is injectable — the real one needs a live event stream no test bundle has. Drag-perf card d491e7d3. Claude-Session: https://claude.ai/code/session_01CqjXB7ASoWtbyoGod68k97 --- DRAG-REORDER.md | 2 + Kanban/UI/Board/BoardDrops.swift | 90 ++++- Kanban/UI/Board/BoardView.swift | 15 + Kanban/UI/Board/DragSession.swift | 227 ++++++++++- KanbanTests/ModifierFlipTests.swift | 561 ++++++++++++++++++++++++++++ 5 files changed, 885 insertions(+), 10 deletions(-) create mode 100644 KanbanTests/ModifierFlipTests.swift diff --git a/DRAG-REORDER.md b/DRAG-REORDER.md index 6409891..c6a6f66 100644 --- a/DRAG-REORDER.md +++ b/DRAG-REORDER.md @@ -112,6 +112,8 @@ Two carve-outs: Geometry does not change across the boundary. The destination board's own resting layout answers the proposal, in the destination's own `standard` and gap; the arriving run's span is its unit counts measured against the destination's standard. What changes is only which commit runs and on which store — see below. +**"Sampled every frame" means sampled on every drop callback, and drop callbacks arrive only while the mouse moves.** A modifier pressed against a perfectly still pointer therefore reaches nothing: the operation decides what the *source* board's resting layout holds (a copy leaves its originals standing — see Resting-layout zones), whether the trash column takes the drop at all, and which index space the shadows are counted in, and all three used to wait for the next twitch of the mouse. So a `.flagsChanged` watch runs for exactly the drag's lifetime — armed at `begin`, stopped at `end`, which is where every way a drag can finish already funnels — and publishes a counter; the board window under the cursor turns that counter back into **the one shared retarget** (`BoardDropContext.retargetAfterModifierFlip`), the same seam the autoscroll driver's every scroll step goes through. The flip carries no location and needs none: the retargets read the physical mouse, so a stationary pointer is simply the cursor they already read. The window that answers is the one whose surface resolved the standing proposal — recorded per retarget as a `LaneDropRegistry`, which is per board *window* — so a cross-board drag re-proposes against the board being hovered and never against the one it came from. A settled release ignores flips entirely, the same freeze the committed-overlay hold applies to `propose` and to the operation itself. + ## The drop commits The commit is the store's, and it is one `performWrite` bracket per gesture whatever the set's size: one app-mediated reload, and (on git boards) one commit rather than N. Every one of them takes an index counted **against the destination's rendered items as the resting layout showed them** — so the number the geometry produced is the number the writer consumes, unrewritten. For every move that means "with the dragged run removed"; for the within-board ⌥-copy it means "with the originals still there", because a copy leaves them there and the zones counted them (see **Resting-layout zones**). Cross-board arrivals never face the question: the destination never held the originals. diff --git a/Kanban/UI/Board/BoardDrops.swift b/Kanban/UI/Board/BoardDrops.swift index 32fe473..5e8f154 100644 --- a/Kanban/UI/Board/BoardDrops.swift +++ b/Kanban/UI/Board/BoardDrops.swift @@ -381,7 +381,11 @@ struct BoardDropContext { /// still "the lanes minus the run" every time this function actually runs, and `moveLanes` / /// `receiveLanes` keep the index space they always had. func retargetLanes() { - guard session.isDraggingLanes, let cursor = stripCursor() else { return } + guard session.isDraggingLanes else { return } + // Before the cursor, so the flip's replay address is "the surface that got the event" rather + // than "the surface that managed to answer" (`DragSession.noteRetarget`). + session.noteRetarget(.strip, registry: registry) + guard let cursor = stripCursor() else { return } let hidden = session.hiddenMembers(onBoardRooted: store.rootKey) let resting = store.snapshot.lanes.filter { !hidden.contains($0.id) } let restingUnits = resting.map { LaneLayoutMath.displayUnits(of: $0) } @@ -422,7 +426,12 @@ struct BoardDropContext { let span = DragSignposts.beginRetargetCards() defer { DragSignposts.endRetargetCards(span) } #endif - guard session.isDraggingCards, let cursor = globalCursor() else { return } + guard session.isDraggingCards else { return } + // See `retargetLanes` — recorded before the cursor is read, and by the retarget rather than + // by its callers, so the delegate, the strip's fall-through and the autoscroll driver all + // leave the same address behind. + session.noteRetarget(.lane(laneID), registry: registry) + guard let cursor = globalCursor() else { return } guard let resting = session.restingLayouts.layout( inLane: laneID, of: store, @@ -484,13 +493,17 @@ struct BoardDropContext { /// A method rather than a property because it is not free of consequence: the operation is /// re-resolved against the modifiers *at this instant*, which is also what keeps the badge honest /// while the cursor sits over the column (`dropProposal`). - func acceptsTrashDrop() -> Bool { + /// + /// `modifiers` defaults to the live flags, which is the only thing the app ever passes — the + /// parameter is `DragSession.resolveOperation`'s own seam, one rung out, so the refusal this + /// gate exists for (⌥ over the column) is checkable without a keyboard (`ModifierFlipTests`). + func acceptsTrashDrop(modifiers: NSEvent.ModifierFlags = NSEvent.modifierFlags) -> Bool { guard let sourceRoot = session.sourceRoot else { return false } return TrashDrop.accepts( kind: session.kind, container: session.container, isWithinBoard: sourceRoot == store.rootKey, - operation: session.resolveOperation(destinationRoot: store.rootKey), + operation: session.resolveOperation(destinationRoot: store.rootKey, modifiers: modifiers), isTrashShown: store.transient.isTrashVisible, acceptsMutations: store.acceptsBoardMutations ) @@ -506,9 +519,19 @@ struct BoardDropContext { /// trash proposes nothing for you": the column declines to be a target, it does not cancel the /// drag the user is still holding. So a lane drag reorders across the column exactly as it always /// did, and an ⌥-copy released over it still lands where its shadows are. - func retargetTrash() { + func retargetTrash(modifiers: NSEvent.ModifierFlags = NSEvent.modifierFlags) { + // A session, or there is nothing to record an address for. Provably a no-op otherwise — + // every branch below is kind-guarded one level down — and stated so that "an origin means a + // drag" holds (`DragSession.noteRetarget`). + guard session.isActive else { return } revalidateProposal() - guard acceptsTrashDrop() else { + // The fall-through below may overwrite this address with the surface that actually answered, + // and that is exactly right: a flip replays whatever resolved the proposal last. The one + // case this gate exists for keeps it — a cursor over the column resolves to no lane, so the + // strip's card fall-through holds without recording, and the column stays the address the + // ⌥-release has to be re-asked at. + session.noteRetarget(.trash, registry: registry) + guard acceptsTrashDrop(modifiers: modifiers) else { retargetFromStrip() return } @@ -519,6 +542,61 @@ struct BoardDropContext { )) } + // MARK: Retargeting — the modifier flip + + /// **The stationary modifier flip's answer** (04-interactions.md ▸ Drag and drop: "⌥ always + /// forces copy and ⌘ always forces move … the badge tracks the effective operation live"). + /// + /// The modifiers are sampled inside `resolveOperation`, which is reached from `dropUpdated` and + /// the commit — and drop callbacks arrive only while the mouse **moves**. So ⌥ pressed with the + /// pointer perfectly still used to change nothing visible until the next twitch: the source + /// board's originals stayed lifted, the trash kept refusing (or kept offering) against the old + /// operation, and the shadows stood in an index space the release would no longer count in. + /// This is the missing sample, arriving from the keyboard instead of from the mouse. + /// + /// **The third client of the one shared retarget** (DRAG-REORDER.md § Edge autoscroll: "the + /// lane's drop delegate and the autoscroll driver must go through one shared retarget, so they + /// can never disagree"). It resolves nothing itself — it re-runs the retarget that resolved the + /// standing proposal, at the surface that resolved it (`DragSession.RetargetOrigin`), so a flip + /// and a mouse sample land in exactly the same place by construction. + /// + /// **No location comes off the flip event, and none is needed.** Every retarget reads the + /// physical cursor through this window (`globalCursor`), which is precisely why the autoscroll + /// driver can re-propose against a stationary pointer while the content moves under it; here the + /// content stands still and the *operation* moves instead. + /// + /// Three guards, in the order that makes each one cheap: + /// + /// - **A settled release is past retargeting.** `resolveOperation` and `propose` both freeze + /// under a hold, so this would be inert anyway; refusing outright means a flip during the + /// settle costs nothing at all, and states the freeze where it can be read. + /// - **Only the window under the cursor answers.** The origin names a `LaneDropRegistry`, which + /// is per board *window* — so a cross-board drag re-proposes against the board being hovered + /// rather than the one it was picked up from, two windows on one board do not both answer, and + /// a window closed mid-drag drops out silently (the reference is weak). + /// - **The operation is re-resolved before the retarget, not after it.** `dropUpdated` runs them + /// the other way round and can afford to: at 60+ samples a second the next event repairs the + /// one-sample lag. A flip gets one shot, and the retarget's index space is a function of the + /// operation (`retargetCards` → `hiddenMembers`), so the operation has to be current *first* + /// or the shadows would land in the space the flip just left. + func retargetAfterModifierFlip(modifiers: NSEvent.ModifierFlags = NSEvent.modifierFlags) { + guard session.isActive, session.hold == nil else { return } + guard let origin = session.retargetOrigin, origin.registry === registry else { return } + + session.resolveOperation(destinationRoot: store.rootKey, modifiers: modifiers) + + switch origin.surface { + case .strip: + retargetFromStrip() + case let .lane(laneID): + // The lane delegate's own pair, in its order: rule 2 first, then the masonry's zones. + revalidateProposal() + retargetCards(inLane: laneID) + case .trash: + retargetTrash(modifiers: modifiers) + } + } + // MARK: Retargeting — external Finder file sessions /// Whether `info` is an **external Finder file** session rather than one of ours. diff --git a/Kanban/UI/Board/BoardView.swift b/Kanban/UI/Board/BoardView.swift index 551e14d..c8f3b04 100644 --- a/Kanban/UI/Board/BoardView.swift +++ b/Kanban/UI/Board/BoardView.swift @@ -188,6 +188,21 @@ struct BoardView: View { .onChange(of: store.landedReloads) { _, _ in resize.handOff(against: store.snapshot.lanes) } + // **The stationary modifier flip's re-proposal** (`BoardDropContext.retargetAfterModifierFlip`). + // Drop callbacks arrive only while the mouse moves, so a ⌥ pressed against a still pointer + // reaches the drag through the session's `.flagsChanged` watch instead — and lands back on + // the one shared retarget here, in the board window whose surface resolved the proposal. + // + // Every open board window carries this handler and exactly one of them acts: the flip + // replays against a `LaneDropRegistry`, which is this window's alone. + // + // **No transaction of its own**, deliberately: a flip must produce exactly what the next + // mouse sample would have produced, and `dropUpdated` wraps nothing either. What moves is + // keyed declaratively where it is drawn — the strip on its proposal, each lane on its shadow + // run (03-board-ui.md § Motion, transactions keyed narrowly). + .onChange(of: appModel.dragSession.modifierGeneration) { _, _ in + dropContext.retargetAfterModifierFlip() + } .trashPurgeAlert(store: store, confirmations: confirmations) // The board's own anchor for the Style… popover — the surface a board-targeted session hangs // off, since the board has no item to attach to (`styleEditorPresentation`'s `nil` anchor). diff --git a/Kanban/UI/Board/DragSession.swift b/Kanban/UI/Board/DragSession.swift index f58f085..95ce03f 100644 --- a/Kanban/UI/Board/DragSession.swift +++ b/Kanban/UI/Board/DragSession.swift @@ -351,6 +351,56 @@ final class DragSession { /// `CommittedHold`. private(set) var hold: CommittedHold? + // MARK: The modifier flip + + /// **The stationary-flip nudge**: bumped once for every change to ⌥/⌘ while a session is in + /// flight and unsettled (`ModifierFlipSource`, armed at `begin` and stopped at `end`). + /// + /// The whole of what this object can say about a flip, and deliberately so. The *effect* of a + /// flip is a re-proposal, and re-proposing needs a board window's geometry — which board is + /// under the cursor, where its lanes are drawn — none of which an app-wide session has. So the + /// flip is published as a counter and the hovered board's own drop context turns it back into + /// the one shared retarget (`BoardDropContext.retargetAfterModifierFlip`), which is the same + /// seam the autoscroll driver's every scroll step goes through. + /// + /// Observed, unlike everything else the event handlers write here: it exists to invalidate a + /// board window's body. That costs a strip body pass per **keystroke**, not per mouse sample — + /// and the flip changes `hiddenMembers`, so that pass was happening anyway. + private(set) var modifierGeneration = 0 + + /// The board surface that last resolved this session's proposal — which window, and which of + /// its drop surfaces — so a flip can re-run *that* retarget rather than guess at one. + /// + /// **The window is matched by registry identity, never by board root.** Two windows open on one + /// board share a store and a root but not a `LaneDropRegistry` (the cache's key already turns on + /// exactly this), and only one of them has the cursor over it; a root comparison would have the + /// other one retargeting against a cursor that is nowhere near its lanes. + /// + /// **Weak**, because a board window can close mid-drag: the session outlives it, and a flip + /// afterwards simply finds no one to answer — which is the honest answer, since the surface that + /// was resolving the proposal is gone. + /// + /// `@ObservationIgnored` for `LaneDropRegistry`'s own reason: it is written from *event* + /// handlers, on every sample of every drag, and a body that re-ran for it would be the animation + /// feedback loop the drop model exists to avoid. + @ObservationIgnored private(set) var retargetOrigin: RetargetOrigin? + + /// Which board window ran the last retarget, and which of its surfaces. + struct RetargetOrigin { + + /// The three surfaces that resolve a proposal — the three retargets a flip can replay. + /// Each names the function that recorded it: `.strip` is `retargetLanes`, `.lane` is + /// `retargetCards(inLane:)`, `.trash` is `retargetTrash`. + enum Surface: Equatable { + case strip + case lane(ItemID) + case trash + } + + weak var registry: LaneDropRegistry? + var surface: Surface + } + // MARK: The external file mode /// Where an external Finder file drag would land, or `nil` when there is none in flight or it is @@ -368,13 +418,31 @@ final class DragSession { @ObservationIgnored private var fileWatchdog: Task? @ObservationIgnored private var holdTimeoutTask: Task? + /// Where the flips come from, and the running watch — armed at `begin`, stopped at `end`, and + /// nowhere else (see `armFlipWatch`). + @ObservationIgnored private let flipSource: any ModifierFlipSource + @ObservationIgnored private var flipWatch: (any ModifierFlipWatch)? + + /// The ⌥/⌘ state the last flip reported, so a `.flagsChanged` that moved neither — ⇧ for the + /// marquee's own grammar, ⌃, caps lock, a function key — costs nothing at all. + @ObservationIgnored private var flipFlags: NSEvent.ModifierFlags = [] + /// How long this session's holds may stand with no snapshot arriving — `CommittedHold.timeout`, /// and a `var` for one reason only: the discard path is a `Task` sleeping on the main actor, and /// a test that had to wait the real figure out would be a 1.5 s wall clock in the suite /// (`DragSessionTests`). Nothing in the app writes it. @ObservationIgnored var holdTimeout: Duration = CommittedHold.timeout - init() {} + /// **The two flags the effective operation is a function of** — `DragLocality.operation` reads + /// these and nothing else, so these are the whole of what "a flip" means here. + static let operationFlags: NSEvent.ModifierFlags = [.option, .command] + + /// `flipSource` is defaulted to the shipping local monitor; it is a parameter for the reason + /// `DragAutoScroller`'s tick source is one — the real thing needs a live `NSApplication` event + /// stream, which a test bundle has no way to feed (`ModifierFlipTests`). + init(flipSource: any ModifierFlipSource = LocalModifierFlipSource()) { + self.flipSource = flipSource + } // MARK: Queries @@ -411,9 +479,12 @@ final class DragSession { /// /// Reading `operation` here is what makes the flip visible at all: the property is observed, so /// every surface that builds a resting layout off this method re-renders when it changes. The - /// flip lands on the next `dropUpdated`, since that is where the operation is re-resolved; a - /// modifier pressed with the mouse perfectly still waits for the next motion (Backlog ▸ - /// Stationary modifier flips). + /// flip lands on the next `dropUpdated`, since that is where the operation is re-resolved — + /// **or, with the mouse perfectly still, on the flip's own nudge**: drop callbacks arrive only + /// while the mouse moves, so a `.flagsChanged` monitor bumps `modifierGeneration` and the + /// hovered board re-runs the retarget the last callback ran + /// (`BoardDropContext.retargetAfterModifierFlip`). A stationary ⌥ is therefore answered at the + /// keystroke rather than at the next mouse sample. /// /// **The hold freezes the answer**, because `resolveOperation` refuses to move once a release /// has settled: a settled copy keeps its originals on screen and a settled move keeps them @@ -596,7 +667,9 @@ final class DragSession { // The reload-resolved drag set: vanished members leave it silently, which is what // `survivors` reads and what "an emptied drag cancels itself" is stated in terms of. source.transient.dragMembers = ItemReferenceSet(ids: memberSet, container: container) + self.retargetOrigin = nil armWatchdog() + armFlipWatch() } /// Records a new proposal. `nil` withdraws it — rule 2's "the shadow withdraws". @@ -609,6 +682,28 @@ final class DragSession { proposal = target } + /// Records which board window's surface just resolved the proposal (`RetargetOrigin`) — the + /// address a stationary modifier flip replays its retarget at. + /// + /// Written by the three retargets themselves rather than by their five callers, so the drop + /// delegates, the strip's fall-through and the autoscroll driver cannot disagree about where a + /// flip should land any more than they can disagree about where the drop should. + /// + /// **Recorded even when the retarget goes on to hold**, which is the point: a cursor over a gap + /// or a dead region leaves the proposal exactly where it was, and a flip there must re-ask the + /// same surface the same question — with the modifiers now saying something else. + /// + /// Unchanged addresses are not re-stored, `propose`'s own habit and for a sharper reason here: + /// this runs on every mouse sample *and* every autoscroll frame, and the steady state of a drag + /// is the same surface answering over and over — a weak reference restored per frame for no + /// change at all is exactly the kind of hot-path cost the drag model has been shedding. + func noteRetarget(_ surface: RetargetOrigin.Surface, registry: LaneDropRegistry) { + if let origin = retargetOrigin, origin.surface == surface, origin.registry === registry { + return + } + retargetOrigin = RetargetOrigin(registry: registry, surface: surface) + } + /// Re-resolves the effective operation against the board under the cursor and the modifiers /// **right now**, and hands it back for the `DropProposal` the badge tracks. /// @@ -651,8 +746,13 @@ final class DragSession { operation = .move sourceStore = nil sourceRoot = nil + retargetOrigin = nil watchdog?.cancel() watchdog = nil + // The monitor's lifetime is the session's, and this is the sentence that makes it true: + // every path that ends a drag — a delegate's cancel, the button-up belt, the watchdog, the + // hold's hand-off, the hold's timeout — funnels through here. + stopFlipWatch() endHold() } @@ -761,6 +861,56 @@ final class DragSession { } } + // MARK: The modifier flip — the watch, and what a flip means + + /// Starts watching ⌥/⌘ for this session (`ModifierFlipSource`). + /// + /// **Armed at `begin` and stopped at `end`, deliberately nowhere else.** The watchdog already + /// guarantees `end` runs for every session macOS never reports the finish of, so tying the + /// monitor to that same bracket is what makes "it cannot outlive the drag" structural rather + /// than a list of call sites to keep in step — the resting-layout cache's precedent exactly. + /// A previous watch is stopped first for `armWatchdog`'s reason: a second `begin` with a session + /// somehow still standing must not leave the first one's monitor installed. + /// + /// The **committed hold** deliberately does not stop it: a hold is still this session, and the + /// freeze it applies is `resolveOperation`'s and `propose`'s, asked at the flip rather than + /// spelled a second time in the lifecycle (`noteFlip` restates it anyway, so a flip during a + /// settle costs not even a body pass). + private func armFlipWatch() { + stopFlipWatch() + // The baseline is the modifier state the drag is *starting* under — a ⌥ that was already + // down at pickup is not a flip, and `DragLocality` has already seen it. + flipFlags = NSEvent.modifierFlags.intersection(Self.operationFlags) + flipWatch = flipSource.watch { [weak self] flags in + self?.noteFlip(flags) + } + } + + private func stopFlipWatch() { + flipWatch?.stop() + flipWatch = nil + } + + /// A `.flagsChanged` arrived. Publishes the nudge the hovered board turns back into a + /// re-proposal, and only when something the operation depends on actually moved. + /// + /// **The event's location is deliberately unread.** A flip is not a pointer event: the mouse is + /// exactly where the last drop callback left it, and every retarget reads the *physical* cursor + /// through its own window (`BoardDropContext.globalCursor`) rather than any event's coordinates. + /// That is the same reason the autoscroll driver needs no events at all to keep re-proposing. + /// + /// **A settled release ignores flips**, the third face of the freeze `resolveOperation` and + /// `propose` already wear: the write named an operation and the overlay draws that operation + /// until the echo lands, so a ⌥ released between the drop and the reload must not re-lift + /// originals the write is leaving in place. + private func noteFlip(_ flags: NSEvent.ModifierFlags) { + let relevant = flags.intersection(Self.operationFlags) + guard relevant != flipFlags else { return } + flipFlags = relevant + guard isActive, hold == nil else { return } + modifierGeneration &+= 1 + } + /// The file mode's own watchdog, and its only guaranteed termination path. /// /// An external session is not ours to end: no `performDrop` runs when the user drops the files @@ -785,3 +935,72 @@ final class DragSession { } } } + +// MARK: - Watching the modifiers + +/// Where `DragSession` learns that ⌥ or ⌘ moved — the seam under the local `.flagsChanged` monitor, +/// and `DragAutoScrollTickSource`'s exact cousin: the shipping implementation needs a live +/// `NSApplication` event stream, which a test bundle has no way to feed. +/// +/// `Sendable` so a session's stored source is, like the tick source's; the watch it hands back is +/// main-actor state and the handler runs there, which is where every drag input is read. +protocol ModifierFlipSource: Sendable { + + /// Starts reporting modifier changes, until the returned watch is stopped. The flags are the + /// event's own — the *whole* set, since deciding which bits matter is the session's job + /// (`DragSession.operationFlags`). + @MainActor + func watch(_ onFlip: @escaping @MainActor (NSEvent.ModifierFlags) -> Void) -> any ModifierFlipWatch +} + +/// A running modifier watch. `stop()` is what retires the underlying monitor, and it is idempotent +/// so the session's teardown can call it on every path without asking whether one is installed. +@MainActor +protocol ModifierFlipWatch: AnyObject { + func stop() +} + +/// The shipping source: a **local** `NSEvent` monitor for `.flagsChanged`. +/// +/// Local rather than global, for two reasons that point the same way. A global monitor for keyboard +/// events needs Accessibility permission — an enormous ask for a drag affordance — and it would +/// report modifiers pressed while another app is frontmost, which is not a flip in *this* drag at +/// all. The events a drag actually needs are the ones being dispatched to this application while it +/// holds the session. +struct LocalModifierFlipSource: ModifierFlipSource { + + @MainActor + func watch(_ onFlip: @escaping @MainActor (NSEvent.ModifierFlags) -> Void) -> any ModifierFlipWatch { + LocalModifierFlipWatch(onFlip) + } +} + +/// The monitor token's owner, and the one place it is removed. +/// +/// `addLocalMonitorForEvents` hands back an opaque token that `removeMonitor` **must** be given — +/// AppKit retains the handler until it is, so a token dropped on the floor is a block that keeps +/// firing for the rest of the process. Holding it in an object whose only method retires it is what +/// makes the removal unmissable: `DragSession.end()` stops the watch, and every way a drag can +/// finish goes through `end()`. +@MainActor +private final class LocalModifierFlipWatch: ModifierFlipWatch { + + private var token: Any? + + init(_ onFlip: @escaping @MainActor (NSEvent.ModifierFlags) -> Void) { + token = NSEvent.addLocalMonitorForEvents(matching: .flagsChanged) { event in + // Local monitors run on the main thread, before the event reaches its window. + MainActor.assumeIsolated { onFlip(event.modifierFlags) } + // **Returned unchanged, always.** ⌥ and ⌘ mean things to the rest of the app — the + // click grammar, the menu bar's key equivalents — and a monitor that swallowed them + // would be reading the drag's modifiers by taking them away from everything else. + return event + } + } + + func stop() { + guard let token else { return } + NSEvent.removeMonitor(token) + self.token = nil + } +} diff --git a/KanbanTests/ModifierFlipTests.swift b/KanbanTests/ModifierFlipTests.swift new file mode 100644 index 0000000..e434c98 --- /dev/null +++ b/KanbanTests/ModifierFlipTests.swift @@ -0,0 +1,561 @@ +import AppKit +import Foundation +import Testing +@testable import Kanban + +/// **The stationary modifier flip** (04-interactions.md ▸ Drag and drop: "⌥ always forces copy and +/// ⌘ always forces move … the badge tracks the effective operation live"). +/// +/// The effective operation is sampled inside `DragSession.resolveOperation`, which is reached from +/// `dropUpdated` and from the commit — and `DropDelegate` callbacks arrive only while the mouse +/// **moves**. So a modifier pressed against a perfectly still pointer used to change nothing until +/// the next twitch: the source board's originals stayed lifted (`hiddenMembers` follows the +/// operation since the copy-redraw ruling), the trash column kept refusing a copy it would now take, +/// and the shadows stood in an index space the release no longer counted in. +/// +/// The fix is a `.flagsChanged` watch whose lifetime is the drag's, and whose only output is a +/// counter; the board window under the cursor turns that counter back into **the one shared +/// retarget** (`BoardDropContext.retargetAfterModifierFlip`) — the third client of the seam +/// DRAG-REORDER.md § Edge autoscroll opened for the autoscroll driver. +/// +/// Two halves, tested at the two seams they live at: +/// +/// - the **watch's lifetime** and what counts as a flip, on `DragSession`, through a scripted +/// source — the shipping one is a local `NSEvent` monitor, which needs a live application event +/// stream a test bundle cannot feed (`DragAutoScrollDriverTests`' scripted tick source exactly); +/// - the **funnel**, on `BoardDropContext`, through the modifier seam `resolveOperation` already +/// established: the flags are a defaulted parameter, so the whole re-proposal is checkable +/// without a keyboard. +/// +/// What is *not* here, because no unit can hold it: whether the system's own drag badge tracks +/// modifiers without our help (`NSDraggingSource` ANDs modifier-driven operations into the mask +/// unless `ignoreModifierKeysWhileDragging`), and whether a local monitor is delivered inside +/// AppKit's drag-tracking loop at all. Both are live-verify items; the source is a seam precisely so +/// the second one has somewhere to be answered. + +// MARK: - The scripted source + +/// A stand-in for `LocalModifierFlipSource` whose flags the test presses, and which records the one +/// thing the real source must do when a drag ends: it removes the monitor. +@MainActor +private final class ScriptedFlips: ModifierFlipSource { + + /// How many monitors this source has installed, and how many have been retired — the pair every + /// leak claim below is made of. + private(set) var installs = 0 + private(set) var removals = 0 + + private var handler: (@MainActor (NSEvent.ModifierFlags) -> Void)? + private var current: Watch? + + /// Whether a monitor is installed right now. + var isWatching: Bool { handler != nil } + + func watch( + _ onFlip: @escaping @MainActor (NSEvent.ModifierFlags) -> Void + ) -> any ModifierFlipWatch { + installs += 1 + handler = onFlip + let watch = Watch(source: self) + current = watch + return watch + } + + /// A `.flagsChanged` arriving with this flag state. The event reports the **whole** set rather + /// than a delta, which is what the session's own comparison assumes. + func press(_ flags: NSEvent.ModifierFlags) { + handler?(flags) + } + + fileprivate func retire(_ watch: Watch) { + guard current === watch else { return } + removals += 1 + handler = nil + current = nil + } + + /// Stopping twice is not two removals — the session's teardown calls `stop()` on every path + /// without asking whether a watch is installed. + @MainActor + fileprivate final class Watch: ModifierFlipWatch { + private unowned let source: ScriptedFlips + private var stopped = false + + init(source: ScriptedFlips) { self.source = source } + + func stop() { + guard !stopped else { return } + stopped = true + source.retire(self) + } + } +} + +// MARK: - Fixtures + +/// One lane with one card — enough for a session to pick up, a lane to propose into, and a trash +/// column to refuse. +@MainActor +private func makeBoard() throws -> WriterFixture { + let fixture = try WriterFixture() + try fixture.item("", Item.board) + try fixture.item(Ident.lane1, Item.rich(order: "1024", title: "Todo")) + try fixture.item("\(Ident.lane1)/\(Ident.card1)", Item.rich(order: "1024", title: "First")) + return fixture +} + +/// A drop context with no window, which is exactly what the flip path can be asked about: every +/// cursor-dependent retarget holds (`globalCursor` answers `nil`), so what is left is the operation, +/// the origin dispatch and the trash's modifier-sensitive refusal — the three things a flip decides. +@MainActor +private func makeDrops( + store: BoardStore, + session: DragSession, + registry: LaneDropRegistry +) -> BoardDropContext { + BoardDropContext( + store: store, + session: session, + registry: registry, + gap: 12, + window: { nil }, + stripFrame: { .zero }, + standard: { 260 } + ) +} + +@MainActor +private func pickUpCard(_ session: DragSession, from store: BoardStore) { + session.beginCards( + [ItemID(rawValue: Ident.card1)], + folders: [store.rootURL + .appendingPathComponent(Ident.lane1, isDirectory: true) + .appendingPathComponent(Ident.card1, isDirectory: true)], + heights: [44], + container: .board, + source: store + ) +} + +// MARK: - The watch's lifetime + +/// **The monitor cannot outlive the drag**, and the reason it cannot is structural rather than a +/// list of call sites: it is armed at `begin` and stopped at `end`, and `end` is where every way a +/// drag can finish already funnels — a delegate's cancel, the button-up belt, the watchdog, the +/// hold's hand-off, the hold's timeout. +@MainActor +@Suite("Modifier flips ▸ the watch's lifetime") +struct ModifierFlipLifetimeTests { + + @Test("A drag arms the watch, and ending it retires the monitor") + func armedAtBeginRetiredAtEnd() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let flips = ScriptedFlips() + let session = DragSession(flipSource: flips) + + #expect(!flips.isWatching, "an idle session watches nothing") + + pickUpCard(session, from: store) + #expect(flips.isWatching) + #expect(flips.installs == 1) + + session.end() + #expect(!flips.isWatching) + #expect(flips.removals == 1, "the token must be handed back, or the block fires forever") + } + + @Test("A second drag never leaves the first one's monitor installed") + func aSecondBeginRetiresTheFirstWatch() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let flips = ScriptedFlips() + let session = DragSession(flipSource: flips) + + pickUpCard(session, from: store) + // `begin` without an intervening `end` — the lifecycle trap the watchdog exists for, where a + // finished session's events arrive after the next drag has started. + pickUpCard(session, from: store) + + #expect(flips.installs == 2) + #expect(flips.removals == 1, "arming replaces the watch rather than stacking a second one") + #expect(flips.isWatching) + + session.end() + #expect(flips.removals == 2) + } + + /// The path no delegate can see: a drag macOS never reports the end of. The watchdog polls the + /// physical button and clears the session, and clearing the session is what retires the monitor. + @Test("The watchdog's own teardown retires the monitor") + func theWatchdogRetiresTheWatch() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let flips = ScriptedFlips() + let session = DragSession(flipSource: flips) + + pickUpCard(session, from: store) + #expect(flips.isWatching) + + // The button is not down in a test bundle, so the watchdog's poll plus its grace period is + // the whole wait. Bounded, so a watchdog that never fires fails rather than hangs. + var cleared = false + for _ in 0..<200 where !cleared { + try? await Task.sleep(for: .milliseconds(10)) + cleared = !session.isActive + } + + #expect(cleared, "the watchdog is the guaranteed termination path") + #expect(!flips.isWatching) + #expect(flips.removals == 1) + } + + /// A Finder file session arms none of this object's own state, so it arms no watch either — and + /// it needs none: a file drop is always a copy (`fileDropProposal`), whatever is held down. + @Test("An external file session arms no watch") + func fileSessionsAreNotWatched() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let flips = ScriptedFlips() + let session = DragSession(flipSource: flips) + + session.proposeFile(FileDropTarget( + boardRoot: store.rootKey, + landing: .create(laneID: ItemID(rawValue: Ident.lane1), index: 0), + fileCount: 1 + )) + + #expect(!flips.isWatching) + #expect(flips.installs == 0) + session.proposeFile(nil) + } +} + +// MARK: - What counts as a flip + +/// The nudge itself: one counter, bumped only when something the *operation* is a function of +/// actually moved (`DragLocality.operation` reads ⌥ and ⌘, and nothing else). +@MainActor +@Suite("Modifier flips ▸ the nudge") +struct ModifierFlipNudgeTests { + + @Test("Pressing ⌥ and releasing it are two flips") + func pressAndReleaseBothNudge() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let flips = ScriptedFlips() + let session = DragSession(flipSource: flips) + + pickUpCard(session, from: store) + let base = session.modifierGeneration + + flips.press([.option]) + #expect(session.modifierGeneration == base + 1) + + flips.press([]) + #expect(session.modifierGeneration == base + 2) + } + + @Test("A flags event that moves neither ⌥ nor ⌘ is not a flip") + func irrelevantFlagsAreIgnored() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let flips = ScriptedFlips() + let session = DragSession(flipSource: flips) + + pickUpCard(session, from: store) + let base = session.modifierGeneration + + // ⇧ has its own meaning in the click grammar and none at all in the drag's operation. + flips.press([.shift]) + flips.press([.control]) + flips.press([.capsLock]) + #expect(session.modifierGeneration == base) + + // And ⌥ *with* one of them along for the ride is still exactly one flip. + flips.press([.shift, .option]) + #expect(session.modifierGeneration == base + 1) + flips.press([.control, .option]) + #expect(session.modifierGeneration == base + 1, "⌥ never moved") + } + + /// The freeze, at its earliest point: a settled release is past retargeting, so a ⌥ released + /// between the drop and the echo must not even cost a board window a body pass. + @Test("A flip under a committed hold is not a nudge") + func theHoldSwallowsFlips() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let flips = ScriptedFlips() + let session = DragSession(flipSource: flips) + + pickUpCard(session, from: store) + session.propose(DropTarget( + boardRoot: store.rootKey, + container: .lane(ItemID(rawValue: Ident.lane1)), + index: 0 + )) + session.commit(into: store) + let base = session.modifierGeneration + + flips.press([.option]) + flips.press([]) + #expect(session.modifierGeneration == base) + } + + @Test("A flip after the drag is over reaches nothing at all") + func aFinishedDragHearsNothing() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let flips = ScriptedFlips() + let session = DragSession(flipSource: flips) + + pickUpCard(session, from: store) + session.end() + let base = session.modifierGeneration + + flips.press([.option]) + #expect(session.modifierGeneration == base, "the monitor is gone; this is the belt") + } +} + +// MARK: - The funnel: a flip runs the one shared retarget + +/// **The third client of the shared retarget.** A flip resolves nothing of its own: it re-runs the +/// retarget that resolved the standing proposal, at the surface that resolved it, so a keystroke and +/// a mouse sample land in exactly the same place by construction. +@MainActor +@Suite("Modifier flips ▸ the shared retarget") +struct ModifierFlipRetargetTests { + + private static let lane1 = ItemID(rawValue: Ident.lane1) + private static let card1 = ItemID(rawValue: Ident.card1) + + // MARK: The address a flip replays at + + @Test("Each retarget leaves its own address behind, even when it goes on to hold") + func everyRetargetRecordsItsSurface() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let registry = LaneDropRegistry() + let session = DragSession(flipSource: ScriptedFlips()) + let drops = makeDrops(store: store, session: session, registry: registry) + + #expect(session.retargetOrigin == nil, "a fresh session has nowhere to replay") + + pickUpCard(session, from: store) + drops.retargetCards(inLane: Self.lane1) + #expect(session.retargetOrigin?.surface == .lane(Self.lane1)) + #expect(session.retargetOrigin?.registry === registry) + + drops.retargetTrash(modifiers: []) + #expect(session.retargetOrigin?.surface == .trash) + + // The lane level: the strip's own zones. + session.beginLanes( + [Self.lane1], + folders: [store.rootURL.appendingPathComponent(Ident.lane1, isDirectory: true)], + units: [1], + source: store + ) + #expect(session.retargetOrigin == nil, "a new drag starts with no address") + drops.retargetLanes() + #expect(session.retargetOrigin?.surface == .strip) + + session.end() + #expect(session.retargetOrigin == nil) + } + + // MARK: The re-admission at home + + /// The confirmed half of the card, end to end: ⌥ pressed with the mouse perfectly still flips + /// the operation to a copy, and a copy leaves its originals standing in the source board's + /// resting layout (`hiddenMembers`) — which used to wait for the next mouse sample. + @Test("A stationary ⌥ re-admits the originals at home") + func aStationaryOptionRedrawsTheSource() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let registry = LaneDropRegistry() + let session = DragSession(flipSource: ScriptedFlips()) + let drops = makeDrops(store: store, session: session, registry: registry) + + pickUpCard(session, from: store) + drops.retargetCards(inLane: Self.lane1) + #expect(session.operation == .move) + #expect(session.hiddenMembers(onBoardRooted: store.rootKey) == [Self.card1]) + + drops.retargetAfterModifierFlip(modifiers: [.option]) + + #expect(session.operation == .copy) + #expect(session.hiddenMembers(onBoardRooted: store.rootKey).isEmpty, + "a copy's originals stay exactly where they are") + + // And releasing ⌥ lifts them again, with the pointer still untouched. + drops.retargetAfterModifierFlip(modifiers: []) + #expect(session.operation == .move) + #expect(session.hiddenMembers(onBoardRooted: store.rootKey) == [Self.card1]) + } + + /// The one carve-out that outranks the modifier: a within-board lane drag is a reorder and ⌥ is + /// simply ignored there (`DragLocality.operation`). The flip runs; the answer does not move. + @Test("A within-board lane drag ignores the flip, exactly as it ignores the modifier") + func laneDragsIgnoreTheFlip() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let registry = LaneDropRegistry() + let session = DragSession(flipSource: ScriptedFlips()) + let drops = makeDrops(store: store, session: session, registry: registry) + + session.beginLanes( + [Self.lane1], + folders: [store.rootURL.appendingPathComponent(Ident.lane1, isDirectory: true)], + units: [1], + source: store + ) + drops.retargetLanes() + + drops.retargetAfterModifierFlip(modifiers: [.option]) + #expect(session.operation == .move) + #expect(session.hiddenMembers(onBoardRooted: store.rootKey) == [Self.lane1]) + } + + // MARK: The operation-dependent refusal + + /// **The trash refuses copies** (04-interactions.md ▸ The trash): "copying into the trash is not + /// a thing". The gate is a function of the operation, so it is a function of the modifiers — and + /// until now it was re-asked only when the mouse moved, which is why a ⌥ released over the + /// column left it inert under a pointer sitting right on it. + @Test("Releasing ⌥ over the trash proposes the delete, with the pointer perfectly still") + func theTrashReadmitsTheDropOnRelease() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + store.transient.isTrashVisible = true + let registry = LaneDropRegistry() + let session = DragSession(flipSource: ScriptedFlips()) + let drops = makeDrops(store: store, session: session, registry: registry) + + pickUpCard(session, from: store) + + // The cursor arrives over the column with ⌥ down: the copy is refused, and the column + // declines to be a target rather than cancelling the drag. + drops.retargetTrash(modifiers: [.option]) + #expect(session.operation == .copy) + #expect(session.trashProposal(onBoardRooted: store.rootKey) == nil) + + // ⌥ comes up. No mouse event follows — this is the whole point. + drops.retargetAfterModifierFlip(modifiers: []) + + #expect(session.operation == .move) + #expect(session.trashProposal(onBoardRooted: store.rootKey) == TrashDrop.landingIndex) + } + + // MARK: Rule 2, on the flip's path too + + /// The `.lane` replay is the lane delegate's own pair — revalidate, then the masonry's zones — + /// so a proposal whose lane vanished in a reload withdraws on a flip exactly as it would on the + /// next mouse sample. + @Test("A flip revalidates before it retargets: a vanished lane's proposal withdraws") + func theFlipAppliesRuleTwo() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let registry = LaneDropRegistry() + let session = DragSession(flipSource: ScriptedFlips()) + let drops = makeDrops(store: store, session: session, registry: registry) + + pickUpCard(session, from: store) + drops.retargetCards(inLane: Self.lane1) + // A proposal naming a lane this board does not have — the reload that deleted it landed + // while the pointer sat still. + session.propose(DropTarget( + boardRoot: store.rootKey, + container: .lane(ItemID(rawValue: Ident.lane2)), + index: 0 + )) + + drops.retargetAfterModifierFlip(modifiers: [.option]) + + #expect(session.proposal == nil, "deleted lanes are never drop targets") + } + + // MARK: Who answers + + /// The origin names a `LaneDropRegistry`, which is one board **window**'s — so the window under + /// the cursor answers a flip and no other does. A root comparison would have a second window on + /// the same board retargeting against a cursor nowhere near its lanes; a cross-board drag would + /// have the *source* board answering for the destination. + @Test("Only the window that resolved the proposal answers the flip") + func anotherWindowDoesNotAnswer() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let session = DragSession(flipSource: ScriptedFlips()) + let hovered = LaneDropRegistry() + let elsewhere = LaneDropRegistry() + let hoveredDrops = makeDrops(store: store, session: session, registry: hovered) + let otherDrops = makeDrops(store: store, session: session, registry: elsewhere) + + pickUpCard(session, from: store) + hoveredDrops.retargetCards(inLane: Self.lane1) + + otherDrops.retargetAfterModifierFlip(modifiers: [.option]) + #expect(session.operation == .move, "this window is not where the proposal lives") + #expect(session.hiddenMembers(onBoardRooted: store.rootKey) == [Self.card1]) + + hoveredDrops.retargetAfterModifierFlip(modifiers: [.option]) + #expect(session.operation == .copy) + } + + @Test("A flip with no address recorded resolves nothing") + func noOriginNoAnswer() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let session = DragSession(flipSource: ScriptedFlips()) + let drops = makeDrops(store: store, session: session, registry: LaneDropRegistry()) + + pickUpCard(session, from: store) + drops.retargetAfterModifierFlip(modifiers: [.option]) + + #expect(session.operation == .move, "no surface has resolved a proposal yet") + } + + // MARK: The freeze + + /// The same freeze `propose` and `resolveOperation` wear, asked once more at the funnel: the + /// write named an operation and the overlay draws *that* operation until the echo lands, so a ⌥ + /// released between the drop and the reload must not re-lift originals the write is leaving in + /// place. + @Test("A settled release is past retargeting — a flip changes nothing") + func theHoldFreezesTheFlip() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + store.transient.isTrashVisible = true + let registry = LaneDropRegistry() + let session = DragSession(flipSource: ScriptedFlips()) + let drops = makeDrops(store: store, session: session, registry: registry) + + pickUpCard(session, from: store) + drops.retargetTrash(modifiers: []) + #expect(session.trashProposal(onBoardRooted: store.rootKey) == TrashDrop.landingIndex) + + session.commit(into: store) + drops.retargetAfterModifierFlip(modifiers: [.option]) + + #expect(session.operation == .move, "the write named the move") + #expect(session.trashProposal(onBoardRooted: store.rootKey) == TrashDrop.landingIndex) + #expect(session.hiddenMembers(onBoardRooted: store.rootKey) == [Self.card1]) + } +}