diff --git a/Kanban/App/WindowToolbar.swift b/Kanban/App/WindowToolbar.swift index c03df20..83b2930 100644 --- a/Kanban/App/WindowToolbar.swift +++ b/Kanban/App/WindowToolbar.swift @@ -53,7 +53,7 @@ struct ToolbarItemSpec { let identifier: NSToolbarItem.Identifier let label: String - /// The SF Symbol the item draws. `nil` for a hosted control, which draws itself. + /// The SF Symbol the item draws. `nil` for the search item, whose field draws itself. let symbol: String? let behavior: Behavior @@ -68,10 +68,21 @@ struct ToolbarItemSpec { /// 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 hosted control — the board's search field. `make` is handed `true` when the view is - /// bound for the toolbar itself and `false` when it is the customization palette's copy, so - /// only the real one claims window-scoped wiring. - case control(width: CGFloat, make: (_ willBeInsertedIntoToolbar: Bool) -> NSView) + /// 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 + /// keyboard*, the resting width being the item's own. + /// + /// `make` is handed `true` when the field is bound for the toolbar itself and `false` when + /// it is the customization palette's copy, so only the real one claims window-scoped + /// wiring. `install` runs for that real item alone, and is where a caller wires the things + /// that need the *item* rather than the field — expanding it and putting the keyboard in it + /// is one call on `NSSearchToolbarItem`, and no field can make it. + case searchField( + focusedWidth: CGFloat, + make: (_ willBeInsertedIntoToolbar: Bool) -> NSSearchField, + install: (NSSearchToolbarItem) -> Void + ) } /// The vocabulary rule applied: an item that mirrors a menu row takes that row's title, minus a @@ -103,14 +114,14 @@ struct ToolbarItemSpec { // MARK: State - /// The item's live enablement. Responder-chain items and hosted controls answer `true`: the + /// 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. var isEnabled: Bool { switch behavior { case let .button(isEnabled, _): isEnabled() case let .toggle(isEnabled, _, _): isEnabled() - case .responderAction, .control: true + case .responderAction, .searchField: true } } @@ -118,7 +129,7 @@ struct ToolbarItemSpec { var isOn: Bool? { switch behavior { case let .toggle(_, isOn, _): isOn() - case .button, .responderAction, .control: nil + case .button, .responderAction, .searchField: nil } } @@ -128,7 +139,7 @@ struct ToolbarItemSpec { switch behavior { case let .button(_, perform): perform() case let .toggle(_, isOn, setOn): setOn(!isOn()) - case .responderAction, .control: break + case .responderAction, .searchField: break } } } @@ -155,10 +166,12 @@ struct ToolbarItemSpec { /// 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. -/// - **The search item hosts 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 back to the strip). A toolbar that already -/// speaks AppKit hosts it directly. +/// - **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 +/// back to the strip). The item is where grow-on-focus, the cancel button's staging, and the +/// overflow row all live, and it exists only in AppKit; a toolbar that already speaks AppKit +/// hosts it directly. /// - **The window is already proxied.** `HostedWindowController` fronts SwiftUI's window delegate /// and installs the board's titlebar accessory; a toolbar is the same kind of thing hung on the /// same window, through the same install-once seam. @@ -280,8 +293,15 @@ final class WindowToolbarController: NSObject, NSToolbarDelegate { return makeToggleItem(spec) case let .responderAction(selector): return makeResponderItem(spec, selector: selector) - case let .control(width, make): - return makeControlItem(spec, width: width, view: make(willBeInsertedIntoToolbar)) + case let .searchField(focusedWidth, make, install): + return makeSearchItem( + spec, + focusedWidth: focusedWidth, + field: make(willBeInsertedIntoToolbar), + // The palette's copy is a picture of the item, not a second live one: it claims no + // window-scoped wiring, so it is handed none. + install: willBeInsertedIntoToolbar ? install : nil + ) } } @@ -346,21 +366,43 @@ final class WindowToolbarController: NSObject, NSToolbarDelegate { return item } - /// The board's search field, hosted. Sized rather than flexible, matching the width the field - /// shipped with in m5. - private func makeControlItem(_ spec: ToolbarItemSpec, width: CGFloat, view: NSView) -> NSToolbarItem { - view.translatesAutoresizingMaskIntoConstraints = false - view.widthAnchor.constraint(equalToConstant: width).isActive = true - - let item = NSToolbarItem(itemIdentifier: spec.identifier) + /// The board's search field in the item AppKit wrote for it. + /// + /// **No width constraint here.** `NSSearchToolbarItem` manages the field's layout, and its + /// header says custom width constraints "should not conflict with" the preferred width — so the + /// em-based figure is handed over as a *preference*, which the item applies "whenever it gets + /// the keyboard focus". The resting width is the item's own, and grow-on-focus is what that + /// pair of facts means on screen. + /// + /// **Neither the overflow row nor the visibility priority is written here, and both omissions + /// are decisions.** The item ships its own `menuFormRepresentation` — a row titled from `label` + /// carrying a live AppKit action that widens the window until the field is usable — where a + /// custom-view item ships a blank one that has to be replaced; assigning here (`nil` included) + /// destroys it. And its `visibilityPriority` already starts one step *above* `.high`, so the + /// nudge a custom-view item needs to stay out of the overflow would be a demotion here. + private func makeSearchItem( + _ spec: ToolbarItemSpec, + focusedWidth: CGFloat, + field: NSSearchField, + install: ((NSSearchToolbarItem) -> Void)? + ) -> NSToolbarItem { + let item = NSSearchToolbarItem(itemIdentifier: spec.identifier) decorate(item, with: spec) - item.view = view - // No menu form representation, deliberately: a search field in the overflow *menu* is a - // field nobody can type into, so AppKit's generated row — label, no action, disabled — is - // the honest presentation. ⌘F covers that window: an installed field that cannot take the - // keyboard falls through to the transient strip (`BoardSearchPresentation.focusField`). - // The high priority keeps the board's one default item out of the overflow to begin with. - item.visibilityPriority = .high + // Configured before assignment, as the item's header asks — with one exception the header + // does not name: assignment stamps the *item's* enablement onto the field, so a field the + // caller made inert (the customization palette's copy) comes back live. The caller's answer + // is the one that counts, so it is put back. + let isFieldEnabled = field.isEnabled + item.searchField = field + field.isEnabled = isFieldEnabled + item.preferredWidthForSearchField = focusedWidth + // **Escape is staged** (04-interactions.md ▸ Search, settled: "in a non-empty field it + // clears the query, focus staying in the field; in an empty field it returns focus to the + // board"). AppKit's default is for the cancel button to clear *and* resign, which collapses + // the first two steps of that staircase into one — so the field keeps the keyboard, and the + // second press is what hands it back (`BoardSearchFieldController`). + item.resignsFirstResponderWithCancel = false + install?(item) return item } diff --git a/Kanban/UI/Board/BoardSearchField.swift b/Kanban/UI/Board/BoardSearchField.swift index d78ca6c..fc37d58 100644 --- a/Kanban/UI/Board/BoardSearchField.swift +++ b/Kanban/UI/Board/BoardSearchField.swift @@ -59,8 +59,13 @@ final class BoardSearchPresentation { /// arrives one SwiftUI update later, and claims the keyboard when it does. private var wantsFocusOnAppear = false - /// Makes the toolbar's field first responder, **answering whether it could**. `nil` until that - /// field has been made, which is also exactly when the toolbar has no search item to focus. + /// Puts the keyboard in the toolbar's field, **answering whether it could**. `nil` until the + /// item that hosts that field has been made, which is also exactly when the toolbar has no + /// search item to focus. + /// + /// It is `NSSearchToolbarItem.beginSearchInteraction()` underneath (`adopt`): one call that + /// expands the item to its focused width *and* moves the keyboard in, rather than a bare + /// `makeFirstResponder` that would focus a field still at its resting width. /// /// The answer matters because an installed item is not always a reachable one: pushed into the /// system overflow by a narrow window, the field is in no window and cannot take the keyboard. @@ -284,7 +289,10 @@ struct FindCommand: View { final class BoardSearchFieldController: NSObject, NSSearchFieldDelegate { /// Which of the field's two homes this one is — the only thing that differs between them, and it - /// differs in exactly one place: which focus handle the presentation gets. + /// differs in exactly one place: which focus handle the presentation gets, and who fills it in. + /// The transient field fills its own in, because a field inside a SwiftUI view is the whole of + /// that home; the toolbar's is the *item's* to give (`adopt`), because expanding the item and + /// putting the keyboard in it is one call on `NSSearchToolbarItem` and the field cannot make it. enum Home { case toolbar case transient @@ -297,6 +305,10 @@ final class BoardSearchFieldController: NSObject, NSSearchFieldDelegate { /// lives exactly as long as whoever holds the field — a toolbar item, or a SwiftUI view. private weak var field: FocusReportingSearchField? + /// The item this field lives in, in the toolbar home — `nil` in the transient one, which has no + /// item and so nothing to collapse. **Weak**: the item owns the field, which owns this. + private weak var item: NSSearchToolbarItem? + /// Makes the field, wires it, and hands it back. The caller owns the result and, through it, /// everything here. static func makeField( @@ -322,10 +334,9 @@ final class BoardSearchFieldController: NSObject, NSSearchFieldDelegate { // outlives neither its window nor its item. switch home { case .toolbar: - presentation.focusField = { [weak field] in - guard let field, let window = field.window else { return false } - return window.makeFirstResponder(field) - } + // Filled in by `adopt` when the item that hosts this field is built: ⌘F's handle there + // is the item's expand-and-focus, which is not the field's to offer. + break case .transient: presentation.focusTransientField = { [weak field] in guard let field, let window = field.window else { return } @@ -335,9 +346,36 @@ final class BoardSearchFieldController: NSObject, NSSearchFieldDelegate { return field } + /// Binds a field made for the toolbar to the `NSSearchToolbarItem` that hosts it — everything + /// about that home the field alone cannot answer. + /// + /// ⌘F becomes `beginSearchInteraction()`, which expands the item to its focused width and moves + /// the keyboard into the field in one call. **It still answers whether it could**: an item + /// pushed into the system overflow has no window, and a field in no window cannot take the + /// keyboard, so ⌘F falls through to the transient strip exactly as a removed item does + /// (`BoardSearchPresentation.focusField`). + /// + /// Called for the installed item only. The customization palette's copy is inert + /// (`makePaletteField`), so there is no controller here to adopt it and no handle for it to + /// claim — a palette item that answered ⌘F would be a second live field. + static func adopt(_ item: NSSearchToolbarItem, presentation: BoardSearchPresentation) { + let field = item.searchField + (field as? FocusReportingSearchField)?.controller?.item = item + presentation.focusField = { [weak item, weak field] in + guard let item, let field, field.window != nil else { return false } + item.beginSearchInteraction() + return true + } + } + /// An inert field for the customization palette's copy of the search item: it looks like the /// real one and does nothing, because a palette item that wrote to the store or claimed ⌘F's /// handle would be a second live field. + /// + /// Three things make it inert and each is load-bearing: no delegate, so a keystroke reaches no + /// store; no controller, so nothing observes the query; and no `adopt`, so ⌘F's handle stays the + /// installed item's. The disable is the part a *user* can see — + /// `NSSearchToolbarItem` overwrites it on assignment, and `WindowToolbarController` puts it back. static func makePaletteField() -> NSSearchField { let field = NSSearchField() field.placeholderString = "Search" @@ -402,6 +440,9 @@ final class BoardSearchFieldController: NSObject, NSSearchFieldDelegate { case #selector(NSResponder.cancelOperation(_:)): if store.searchQuery.isEmpty { presentation.focusBoard?() + // The keyboard has left, so the field goes back to its resting width with it — + // grow-on-focus, undone. A no-op in the transient home, which has no item. + item?.endSearchInteraction() } else { store.clearSearch() control.stringValue = "" diff --git a/Kanban/UI/Board/BoardToolbar.swift b/Kanban/UI/Board/BoardToolbar.swift index b280089..d159f6a 100644 --- a/Kanban/UI/Board/BoardToolbar.swift +++ b/Kanban/UI/Board/BoardToolbar.swift @@ -101,20 +101,30 @@ enum BoardToolbar { "Search", identifier: .boardSearch, symbol: nil, - // The field's width in *characters* rather than points (10-accessibility.md ▸ Text - // scaling: "no fixed point sizes") — the same 17 ems the transient bar's field takes - // (`BoardSearchBar`), so ⌘F's two homes are one width whichever the user is in. - behavior: .control(width: BoardMetrics.em(17, bodyPointSize: BoardMetrics.bodyPointSize)) { - [weak store] willBeInserted in - guard willBeInserted, let store else { - return BoardSearchFieldController.makePaletteField() + // The width in *characters* rather than points (10-accessibility.md ▸ Text scaling: + // "no fixed point sizes") — the same 17 ems the transient bar's field takes + // (`BoardSearchBar`). It is the **focused** width here: `NSSearchToolbarItem` grows + // the field to its preferred width when the keyboard arrives and lets it settle + // back to the item's natural width when the keyboard leaves, so the two homes match + // once the user is typing rather than at rest. + behavior: .searchField( + focusedWidth: BoardMetrics.em(17, bodyPointSize: BoardMetrics.bodyPointSize), + make: { [weak store] willBeInserted in + guard willBeInserted, let store else { + return BoardSearchFieldController.makePaletteField() + } + return BoardSearchFieldController.makeField( + store: store, + presentation: search, + home: .toolbar + ) + }, + // The toolbar half of the field's home that only the *item* can answer: ⌘F's + // expand-and-focus, and the collapse Escape's second step asks for. + install: { item in + BoardSearchFieldController.adopt(item, presentation: search) } - return BoardSearchFieldController.makeField( - store: store, - presentation: search, - home: .toolbar - ) - } + ) ), ] } diff --git a/KanbanTests/ToolbarTests.swift b/KanbanTests/ToolbarTests.swift index ef4711d..9c18061 100644 --- a/KanbanTests/ToolbarTests.swift +++ b/KanbanTests/ToolbarTests.swift @@ -202,20 +202,23 @@ struct BoardToolbarTests { #expect(search.focusField == nil, "nothing to focus until the item exists") + // `NSSearchToolbarItem.view` is unavailable — the item owns its layout — so the field is + // reached through `searchField`, which is also where it is handed in. let palette = try #require(controller.toolbar( controller.toolbar, itemForItemIdentifier: .boardSearch, willBeInsertedIntoToolbar: false - )) - #expect(palette.view is NSSearchField) + ) as? NSSearchToolbarItem) + #expect(!palette.searchField.isEnabled, "the palette's copy is a picture, not a second field") #expect(search.focusField == nil, "a palette copy must not claim ⌘F's handle") let installed = try #require(controller.toolbar( controller.toolbar, itemForItemIdentifier: .boardSearch, willBeInsertedIntoToolbar: true - )) - let field = try #require(installed.view as? NSSearchField) + ) as? NSSearchToolbarItem) + let field = installed.searchField + #expect(field.isEnabled, "the real one takes typing") #expect(search.focusField != nil, "the installed item is the field's home") // The live field writes through per keystroke, which is the m5 contract this milestone @@ -225,6 +228,63 @@ struct BoardToolbarTests { #expect(store.searchQuery == "spec") } + @Test("The search item is AppKit's own, configured for grow-on-focus and a staged Escape") + func searchItemIsTheStockOne() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let controller = BoardToolbar.controller(store: store, search: BoardSearchPresentation()) + + let item = try #require(controller.toolbar( + controller.toolbar, + itemForItemIdentifier: .boardSearch, + willBeInsertedIntoToolbar: true + ) as? NSSearchToolbarItem) + + // The em-based figure is the *focused* width: `NSSearchToolbarItem` applies its preferred + // width "whenever it gets the keyboard focus", and the resting width is the item's own + // (10-accessibility.md ▸ Text scaling: the number is characters, never points). + #expect( + item.preferredWidthForSearchField + == BoardMetrics.em(17, bodyPointSize: BoardMetrics.bodyPointSize) + ) + // **Escape is staged** (04-interactions.md ▸ Search, settled: "in a non-empty field it + // clears the query, focus staying in the field"). AppKit's default cancel button clears + // *and* resigns, which would collapse that first step into the second. + #expect(!item.resignsFirstResponderWithCancel) + + // The overflow row is the item's, and assigning here — `nil` included — destroys it. A + // custom-view item had to supply one; this one must not. + #expect(item.menuFormRepresentation != nil, "the item ships a live overflow row") + // Its own priority already sits above `.high`, so the nudge a custom-view item needed + // would be a demotion. + #expect(item.visibilityPriority.rawValue > NSToolbarItem.VisibilityPriority.high.rawValue) + } + + @Test("⌘F answers false for a field no window can give the keyboard to") + func focusHandleAnswersForAnUnrootedField() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let search = BoardSearchPresentation() + let controller = BoardToolbar.controller(store: store, search: search) + + _ = controller.toolbar( + controller.toolbar, + itemForItemIdentifier: .boardSearch, + willBeInsertedIntoToolbar: true + ) + + // The item exists, so the handle does. Nothing has put it in a window — the same shape the + // system overflow leaves the field in — so ⌘F must fall through to the transient strip + // rather than claim a focus it did not get (`BoardSearchPresentation.focusField`). + let focusField = try #require(search.focusField) + #expect(focusField() == false) + + search.invokeSearch() + #expect(search.isTransient, "⌘F always summons search") + } + @Test("Show Trash is a toggle whose state is the View menu's checkmark") func showTrashTogglesTheQuasiLane() throws { let fixture = try makeBoard() @@ -514,3 +574,151 @@ struct BoardSearchSurfacingTests { #expect(!presentation.isTransient) } } + +// MARK: - The field's two intercepted keys + +/// An `NSSearchToolbarItem` that counts the two interactions the field drives — the only way to see +/// grow-on-focus and its undo without a window, since both are AppKit's own animation underneath. +private final class RecordingSearchItem: NSSearchToolbarItem { + + var interactionsBegun = 0 + var interactionsEnded = 0 + + override func beginSearchInteraction() { + interactionsBegun += 1 + super.beginSearchInteraction() + } + + override func endSearchInteraction() { + interactionsEnded += 1 + super.endSearchInteraction() + } +} + +/// **Escape is staged and Return is swallowed** (04-interactions.md ▸ Search, settled) — the two keys +/// the field gives meanings of its own, and the only two: "every key with the field focused acts on +/// the field — stock `NSSearchField` behavior, no pass-throughs". +@MainActor +@Suite("Toolbar ▸ the search field's two keys") +struct BoardSearchFieldKeyTests { + + /// The field as its toolbar home builds it, adopted by an item that records what it is asked to + /// do — `BoardToolbar`'s wiring, with the item swapped for one that can be read. + private func makeToolbarField( + store: BoardStore, + presentation: BoardSearchPresentation + ) -> (NSSearchField, RecordingSearchItem) { + let field = BoardSearchFieldController.makeField( + store: store, + presentation: presentation, + home: .toolbar + ) + let item = RecordingSearchItem(itemIdentifier: .boardSearch) + item.searchField = field + BoardSearchFieldController.adopt(item, presentation: presentation) + return (field, item) + } + + private func send(_ selector: Selector, to field: NSSearchField) -> Bool? { + field.delegate?.control?(field, textView: NSTextView(), doCommandBy: selector) + } + + @Test("A non-empty field takes Escape as a clear, and keeps the keyboard") + func escapeClearsBeforeItLeaves() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let presentation = BoardSearchPresentation() + var handedBack = 0 + presentation.focusBoard = { handedBack += 1 } + let (field, item) = makeToolbarField(store: store, presentation: presentation) + + field.stringValue = "spec" + field.delegate?.controlTextDidChange?( + Notification(name: NSControl.textDidChangeNotification, object: field) + ) + #expect(store.searchQuery == "spec") + + #expect(send(#selector(NSResponder.cancelOperation(_:)), to: field) == true) + + // "In a non-empty field it clears the query, focus staying in the field." + #expect(store.searchQuery.isEmpty) + #expect(field.stringValue.isEmpty) + #expect(handedBack == 0, "one press, one layer") + #expect(item.interactionsEnded == 0, "the field keeps the keyboard, so it keeps its width") + } + + @Test("An empty field takes Escape as an exit — the board gets the keyboard, the field its width") + func escapeHandsTheKeyboardBack() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let presentation = BoardSearchPresentation() + var handedBack = 0 + presentation.focusBoard = { handedBack += 1 } + let (field, item) = makeToolbarField(store: store, presentation: presentation) + + #expect(send(#selector(NSResponder.cancelOperation(_:)), to: field) == true) + + // "In an empty field it returns focus to the board" — and the grown field settles back with + // the keyboard it just gave up. + #expect(handedBack == 1) + #expect(item.interactionsEnded == 1) + } + + @Test("The transient home takes the same Escape with no item to collapse") + func escapeInTheStripHasNoWidthToUndo() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let presentation = BoardSearchPresentation() + var handedBack = 0 + presentation.focusBoard = { handedBack += 1 } + let field = BoardSearchFieldController.makeField( + store: store, + presentation: presentation, + home: .transient + ) + + #expect(send(#selector(NSResponder.cancelOperation(_:)), to: field) == true) + #expect(handedBack == 1, "one implementation of the staircase, whichever home it is in") + } + + @Test("Return is a swallowed no-op, and every other key falls through to the field editor") + func returnIsSwallowedAndTheRestPassThrough() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let presentation = BoardSearchPresentation() + let (field, _) = makeToolbarField(store: store, presentation: presentation) + + field.stringValue = "spec" + field.delegate?.controlTextDidChange?( + Notification(name: NSControl.textDidChangeNotification, object: field) + ) + + // "The filter is live, there is nothing to submit — it never reaches the board's + // rename/create grammar." + #expect(send(#selector(NSResponder.insertNewline(_:)), to: field) == true) + #expect(store.searchQuery == "spec", "a swallowed key changes nothing") + + // "Stock NSSearchField behavior, no pass-throughs": the field editor keeps everything else, + // caret motion and text selection included. + #expect(send(#selector(NSResponder.moveLeft(_:)), to: field) == false) + #expect(send(#selector(NSResponder.deleteBackward(_:)), to: field) == false) + #expect(send(#selector(NSResponder.moveUp(_:)), to: field) == false) + } + + @Test("⌘F expands and focuses in one call — and claims nothing when the field is in no window") + func focusHandleIsTheItemsInteraction() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let presentation = BoardSearchPresentation() + let (_, item) = makeToolbarField(store: store, presentation: presentation) + + let focusField = try #require(presentation.focusField) + #expect(focusField() == false, "a field in no window cannot take the keyboard") + #expect(item.interactionsBegun == 0, "and must not report a focus it never took") + } +}