fix(panel): extensions keyboard nav, tab strip and scroll bars

Tests/StackNudgePanelCoreTests/ExtensionCatalogTests.swift

@@ -251,16 +251,57 @@ final class ExtensionCatalogTests: XCTestCase {
 
     // A selection pointing at a row that has gone would highlight nothing and
     // make Enter a no-op.
-    func testTheSelectionIsDroppedWhenItsRowDisappears() {
+    func testTheSelectionMovesToTheFirstRowWhenItsOwnRowDisappears() {
         let c = catalog()
         c.selectedID = "installed"
         c.reconcileSelection(among: threeRows)
         XCTAssertEqual(c.selectedID, "installed")
 
+        c.reconcileSelection(among: threeRows.filter { $0.id != "installed" })
+        XCTAssertEqual(c.selectedID, "broken", "lands somewhere rather than nowhere")
+    }
+
+    func testWithNoRowsAtAllThereIsNothingToSelect() {
+        let c = catalog()
+        c.selectedID = "installed"
         c.reconcileSelection(among: [])
         XCTAssertNil(c.selectedID)
     }
 
+    // The page used to open with nothing selected, so Return did nothing while
+    // the footer advertised it. The catalogue arrives after the view does, so
+    // seeding has to survive the rows changing under it.
+    func testArrivingSelectsTheFirstRow() {
+        let c = catalog()
+        XCTAssertNil(c.selectedID)
+        c.reconcileSelection(among: threeRows)
+        XCTAssertEqual(c.selectedID, "broken")
+    }
+
+    // Row order is what makes seeding safe to do unprompted: refusals sort
+    // first, then what is installed, and only then what is merely published, so
+    // Return lands on a page rather than on an install nobody asked for.
+    func testTheSeededRowIsNeverAnInstallWhenAnythingIsInstalled() {
+        let c = catalog()
+        c.reconcileSelection(among: threeRows)
+        let seeded = threeRows.first { $0.id == c.selectedID }
+        XCTAssertEqual(seeded?.isInstalled, true)
+    }
+
+    func testCommandArrowsJumpToTheFirstAndLastRow() {
+        let c = catalog()
+        c.selectEdge(among: threeRows, top: false)
+        XCTAssertEqual(c.selectedID, "available")
+        c.selectEdge(among: threeRows, top: true)
+        XCTAssertEqual(c.selectedID, "broken")
+    }
+
+    func testJumpingIsANoOpWithNoRows() {
+        let c = catalog()
+        c.selectEdge(among: [], top: true)
+        XCTAssertNil(c.selectedID)
+    }
+
     // MARK: - Loading
 
     func testLoadingPublishesTheEntries() {
@@ -522,11 +563,15 @@ final class ExtensionCatalogTests: XCTestCase {
     // method existed from the start and nothing but a test ever called it, so a
     // selection could point at a row that is no longer on screen and Enter
     // would silently do nothing.
-    func testAQueryThatHidesTheSelectedRowDropsTheSelection() {
+    //
+    // It moves to what is left rather than to nothing, which was only half the
+    // fix: an empty selection leaves Enter doing exactly the nothing this was
+    // written about, under a footer still advertising it.
+    func testAQueryThatHidesTheSelectedRowMovesItToWhatIsLeft() {
         let c = catalog()
         c.selectedID = "derby"
         c.reconcileSelection(among: ExtensionCatalog.matching(sample, query: "system"))
-        XCTAssertNil(c.selectedID)
+        XCTAssertEqual(c.selectedID, "system")
     }
 
     func testAQueryThatStillShowsTheSelectedRowKeepsIt() {

Tests/StackNudgePanelCoreTests/ExtensionConfigTests.swift

@@ -52,6 +52,126 @@ final class ExtensionConfigTests: XCTestCase {
         XCTAssertFalse(m.values.values.contains { $0.hasPrefix("xoxb-") })
     }
 
+    // ⏎ saves and hands focus back in one keystroke, and a field commits its
+    // value as it loses focus. An unguarded setter wrote the same string back
+    // and cleared the confirmation in the same frame the save set it, so the
+    // only thing on screen that said the save had happened flickered out.
+    func testWritingTheSameValueBackDoesNotClearTheConfirmation() {
+        let m = model(keys: [key("STACKNUDGE_EXT_DERBY_ORG")],
+                      existing: ["STACKNUDGE_EXT_DERBY_ORG": "stackone"])
+        m.save()
+        XCTAssertTrue(m.saved)
+        m.binding(for: key("STACKNUDGE_EXT_DERBY_ORG")).wrappedValue = "stackone"
+        XCTAssertTrue(m.saved, "a commit of the unchanged value is not an edit")
+    }
+
+    func testAnActualEditStillClearsTheConfirmation() {
+        let m = model(keys: [key("STACKNUDGE_EXT_DERBY_ORG")],
+                      existing: ["STACKNUDGE_EXT_DERBY_ORG": "stackone"])
+        m.save()
+        m.binding(for: key("STACKNUDGE_EXT_DERBY_ORG")).wrappedValue = "other"
+        XCTAssertFalse(m.saved)
+        XCTAssertEqual(m.values["STACKNUDGE_EXT_DERBY_ORG"], "other")
+    }
+
+    // MARK: - Keyboard
+
+    // The page opens with no field focused: a focused field is first responder
+    // and takes every key before FloatingPanel.keyDown runs, so the selection
+    // is what ⏎ hands focus to. Unseeded, ⏎ did nothing at all on arrival
+    // while the footer advertised it.
+    func testTheFirstFieldIsSelectedOnArrival() {
+        let m = model(keys: [key("STACKNUDGE_EXT_DERBY_ORG"), key("STACKNUDGE_EXT_DERBY_BASE")])
+        XCTAssertEqual(m.selection, .field("STACKNUDGE_EXT_DERBY_ORG"))
+    }
+
+    // The page draws a back chevron, a Save and a Remove as well as its fields.
+    // A traversal over the fields alone walks past three buttons nobody can
+    // reach, which is what it did.
+    func testTheTraversalCoversEveryControlOnThePage() {
+        let m = model(keys: [key("A"), key("B")])
+        XCTAssertEqual(m.targets, [.back, .field("A"), .field("B"), .save, .remove])
+    }
+
+    // No Save button on a page with nothing to save, so no Save target either.
+    // Back and Remove are on every extension's page, so ↑↓ always do something.
+    func testAnExtensionWithNoKeysStillHasBackAndRemoveToWalk() {
+        let m = model(keys: [])
+        XCTAssertEqual(m.targets, [.back, .remove])
+        XCTAssertEqual(m.selection, .back)
+    }
+
+    func testArrowsWalkEveryTargetAndStopAtTheEnds() {
+        let m = model(keys: [key("A"), key("B")])
+        XCTAssertEqual(m.selection, .field("A"))
+        m.moveSelection(by: -1)
+        XCTAssertEqual(m.selection, .back, "up from the first field reaches the chevron")
+        m.moveSelection(by: -1)
+        XCTAssertEqual(m.selection, .back, "stops rather than wrapping")
+        for _ in 0..<5 { m.moveSelection(by: 1) }
+        XCTAssertEqual(m.selection, .remove, "and stops at the far end too")
+    }
+
+    func testMovingWalksFromAFieldOntoTheButtons() {
+        let m = model(keys: [key("A")])
+        m.moveSelection(by: 1)
+        XCTAssertEqual(m.selection, .save)
+        m.moveSelection(by: 1)
+        XCTAssertEqual(m.selection, .remove)
+    }
+
+    func testCommandArrowsJumpToTheFirstAndLastTarget() {
+        let m = model(keys: [key("A"), key("B")])
+        m.selectEdge(top: false)
+        XCTAssertEqual(m.selection, .remove)
+        m.selectEdge(top: true)
+        XCTAssertEqual(m.selection, .back)
+    }
+
+    // Only a field has anywhere to put focus. ⏎ on a button acts on it
+    // instead, which the controller resolves off this selection.
+    func testTheSelectedKeyIsOnlyAFieldsKey() {
+        let m = model(keys: [key("A")])
+        XCTAssertEqual(m.selectedKey, "A")
+        m.selection = .remove
+        XCTAssertNil(m.selectedKey)
+    }
+
+    // ⏎ hands the selected field first-responder status, which the view
+    // cannot be asked for directly: @FocusState is view state, so the model
+    // raises a request the way ExtensionCatalog does for its search field.
+    func testEnterAsksForTheSelectedFieldToTakeFocus() {
+        let m = model(keys: [key("A")])
+        m.focusSelectedField()
+        XCTAssertEqual(m.fieldFocusRequests, 1)
+    }
+
+    // Nothing to focus while the selection is on a button. The view would
+    // otherwise set focus to nil, which reads as a keystroke that dismissed the
+    // selection rather than one that pressed the button.
+    func testEnterOnAButtonAsksForNoFieldFocus() {
+        let m = model(keys: [key("A")])
+        m.selection = .remove
+        m.focusSelectedField()
+        XCTAssertEqual(m.fieldFocusRequests, 0)
+    }
+
+    func testTheSelectionFallsBackWhenItsTargetIsGone() {
+        let m = model(keys: [key("A"), key("B")])
+        m.selection = .field("GONE")
+        m.reconcileSelection()
+        XCTAssertEqual(m.selection, .back)
+    }
+
+    // The Save target goes with the Save button on an extension declaring
+    // nothing, so a selection carried onto such a page has to move.
+    func testASaveSelectionIsReconciledOnAPageWithNoSaveButton() {
+        let m = model(keys: [])
+        m.selection = .save
+        m.reconcileSelection()
+        XCTAssertEqual(m.selection, .back)
+    }
+
     // MARK: - Saving
 
     func testSavingWritesEveryDeclaredKey() {

Tests/StackNudgePanelCoreTests/FooterHintTests.swift

@@ -457,3 +457,169 @@ final class ExtensionsKeyActionTests: XCTestCase {
         }
     }
 }
+
+// Where each keystroke goes on an extension's own configuration form, and what
+// its footer promises at each of the two levels. Raw virtual key codes, since
+// the panel's KeyCode table is private: 53 Esc, 126/125 up/down, 36 Return,
+// 76 numpad Enter, 48 Tab, 123/124 left/right, 1 "s", 51 delete.
+//
+// The page is a list of text fields, and a focused field is first responder, so
+// it takes every key before FloatingPanel.keyDown runs. That is why the
+// traversal lives one level above the fields: the form used to own Esc and
+// nothing else, which left it reachable only with the mouse or with Tab while
+// the footer advertised Return.
+final class ExtensionConfigKeyActionTests: XCTestCase {
+
+    private func action(_ keyCode: UInt16) -> PanelController.ExtensionConfigKeyAction {
+        PanelController.extensionConfigKeyAction(keyCode: keyCode)
+    }
+
+    func test_escapeStepsBackOffThePage() {
+        XCTAssertEqual(action(53), .back)
+    }
+
+    func test_verticalArrowsMoveBetweenFields() {
+        XCTAssertEqual(action(126), .moveSelection(-1))
+        XCTAssertEqual(action(125), .moveSelection(1))
+    }
+
+    // Return and Tab are both how a macOS form is entered, and at this level
+    // nothing else claims either of them. What they act on is the selection,
+    // which the controller resolves: a field takes focus, a button fires.
+    func test_returnAndTabActOnWhateverIsSelected() {
+        XCTAssertEqual(action(36), .activateSelection)
+        XCTAssertEqual(action(76), .activateSelection)
+        XCTAssertEqual(action(48), .activateSelection)
+    }
+
+    func test_horizontalArrowsDoNothingOnAForm() {
+        XCTAssertEqual(action(123), .swallow)
+        XCTAssertEqual(action(124), .swallow)
+    }
+
+    // Nothing falls through to the Events bindings, which map Return to
+    // approving a permission prompt on a tab this page isn't showing.
+    func test_everyKeyIsAccountedFor() {
+        for code in UInt16(0)...UInt16(130) {
+            _ = action(code)
+        }
+    }
+}
+
+// The same form's footer. Each hint has to be true at the level it appears on:
+// the bar used to advertise Save on a page where Return did nothing until a
+// field had been clicked, and on extensions that declare no fields at all.
+final class ExtensionConfigFooterTests: XCTestCase {
+
+    private func hints(keyCount: Int, editing: Bool,
+                       selection: ExtensionConfigModel.Target? = nil,
+                       valid: Bool = true) -> [FooterHintSpec] {
+        ExtensionConfigView.footerHints(keyCount: keyCount, editing: editing,
+                                        selection: selection, valid: valid)
+    }
+
+    private func labels(_ specs: [FooterHintSpec]) -> [String] { specs.map(\.label) }
+
+    // Every installed extension opens a page, because that is where Remove
+    // lives. One declaring nothing still has the chevron and Remove to walk, so
+    // the traversal is advertised; there is just no Save and no Edit.
+    func test_noKeys_stillOffersTheTraversal() {
+        XCTAssertEqual(labels(hints(keyCount: 0, editing: false, selection: .back)),
+                       ["Move", "Back", "Remove"])
+    }
+
+    // One hint per action. A separate primary hint naming the selected target
+    // printed the bar's own labels twice the moment the ring reached the
+    // chevron: "Back ⏎ · Move · Back Esc".
+    func test_noLabelIsAdvertisedTwice() {
+        for selection: ExtensionConfigModel.Target in [.back, .field("A"), .save, .remove] {
+            let names = labels(hints(keyCount: 2, editing: false, selection: selection))
+            XCTAssertEqual(Set(names).count, names.count, "duplicate in \(names)")
+        }
+    }
+
+    func test_severalKeys_advertiseTheTraversal() {
+        let specs = hints(keyCount: 2, editing: false, selection: .field("A"))
+        XCTAssertEqual(labels(specs), ["Edit", "Move", "Save", "Back", "Remove"])
+        // Both the step and the jump, riding on one label rather than paying
+        // for a second, exactly as the Events bar carries its own.
+        XCTAssertEqual(specs.first { $0.label == "Move" }?.keys, ["↑↓", "⌘↑↓"])
+    }
+
+    // ⏎ rides on the hint for whatever the ring is on, so the bar always says
+    // what the next keystroke will do without repeating itself.
+    func test_returnRidesOnTheSelectedTargetsOwnHint() {
+        func keys(_ selection: ExtensionConfigModel.Target, _ label: String) -> [String]? {
+            hints(keyCount: 2, editing: false, selection: selection)
+                .first { $0.label == label }?.keys
+        }
+        XCTAssertEqual(keys(.field("A"), "Edit"), ["⏎"])
+        XCTAssertEqual(keys(.save, "Save"), ["⏎", "⌘S"])
+        XCTAssertEqual(keys(.remove, "Remove"), ["⏎", "⌘⌫"])
+        XCTAssertEqual(keys(.back, "Back"), ["⏎", "Esc"])
+    }
+
+    // And only there. A hint the ring is not on keeps its own key alone.
+    func test_returnIsNotAdvertisedOnUnselectedTargets() {
+        let specs = hints(keyCount: 2, editing: false, selection: .field("A"))
+        XCTAssertEqual(specs.first { $0.label == "Save" }?.keys, ["⌘S"])
+        XCTAssertEqual(specs.first { $0.label == "Back" }?.keys, ["Esc"])
+        XCTAssertEqual(specs.first { $0.label == "Remove" }?.keys, ["⌘⌫"])
+    }
+
+    // Edit has no key of its own, so it is the one hint that exists only while
+    // the ring is on a field.
+    func test_editAppearsOnlyOnAField() {
+        XCTAssertFalse(labels(hints(keyCount: 2, editing: false, selection: .remove)).contains("Edit"))
+        XCTAssertTrue(labels(hints(keyCount: 2, editing: false, selection: .field("A"))).contains("Edit"))
+    }
+
+    // Inside a field the page has given the keyboard away: Return saves,
+    // Esc hands it back rather than leaving the page.
+    func test_editing_namesWhatTheKeysDoFromInsideAField() {
+        XCTAssertEqual(labels(hints(keyCount: 2, editing: true)),
+                       ["Save", "Next field", "Done", "Remove"])
+    }
+
+    func test_editing_dropsTabWithOnlyOneField() {
+        XCTAssertEqual(labels(hints(keyCount: 1, editing: true)), ["Save", "Done", "Remove"])
+    }
+
+    // ⌘⌫ is a field-editor binding (deleteToBeginningOfLine), so a focused
+    // field takes it before the panel sees it. It dims rather than
+    // disappearing: the bar must not reflow as focus moves.
+    func test_removeDimsWhileEditingRatherThanVanishing() {
+        let idle = hints(keyCount: 1, editing: false).first { $0.label == "Remove" }
+        let busy = hints(keyCount: 1, editing: true).first { $0.label == "Remove" }
+        XCTAssertEqual(idle?.dimmed, false)
+        XCTAssertEqual(busy?.dimmed, true)
+    }
+
+    // ⌘S is not a field-editor binding, so unlike ⌘⌫ it works from inside a
+    // field too; at that level Return is the shorter way to the same thing.
+    func test_saveIsReachableAtBothLevels() {
+        XCTAssertEqual(hints(keyCount: 1, editing: false).first { $0.label == "Save" }?.keys, ["⌘S"])
+        XCTAssertEqual(hints(keyCount: 1, editing: true).first { $0.label == "Save" }?.keys, ["⏎"])
+    }
+
+    // A value the form will refuse gets the same answer from the bar that it
+    // gets from the button, which disables itself. Dimmed rather than dropped:
+    // the hint is real, it just does not apply to what is typed.
+    func test_saveDimsOnAValueTheFormWillRefuse() {
+        for editing in [true, false] {
+            let spec = hints(keyCount: 1, editing: editing, selection: .field("A"), valid: false)
+                .first { $0.label == "Save" }
+            XCTAssertEqual(spec?.dimmed, true, "editing: \(editing)")
+        }
+    }
+
+    // Nothing on this bar may be dropped before the two that navigate it.
+    func test_backAndRemoveAreNeverSheddable() {
+        for editing in [true, false] {
+            for spec in hints(keyCount: 2, editing: editing, selection: .field("A"))
+            where ["Back", "Done", "Remove"].contains(spec.label) {
+                XCTAssertNil(spec.shedOrder, "\(spec.label) must not shed")
+            }
+        }
+    }
+}