diff --git a/Tests/StackNudgePanelCoreTests/ExtensionCatalogTests.swift b/Tests/StackNudgePanelCoreTests/ExtensionCatalogTests.swift index ce0183d..1a11375 100644 --- a/Tests/StackNudgePanelCoreTests/ExtensionCatalogTests.swift +++ b/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() { diff --git a/Tests/StackNudgePanelCoreTests/ExtensionConfigTests.swift b/Tests/StackNudgePanelCoreTests/ExtensionConfigTests.swift index f83c6c9..290f0f0 100644 --- a/Tests/StackNudgePanelCoreTests/ExtensionConfigTests.swift +++ b/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() { diff --git a/Tests/StackNudgePanelCoreTests/FooterHintTests.swift b/Tests/StackNudgePanelCoreTests/FooterHintTests.swift index 101e987..a9a0abc 100644 --- a/Tests/StackNudgePanelCoreTests/FooterHintTests.swift +++ b/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") + } + } + } +} diff --git a/Tests/StackNudgePanelCoreTests/PanelKeyRoutingTests.swift b/Tests/StackNudgePanelCoreTests/PanelKeyRoutingTests.swift index 40c3a35..330a1b0 100644 --- a/Tests/StackNudgePanelCoreTests/PanelKeyRoutingTests.swift +++ b/Tests/StackNudgePanelCoreTests/PanelKeyRoutingTests.swift @@ -22,7 +22,8 @@ final class PanelKeyRoutingTests: XCTestCase { // checks, which is how extensionTab slipped through the first version. private let allModes: [PanelMode] = [ .events, .sessions, .usage, .outcomes, .extensionTab("any"), - .settings, .phrases, .updateConfirm, .updating, .postUpdate, + .settings, .phrases, .extensions, .extensionConfig("any"), + .updateConfirm, .updating, .postUpdate, .bootstrap, .uninstall, ] @@ -38,4 +39,54 @@ final class PanelKeyRoutingTests: XCTestCase { XCTAssertFalse(PanelController.eventsOwnsKeyboard(.extensionTab(id))) } } + + // Both Settings sub-pages carry an id or sit off the tab strip, which is + // how .extensionTab was missed from this list the first time round. + func testTheExtensionSubPagesNeverOwnThemEither() { + for id in ["derby", "", "events"] { + XCTAssertFalse(PanelController.eventsOwnsKeyboard(.extensionConfig(id))) + } + XCTAssertFalse(PanelController.eventsOwnsKeyboard(.extensions)) + } +} + +// Which tab the strip scrolls to keep in view. The strip is the one row in the +// panel whose width is decided by what the user installs, so it scrolls, and a +// scroll target that names something the strip never renders does nothing at all +// while looking like it works. +final class TabStripAnchorTests: XCTestCase { + + private let tabs: [PanelMode] = [ + .events, .sessions, .usage, .outcomes, + .extensionTab("derby"), .extensionTab("system"), .settings, + ] + + func testATabIsItsOwnAnchor() { + for tab in tabs { + XCTAssertEqual(PanelContentView.tabStripAnchor(for: tab, in: tabs), tab) + } + } + + // Settings' sub-pages draw the strip but are not in it, so the mode itself + // is an id nothing renders. Its tab is what should stay on screen while you + // are a level inside it. + func testSettingsSubPagesAnchorOnTheSettingsTab() { + for mode: PanelMode in [.phrases, .extensions, .extensionConfig("derby"), + .updateConfirm, .uninstall] { + XCTAssertEqual(PanelContentView.tabStripAnchor(for: mode, in: tabs), .settings) + } + } + + // The full-screen takeovers draw no strip at all. + func testTakeoverModesAnchorNowhere() { + for mode: PanelMode in [.updating, .postUpdate, .bootstrap] { + XCTAssertNil(PanelContentView.tabStripAnchor(for: mode, in: tabs)) + } + } + + // An extension removed while its tab was open leaves the mode naming a tab + // that is gone. Scrolling to it would be scrolling to nothing. + func testATabThatIsNoLongerInTheStripAnchorsNowhere() { + XCTAssertNil(PanelContentView.tabStripAnchor(for: .extensionTab("gone"), in: tabs)) + } } diff --git a/panel/Components.swift b/panel/Components.swift index 095d7f1..c5fe66d 100644 --- a/panel/Components.swift +++ b/panel/Components.swift @@ -292,6 +292,33 @@ struct ThinScrollers: NSViewRepresentable { } } +// Take the horizontal scroller off a ScrollView entirely. +// +// `.scrollIndicators(.hidden)` is not enough: with "Show scroll bars: Always" +// set in System Settings the scroller is a legacy inset NSScroller, which claims +// a row of its own. On the tab strip that is a full-width bar under the tabs and +// several points of height in a header that has about twenty to give. +// +// Same superview walk as ThinScrollers, for the same reason: SwiftUI exposes no +// handle on the NSScrollView it makes. +struct NoHorizontalScroller: NSViewRepresentable { + func makeNSView(context: Context) -> NSView { NSView(frame: .zero) } + func updateNSView(_ nsView: NSView, context: Context) { + DispatchQueue.main.async { + var current: NSView? = nsView + while let view = current { + if let scrollView = view as? NSScrollView { + scrollView.scrollerStyle = .overlay + scrollView.hasHorizontalScroller = false + scrollView.autohidesScrollers = true + return + } + current = view.superview + } + } + } +} + // Collapse the focused field editor's selection to a caret at its end. // // AppKit selects a field's entire contents when it becomes first responder, diff --git a/panel/ExtensionConfigView.swift b/panel/ExtensionConfigView.swift index 56a4c0c..dfa0586 100644 --- a/panel/ExtensionConfigView.swift +++ b/panel/ExtensionConfigView.swift @@ -32,6 +32,50 @@ final class ExtensionConfigModel: ObservableObject { // screen rather than to whatever was saved a minute ago. @Published private(set) var saved = false + // Everything on the page the keyboard can land on, in the order it is drawn. + // Not just the fields: the page renders a back chevron, a Save and a Remove, + // and a traversal that walks past three buttons nobody can reach is the + // thing that makes a keyboard-native panel feel half-wired. Each has a + // shortcut of its own as well, which is what they had instead. + enum Target: Equatable { + case back + case field(String) + case save + case remove + } + + // Where the keyboard is while no field has focus. A focused text field is + // first responder and takes ↑↓ before FloatingPanel.keyDown ever sees them, + // so the traversal has to live at the level above the fields, exactly as the + // browser's does above its search field. Seeded rather than left nil: + // arriving with nothing selected makes the footer describe a key that does + // nothing. + @Published var selection: Target? + // Bumped to hand the selected field first-responder status, mirroring + // ExtensionCatalog.searchFocusRequests. The page deliberately opens + // *unfocused* so ⌘⌫, ↑↓ and Esc all work on arrival; ⏎ hands over. + @Published private(set) var fieldFocusRequests = 0 + + // Drawing order, which is also the order ↑↓ walk. Save is absent on an + // extension declaring no keys, because there is no Save button on that page + // either; back and remove are always there, so the list is never empty and + // ↑↓ always do something. + var targets: [Target] { + [.back] + keys.map { Target.field($0.key) } + (keys.isEmpty ? [] : [.save]) + [.remove] + } + + // The field the selection is on, if it is on one. The view needs this to + // drive @FocusState, which is keyed by the config key. + var selectedKey: String? { + if case .field(let key) = selection { return key } + return nil + } + + func focusSelectedField() { + guard selectedKey != nil else { return } + fieldFocusRequests += 1 + } + private let persist: (String, String?) -> Void private let didChange: () -> Void let onRemove: () -> Void @@ -55,6 +99,32 @@ final class ExtensionConfigModel: ObservableObject { var seeded: [String: String] = [:] for key in row.configurableKeys { seeded[key.key] = existing[key.key] ?? "" } values = seeded + // The first field where there is one, so ⏎ on arrival starts editing + // rather than walking back out of the page. + selection = row.configurableKeys.first.map { .field($0.key) } ?? .back + } + + // MARK: - Keyboard + + // Clamps rather than wrapping, like every other list in the panel. + func moveSelection(by delta: Int) { + let all = targets + let current = all.firstIndex { $0 == selection } + let next = current.map { min(max($0 + delta, 0), all.count - 1) } + ?? (delta > 0 ? 0 : all.count - 1) + selection = all[next] + } + + func selectEdge(top: Bool) { + selection = top ? targets.first : targets.last + } + + // Keeps the selection on something this page still draws. The model is + // rebuilt per visit so this cannot drift today, but a form that grew its + // fields from anywhere other than the constructor would dangle here first. + func reconcileSelection() { + guard let selection, !targets.contains(selection) else { return } + self.selection = targets.first } // An unset key is removed rather than written empty: the runtime treats a @@ -70,9 +140,16 @@ final class ExtensionConfigModel: ObservableObject { func binding(for key: ExtensionManifest.ConfigKey) -> Binding { Binding(get: { [weak self] in self?.values[key.key] ?? "" }, + // Only a real change clears the confirmation. A field commits + // its value when it loses focus, and ⏎ both saves and hands + // focus back, so an unguarded setter wrote the same string + // straight back and cleared "Saved" in the same frame it was + // set: the save happened, and the only thing that said so + // flickered out of existence. set: { [weak self] newValue in - self?.values[key.key] = newValue - self?.saved = false + guard let self, self.values[key.key] != newValue else { return } + self.values[key.key] = newValue + self.saved = false }) } @@ -146,65 +223,175 @@ struct ExtensionConfigView: View { @ObservedObject var model: ExtensionConfigModel let onBack: () -> Void - // ⌘⌫ is a standard field-editor binding (deleteToBeginningOfLine), so a - // focused field takes it before FloatingPanel.keyDown ever sees it. Rather - // than advertise a key that stops working the moment anyone clicks into a - // field, the hint dims — the same treatment the Settings footer gives its - // Cycle hint on rows where the arrows do nothing. - @FocusState private var fieldFocused: Bool + // Which field is first responder, or nil while the page itself owns the + // keyboard. A String? rather than the Bool this started as: with one Bool + // bound to every field there is no way to say *which* field to focus, so + // the form could only be entered with the mouse or with Tab, and it opened + // with nothing focused at all, which left ⏎ advertised and dead. + @FocusState private var focusedKey: String? + + // Two levels, the shape the browser uses above its search field and the + // Usage tab above its detail. Level one owns ↑↓, ⏎ and ⌘⌫; level two is a + // focused field, which takes every key before FloatingPanel.keyDown runs. + private var editing: Bool { focusedKey != nil } var body: some View { VStack(alignment: .leading, spacing: 0) { header Divider().opacity(0.4) - ScrollView { - VStack(alignment: .leading, spacing: 14) { - summary - if model.keys.isEmpty { - // Not a dead page. Every installed extension opens one, - // because this is where Remove lives and a row that - // opened nothing would make Enter mean something - // different depending on the extension. - Text("This extension has no settings.") - .font(.caption).foregroundStyle(.secondary) - } else { - ForEach(model.keys, id: \.key) { field($0) } + ScrollViewReader { proxy in + ScrollView { + VStack(alignment: .leading, spacing: 14) { + summary + if model.keys.isEmpty { + // Not a dead page. Every installed extension opens + // one, because this is where Remove lives and a row + // that opened nothing would make Enter mean + // something different depending on the extension. + Text("This extension has no settings.") + .font(.caption).foregroundStyle(.secondary) + } else { + ForEach(model.keys, id: \.key) { field($0) } + } + footerRow + } + .padding(.horizontal, 14) + .padding(.vertical, 12) + // On the content, not on the ScrollView: ThinScrollers walks + // superviews for the NSScrollView, as every other call site + // in this tree does. + .background(ThinScrollers()) + } + // Nearest-edge, matching the Settings detail pane: centring + // re-scrolls the whole form on every ↑/↓ to park the selection + // mid-pane, so fields slide under the cursor in a form tall + // enough to scroll and sit still in one that isn't. Without any + // of this the highlight moved somewhere the user could not see, + // which at the panel's 260pt minimum is the second field on. + .onChange(of: model.selection) { target in + guard let anchor = Self.anchor(for: target) else { return } + withAnimation(.easeOut(duration: 0.15)) { + proxy.scrollTo(anchor, anchor: nil) } - footerRow } - .padding(.horizontal, 14) - .padding(.vertical, 12) - // On the content, not on the ScrollView: ThinScrollers walks - // superviews for the NSScrollView, as every other call site - // in this tree does. - .background(ThinScrollers()) } PageFooter { - FooterHint(label: "Back", keys: ["Esc"]) - // Only where there is something to save. This page now opens - // for every installed extension, including ones declaring no - // keys — and a refused one, which is the most common case of - // all — so an unconditional Save hint promised a key that had - // no button and no handler behind it. - if !model.keys.isEmpty { - FooterHint(label: "Save", keys: ["⏎"]) - } - FooterHint(label: "Remove", keys: ["⌘⌫"]) - .opacity(fieldFocused ? 0.35 : 1) + let hints = Self.footerHints(keyCount: model.keys.count, + editing: editing, + selection: model.selection, + valid: model.isValid) + ForEach(hints.indices, id: \.self) { FooterHintRow(spec: hints[$0]) } } } .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + .onAppear { + // Deliberately NOT focused, for the reason the browser states on its + // own search field: a focused field is first responder and + // FloatingPanel.keyDown only fires for what the first responder + // declines, so focusing on arrival hands the field Esc and ⌘⌫ and + // leaves both hints describing keys that no longer do anything. ⏎ + // hands over. + focusedKey = nil + model.reconcileSelection() + } + .onChange(of: model.fieldFocusRequests) { _ in focusedKey = model.selectedKey } + // A click into a field is a selection too, or ↑/↓ after Esc would resume + // from wherever the keyboard last was rather than from the field in + // front of you; the same disagreement between mouse and keyboard the + // Settings cards fixed by moving the index on tap. + .onChange(of: focusedKey) { key in + if let key { model.selection = .field(key) } + } + } + + // The back chevron sits above the scroller and needs no anchor of its own; + // scrolling to the first field is what brings it into view. + static let saveAnchor = "extension-config-save" + static let removeAnchor = "extension-config-remove" + + static func anchor(for target: ExtensionConfigModel.Target?) -> String? { + switch target { + case .field(let key): return key + case .save: return saveAnchor + case .remove: return removeAnchor + case .back, nil: return nil + } + } + + // The bar as data, so the two levels can be asserted rather than read. Every + // hint is true at the level it appears on. ⌘⌫ is the exception that has to + // stay on both: it is a field-editor binding (deleteToBeginningOfLine) and a + // focused field takes it first, so it dims rather than disappearing; the + // bar must not reflow as focus moves. + static func footerHints(keyCount: Int, + editing: Bool, + selection: ExtensionConfigModel.Target? = nil, + valid: Bool = true) -> [FooterHintSpec] { + var hints: [FooterHintSpec] = [] + if editing { + // Dimmed on a value the form will refuse, which is the same answer + // the Save button gives by disabling itself. save() guards too: a + // form that silently wrote a rejected value would be worse than one + // that does nothing. + hints.append(FooterHintSpec(label: "Save", keys: ["⏎"], + primary: true, dimmed: !valid)) + if keyCount > 1 { + hints.append(FooterHintSpec(label: "Next field", keys: ["⇥"], shedOrder: 0)) + } + hints.append(FooterHintSpec(label: "Done", keys: ["Esc"])) + } else { + // One hint per action, with ⏎ added to whichever the ring is on. + // The alternative, a separate primary hint naming the selected + // target, prints the bar's own labels twice: "Back ⏎ · Move · Back + // Esc" the moment the selection reaches the chevron. + // + // Only a field has no key of its own, so it is the only one that + // needs a hint conjured for it. + if case .field = selection { + hints.append(FooterHintSpec(label: "Edit", keys: ["⏎"], primary: true)) + } + // Always more than one target: back and Remove are on every page, + // whatever the extension declares. + hints.append(FooterHintSpec(label: "Move", keys: ["↑↓", "⌘↑↓"])) + // The only way to commit an edit backed out of with Esc, and the + // only Save at all once the field has given focus back. Level one + // only: a focused field swallows ⌘S the way it swallows ⌘⌫, which + // is why the editing bar names ⏎ instead. + if keyCount > 0 { + hints.append(FooterHintSpec(label: "Save", + keys: selection == .save ? ["⏎", "⌘S"] : ["⌘S"], + primary: selection == .save, + dimmed: !valid, + shedOrder: selection == .save ? nil : 1)) + } + hints.append(FooterHintSpec(label: "Back", + keys: selection == .back ? ["⏎", "Esc"] : ["Esc"], + primary: selection == .back)) + } + let removeSelected = selection == .remove && !editing + hints.append(FooterHintSpec(label: "Remove", + keys: removeSelected ? ["⏎", "⌘⌫"] : ["⌘⌫"], + primary: removeSelected, + dimmed: editing)) + return hints } private var header: some View { - HStack(spacing: 8) { + let selected = model.selection == .back && !editing + return HStack(spacing: 8) { Button(action: onBack) { HStack(spacing: 4) { Image(systemName: "chevron.left").font(.caption.weight(.semibold)) Text(model.backLabel).font(.caption) } - .foregroundStyle(.secondary) + .foregroundStyle(selected ? Color.primary : .secondary) + .padding(.horizontal, 6) + .padding(.vertical, 3) + .background(RoundedRectangle(cornerRadius: 6, style: .continuous) + .fill(Color.accentColor.opacity(selected ? 0.18 : 0))) + .overlay(RoundedRectangle(cornerRadius: 6, style: .continuous) + .strokeBorder(Color.accentColor.opacity(selected ? 0.6 : 0), lineWidth: 1.5)) } .buttonStyle(.plain) @@ -242,18 +429,29 @@ struct ExtensionConfigView: View { } private func field(_ key: ExtensionManifest.ConfigKey) -> some View { - VStack(alignment: .leading, spacing: 4) { + // Selected is the keyboard's position while nothing is focused. Once a + // field is first responder the field's own ring says where you are, and + // a second highlight behind it reads as two cursors. + let selected = model.selection == .field(key.key) && !editing + return VStack(alignment: .leading, spacing: 4) { Text(key.displayLabel).font(.caption.weight(.medium)) TextField(key.placeholder ?? "", text: model.binding(for: key)) .textFieldStyle(.roundedBorder) .font(.caption) - .focused($fieldFocused) - .onSubmit { model.save() } + .focused($focusedKey, equals: key.key) + // Saves and hands the keyboard back, so ↑↓ and ⌘⌫ work again + // without a separate keystroke. The browser's search field + // releases on Enter for the same reason. + .onSubmit { + model.save() + focusedKey = nil + } // A focused field is first responder, and FloatingPanel.keyDown - // only fires for what the first responder declines — so without - // this, Esc stopped going back the moment anyone clicked into a - // field, while the footer went on advertising it. - .onExitCommand { onBack() } + // only fires for what the first responder declines, so without + // this, Esc did nothing at all from inside a field. It steps out + // to the page rather than off it: two levels, two Escs, exactly + // as the browser's search field and the history filter behave. + .onExitCommand { focusedKey = nil } if let problem = model.problem(for: key) { Text(problem).font(.caption2).foregroundStyle(.orange) .fixedSize(horizontal: false, vertical: true) @@ -265,14 +463,31 @@ struct ExtensionConfigView: View { // a reviewer reading the extension's PR sees the same name here. Text(key.key).font(.system(size: 9).monospaced()).foregroundStyle(.quaternary) } + .padding(.horizontal, 8) + .padding(.vertical, 6) + .background(RoundedRectangle(cornerRadius: 8, style: .continuous) + .fill(Color.accentColor.opacity(selected ? 0.12 : 0))) + .overlay(RoundedRectangle(cornerRadius: 8, style: .continuous) + .strokeBorder(Color.accentColor.opacity(selected ? 0.6 : 0), lineWidth: 1.5)) + // No tap gesture on the container, and no combined accessibility + // element: both would sit between the click and the text field inside + // it. The mouse already picks a field by focusing it, which syncs the + // selection back through onChange(of: focusedKey). + // + // The scroll anchor. Keyed by the config key because that is what the + // selection is keyed by; an index would go stale against a form whose + // fields came from a manifest. + .id(key.key) } private var footerRow: some View { HStack(spacing: 8) { if !model.keys.isEmpty { - CardButton(title: "Save", prominent: true, enabled: model.isValid) { + CardButton(title: "Save", prominent: true, enabled: model.isValid, + selected: model.selection == .save && !editing) { model.save() } + .id(Self.saveAnchor) if model.saved { Text("Saved").font(.caption2).foregroundStyle(.secondary) } @@ -280,7 +495,9 @@ struct ExtensionConfigView: View { Spacer() // Here rather than on the Settings card, so Enter on an installed // extension opens it instead of deleting it. - CardButton(title: "Remove") { model.onRemove() } + CardButton(title: "Remove", + selected: model.selection == .remove && !editing) { model.onRemove() } + .id(Self.removeAnchor) } .padding(.top, 6) } diff --git a/panel/ExtensionTabView.swift b/panel/ExtensionTabView.swift index 27235d6..18819d4 100644 --- a/panel/ExtensionTabView.swift +++ b/panel/ExtensionTabView.swift @@ -81,8 +81,16 @@ struct ExtensionTabView: View { } .padding(.horizontal, 12) .padding(.vertical, 8) + // On the content, not on the ScrollView, which is where + // this used to be. ThinScrollers walks *superviews* for + // the NSScrollView, so out there it is a sibling of the + // scroll view rather than a descendant: the walk starts + // above the thing it is looking for and never finds it. + // It silently did nothing, which left an extension's tab + // as the one page in the panel still drawing the system's + // full-width scroller. + .background(ThinScrollers()) } - .background(ThinScrollers()) // Keyboard selection has to bring its row with it; ↑/↓ past // the fold otherwise move an invisible highlight, and ⏎ acts // on a row the user can't see. Every other list pane here diff --git a/panel/ExtensionsBrowser.swift b/panel/ExtensionsBrowser.swift index 9f1762b..c280f4b 100644 --- a/panel/ExtensionsBrowser.swift +++ b/panel/ExtensionsBrowser.swift @@ -176,11 +176,25 @@ final class ExtensionCatalog: ObservableObject { install(entry) } - // Keeps the selection on a row that still exists after a reload or a - // removal, rather than pointing at nothing. + // ⌘↑↓, which every other list page in the panel answers. + func selectEdge(among rows: [ExtensionRow], top: Bool) { + guard !rows.isEmpty else { return } + selectedID = top ? rows.first?.id : rows.last?.id + } + + // Keeps the selection on a row that still exists after a reload, a removal + // or a query, and puts it on the first row when there is nothing valid to + // keep. Dropping to nil was half the job: it left the page with a footer + // advertising ⏎ against no selection, which is also how it opened: the + // catalogue arrives after the view does, so there was nothing to select at + // onAppear and nothing selected it afterwards either. + // + // Row order is what makes seeding safe to do unprompted: refusals sort + // first, then what is installed, and only then what is merely published. On + // any machine with an extension on it ⏎ lands on a page, not an install. func reconcileSelection(among rows: [ExtensionRow]) { - guard let selectedID else { return } - if !rows.contains(where: { $0.id == selectedID }) { self.selectedID = nil } + if let selectedID, rows.contains(where: { $0.id == selectedID }) { return } + selectedID = rows.first?.id } // Pure, so the matching rule is testable without a view. @@ -342,16 +356,29 @@ struct ExtensionsView: View { header searchField Divider().opacity(0.4) - ScrollView { - VStack(alignment: .leading, spacing: 8) { - catalogueBody(rows) + ScrollViewReader { proxy in + ScrollView { + VStack(alignment: .leading, spacing: 8) { + catalogueBody(rows) + } + .padding(.horizontal, 14) + .padding(.vertical, 12) + // On the content, not on the ScrollView: ThinScrollers walks + // superviews for the NSScrollView, and every other call site in + // the tree attaches it this way. + .background(ThinScrollers()) + } + // Nearest-edge, matching the Settings detail pane. Cards here + // carry a description, a "Reads …" line and a "Needs …" line, + // so about two of them fit at the panel's 260pt minimum, and + // without this ↑↓ moved a highlight straight off the bottom of + // a catalogue of any size, which is the whole page. + .onChange(of: catalog.selectedID) { id in + guard let id else { return } + withAnimation(.easeOut(duration: 0.15)) { + proxy.scrollTo(id, anchor: nil) + } } - .padding(.horizontal, 14) - .padding(.vertical, 12) - // On the content, not on the ScrollView: ThinScrollers walks - // superviews for the NSScrollView, and every other call site in - // the tree attaches it this way. - .background(ThinScrollers()) } PageFooter { @@ -359,8 +386,15 @@ struct ExtensionsView: View { // there is a query to clear first. FooterHint(label: catalog.query.isEmpty ? "Back" : "Clear", keys: ["Esc"]) FooterHint(label: "Search", keys: ["/"]) - FooterHint(label: "Select", keys: ["↑", "↓"]) + // Dimmed rather than dropped when a query, or an empty + // catalogue, leaves nothing to walk: the bar must not reflow as + // the list filters, and an advertised key that does nothing is + // the thing this page kept doing. Same treatment the Settings + // footer gives its Cycle hint. + FooterHint(label: "Select", keys: ["↑↓", "⌘↑↓"]) + .opacity(rows.isEmpty ? 0.35 : 1) FooterHint(label: activationLabel(in: rows), keys: ["⏎"]) + .opacity(rows.isEmpty ? 0.35 : 1) // Only where Enter is busy doing something else. An installed // row with an update pending takes Enter for the update, so // without this there would be no keyboard route to its page. @@ -375,6 +409,10 @@ struct ExtensionsView: View { .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) .onAppear { catalog.loadIfNeeded() + // Whatever is already on disk. The catalogue lands later and the + // onChange below seeds again from it, but an installed or refused + // extension is selectable from the first frame. + catalog.reconcileSelection(among: rows) // Deliberately NOT focused. A focused field is first responder, and // FloatingPanel.keyDown only fires for what the first responder // declines — so focusing on arrival handed the field Esc, ↑↓ and ⏎ @@ -495,7 +533,12 @@ struct ExtensionsView: View { } ForEach(rows) { row in extensionRow(row) } } - case .idle, .loading where rows.isEmpty: + // A where clause binds to the pattern it follows, not to the list, so + // this read as ".idle, or .loading with nothing to show"; and .idle is + // the state the first frame renders in, before onAppear has started the + // fetch. An extension already on disk was hidden behind "Looking for + // extensions…" for that frame. + case .idle where rows.isEmpty, .loading where rows.isEmpty: // Only when there is nothing to show. Reloading a populated // catalogue used to replace the whole list with this, while the // header spinner — which is the actual reload indicator — was @@ -582,6 +625,8 @@ struct ExtensionsView: View { lineWidth: 1.5)) .contentShape(Rectangle()) .onTapGesture { catalog.selectedID = row.id } + // The scroll anchor, keyed by what the selection is keyed by. + .id(row.id) .accessibilityElement(children: .combine) .accessibilityAddTraits(catalog.selectedID == row.id ? [.isSelected] : []) } @@ -639,6 +684,11 @@ struct CardButton: View { let title: String var prominent = false var enabled = true + // Where the keyboard is, on a page whose ↑↓ walk the buttons as well as the + // fields. A ring rather than a deeper fill: the prominent variant is already + // accent-filled and cannot deepen its own fill legibly, which is the same + // reason the Settings "Set up" banner button rings instead. + var selected = false let action: () -> Void var body: some View { @@ -649,6 +699,8 @@ struct CardButton: View { .padding(.vertical, 5) .background(RoundedRectangle(cornerRadius: 6) .fill(prominent ? Color.accentColor.opacity(0.9) : Color.primary.opacity(0.08))) + .overlay(RoundedRectangle(cornerRadius: 6) + .strokeBorder(Color.primary.opacity(selected ? 0.7 : 0), lineWidth: 2)) .foregroundStyle(prominent ? Color.white : Color.primary) } .buttonStyle(.plain) diff --git a/panel/Panel.swift b/panel/Panel.swift index ca5bd44..fbd73f3 100644 --- a/panel/Panel.swift +++ b/panel/Panel.swift @@ -78,6 +78,19 @@ final class FloatingPanel: NSPanel { } } +// Widths of the tab strip's scroller and of the tabs inside it. Two keys rather +// than one because the answer is a comparison of the two, and a single key would +// have the last writer win. +private struct TabsContentWidth: PreferenceKey { + static var defaultValue: CGFloat = 0 + static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { value = nextValue() } +} + +private struct TabsViewportWidth: PreferenceKey { + static var defaultValue: CGFloat = 0 + static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { value = nextValue() } +} + struct PanelContentView: View { @ObservedObject var store: EventStore @@ -90,6 +103,12 @@ struct PanelContentView: View { // needs the config file and the host, so the controller owns it and the // view just asks. let onConfigureExtension: (ExtensionRow) -> Void + // Leaving one of the Settings sub-pages. Owned by the controller for the + // same reason: both exits leave Settings a note about which row to resume + // the keyboard on, and the chevron here and Esc in the key handler have to + // do the identical thing. + let onCloseExtensionsBrowser: () -> Void + let onCloseExtensionConfig: () -> Void // The disk-backed name store. Observed here (rather than inside EventRow) // so resolving a nudge's session label stays a render-time lookup that @@ -102,6 +121,13 @@ struct PanelContentView: View { // (nav.historySearchFocused) and the actual first responder can't drift. @FocusState private var historyFieldFocused: Bool + // Measured, not guessed: whether the tab strip has more tabs than fit is a + // function of the installed extensions, their label lengths and the panel's + // width, and the strip only fades its edges when there is something past + // them. See scrollingTabs. + @State private var tabsContentWidth: CGFloat = 0 + @State private var tabsViewportWidth: CGFloat = 0 + let onGrantPermissions: () -> Void var body: some View { @@ -163,14 +189,14 @@ struct PanelContentView: View { ExtensionsView(catalog: extensionCatalog, host: extensions, onConfigure: onConfigureExtension, - onBack: { nav.mode = .settings }) + onBack: onCloseExtensionsBrowser) // .id(id) for the same reason .extensionTab has one: the // associated value isn't part of a ViewBuilder case's identity, // so two extensions' forms would share one view and the second // would open showing the first's values. case .extensionConfig(let id): if let model = nav.extensionConfig, model.id == id { - ExtensionConfigView(model: model) { nav.mode = model.origin } + ExtensionConfigView(model: model, onBack: onCloseExtensionConfig) .id(id) } else { // Unreachable in practice — the model is set before the @@ -179,7 +205,7 @@ struct PanelContentView: View { ExtensionsView(catalog: extensionCatalog, host: extensions, onConfigure: onConfigureExtension, - onBack: { nav.mode = .settings }) + onBack: onCloseExtensionsBrowser) } case .updateConfirm: UpdateConfirmView( @@ -248,29 +274,134 @@ struct PanelContentView: View { Image(nsImage: MenuBarController.brandMarkImage(height: 14)) .padding(.trailing, 4) - // Driven off orderedTabs rather than listed here, so the strip and - // the shortcuts can't disagree about what sits where. - ForEach(nav.orderedTabs, id: \.self) { mode in - tab(mode, label: tabLabel(mode), count: tabCount(mode), - dotColor: tabDot(mode)) - } - - Spacer() + scrollingTabs + // Pinned outside the scroller, and sized before it: an extension + // brings its own tab, so the strip is the one row in the panel whose + // width is set by something the user installs. Inside the scroller + // they would scroll away with the tabs, and the mute state is not + // something to have to go looking for. muteBell + .layoutPriority(1) - // One combined hint instead of per-tab keycaps — keeps the strip - // uncluttered while still surfacing the shortcut range. + // One combined hint instead of per-tab keycaps, which keeps the + // strip uncluttered while still surfacing the shortcut range. HStack(spacing: 2) { KeyCapView(symbol: "⌘") KeyCapView(symbol: "1-\(min(nav.orderedTabs.count, PanelNav.maxNumberedTabs))") } .opacity(0.7) + .layoutPriority(1) } .padding(.horizontal, 12) .padding(.vertical, 8) } + // The tabs themselves, which scroll sideways once they stop fitting. + // + // They used to be bare in the strip's HStack, which has no way to say "too + // many": at the panel's 560pt minimum width the labels wrapped mid-word + // ("Ses-sions", "Out-come s") and the tabs past the trailing edge were + // unreachable by mouse. ⌘←→ still stepped onto them, which meant selecting + // a tab nobody could see. + private var scrollingTabs: some View { + ScrollViewReader { proxy in + ScrollView(.horizontal) { + HStack(spacing: 4) { + // Driven off orderedTabs rather than listed here, so the + // strip and the shortcuts can't disagree about what sits + // where. + ForEach(nav.orderedTabs, id: \.self) { mode in + tab(mode, label: tabLabel(mode), count: tabCount(mode), + dotColor: tabDot(mode)) + .id(mode) + } + } + // The selected tab's fill runs to the edge of its own bounds, so + // without this the first and last lose a pixel of it to the clip. + .padding(.horizontal, 1) + .background(widthReader(TabsContentWidth.self)) + // On the content, not the ScrollView: the helper walks + // superviews for the NSScrollView, as ThinScrollers does. + .background(NoHorizontalScroller()) + } + .scrollIndicators(.hidden) + .background(widthReader(TabsViewportWidth.self)) + .onPreferenceChange(TabsContentWidth.self) { tabsContentWidth = $0 } + .onPreferenceChange(TabsViewportWidth.self) { tabsViewportWidth = $0 } + .mask(tabsMask) + // ⌘1-9 and ⌘←→ move the selection without touching the scroll + // offset, so a tab picked by keyboard could sit off the edge; the + // same reason the Settings sidebar scrolls to its category. + .onChange(of: nav.mode) { mode in + guard let anchor = Self.tabStripAnchor(for: mode, in: nav.orderedTabs) else { return } + withAnimation(.easeOut(duration: 0.15)) { proxy.scrollTo(anchor, anchor: nil) } + } + .onAppear { + guard let anchor = Self.tabStripAnchor(for: nav.mode, in: nav.orderedTabs) else { return } + proxy.scrollTo(anchor, anchor: nil) + } + } + // Greedy in its own axis, so without this it takes the width the bell + // and the keycaps need. They carry the layout priority; this yields it. + .layoutPriority(-1) + } + + private var tabsOverflow: Bool { tabsContentWidth > tabsViewportWidth + 1 } + + // Narrower than the narrowest tab, so a tab is never hidden by the thing + // whose job is to say a tab is hidden. + private static let tabFadeWidth: CGFloat = 12 + + // The affordance, and the only one there is: a strip that simply clipped + // gave no sign that a tab was past the edge, which is the state the panel is + // in on any machine with a few extensions on it. + // + // Both edges, because macOS 13 gives no scroll offset to read and guessing + // which side the hidden tabs are on would be wrong half the time. Fixed + // width rather than a fraction of the strip: a proportional fade grows with + // the panel, and at full width it would reach across a whole tab. + // + // Collapses to nothing when everything fits, so a panel showing all its tabs + // has no fade eating the first one's leading edge. + private var tabsMask: some View { + HStack(spacing: 0) { + fadeEdge(leading: true) + Rectangle().fill(.black) + fadeEdge(leading: false) + } + } + + private func fadeEdge(leading: Bool) -> some View { + LinearGradient(colors: leading ? [.clear, .black] : [.black, .clear], + startPoint: .leading, endPoint: .trailing) + .frame(width: tabsOverflow ? Self.tabFadeWidth : 0) + } + + private func widthReader(_ key: Key.Type) -> some View + where Key.Value == CGFloat { + GeometryReader { geometry in + Color.clear.preference(key: key, value: geometry.size.width) + } + } + + // Which tab the strip should keep in view for a mode. Settings' own + // sub-pages are not tabs, so scrolling to the mode itself targets an id the + // strip never renders and silently does nothing; the tab they belong to is + // what should stay visible while you are inside one. + static func tabStripAnchor(for mode: PanelMode, in tabs: [PanelMode]) -> PanelMode? { + if tabs.contains(mode) { return mode } + switch mode { + case .phrases, .extensions, .extensionConfig, .updateConfirm, .uninstall: + return tabs.contains(.settings) ? .settings : nil + // No strip is drawn in these, so there is nothing to bring into view. + case .updating, .postUpdate, .bootstrap: + return nil + default: + return nil + } + } + // Global-mute toggle in the header. A click mutes for the configured // default duration (or resumes if already muted); the menu bar offers // explicit durations. Reading nav.muteTick establishes the dependency so @@ -305,6 +436,12 @@ struct PanelContentView: View { HStack(spacing: 5) { Text(label) .font(.caption.weight(isActive ? .semibold : .regular)) + // A tab keeps its natural width and scrolls out of view + // instead of being squeezed. Squeezed is what it did: at the + // panel's 560pt minimum the labels broke mid-word into + // "Ses-sions" and "Out-come s". + .lineLimit(1) + .fixedSize(horizontal: true, vertical: false) if count > 0 { Text("\(count)") .font(.caption2.monospacedDigit()) @@ -914,11 +1051,32 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, configureExtension(row, from: .settings) } + // The two ways back out, in one place each so Esc and the chevron cannot + // disagree. Both leave Settings a note about where the keyboard was, which + // it consumes on the way in. + func closeExtensionsBrowser() { + nav.settingsResumeRow = .browseExtensions + nav.mode = .settings + } + + func closeExtensionConfig() { + let origin = nav.extensionConfig?.origin ?? .extensions + if origin == .settings, let id = nav.extensionConfig?.id { + nav.settingsResumeRow = .installedExtension(id) + } + nav.mode = origin + } + private func removeExtension(_ id: String) { extensionCatalog.remove(id) + let origin = nav.extensionConfig?.origin ?? .settings + // Not the extension's own row, which is the one thing that is about to + // stop existing. Browse is the row next to it and the obvious next move + // after removing something. + if origin == .settings { nav.settingsResumeRow = .browseExtensions } // Back where the page was opened from — the extension it described no // longer exists, so staying on it is not an option. - nav.mode = nav.extensionConfig?.origin ?? .settings + nav.mode = origin } // Installed and refused, as the Extensions settings category renders them. @@ -1065,6 +1223,8 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, store: store, sessions: sessions, nav: nav, phrases: phrases, extensions: extensions, extensionCatalog: extensionCatalog, onConfigureExtension: { [weak self] row in self?.configureExtension(row) }, + onCloseExtensionsBrowser: { [weak self] in self?.closeExtensionsBrowser() }, + onCloseExtensionConfig: { [weak self] in self?.closeExtensionConfig() }, onGrantPermissions: { [weak self] in self?.handleGrantPermissions() } ).environmentObject(SessionPersistence.shared)) // Don't let SwiftUI's preferred / intrinsic content size drive @@ -3764,29 +3924,62 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, return true } - // Phrases mode: ↑/↓ navigate every row (defaults + custom), - // Space toggles the selected default, ⌫ removes the selected - // custom, Esc returns to Settings. Typing / Tab / Enter for - // adding still fall through to SwiftUI's TextField. - // An extension's configuration form: Esc returns to the browser and - // everything else belongs to the text fields. Enter is handled by the - // field's own .onSubmit rather than here, so it saves from whichever - // field has focus. + // An extension's configuration form. Two levels, like the browser below + // and the Usage tab: the page opens with no field focused, so this + // branch owns ↑↓ (move between fields), ⏎ (hand the selected field + // focus), ⌘S (save) and ⌘⌫ (remove). Once a field *is* first + // responder it consumes keys before NSWindow.keyDown, and the field's + // own .onSubmit and .onExitCommand save and step back out. + // + // It used to own Esc and ⌘⌫ and nothing else, which left the form + // reachable only with the mouse or with Tab: ⏎ was advertised in the + // footer and did nothing at all until something had been clicked. if case .extensionConfig = nav.mode { let onlyCommand = mods.intersection([.command, .control, .option, .shift]) == [.command] - // ⌘⌫ rather than a bare ⌫, which is what the field editor wants, - // and deliberately the macOS "move to trash" combination: this is - // the one destructive action on the page and it takes no - // confirmation. - if onlyCommand, event.keyCode == KeyCode.delete || event.keyCode == KeyCode.forwardDelete { - nav.extensionConfig?.onRemove() - return true + if onlyCommand { + switch event.keyCode { + // ⌘⌫ rather than a bare ⌫, which is what the field editor + // wants, and deliberately the macOS "move to trash" + // combination: this is the one destructive action on the page + // and it takes no confirmation. + case KeyCode.delete, KeyCode.forwardDelete: + nav.extensionConfig?.onRemove() + return true + // The only way to commit an edit the user stepped out of with + // Esc, and the only Save at all on a page where ⏎ has handed + // focus back. + // + // Level one only, verified in the app rather than assumed: a + // focused field swallows this exactly as it swallows ⌘⌫, so + // the footer advertises it only while nothing is focused and + // names ⏎ as the way to save from inside a field. + case KeyCode.sKey: + nav.extensionConfig?.save() + return true + // Leave every other ⌘ combination to the app: ⌘Q and friends. + default: + return false + } + } + guard mods.intersection([.command, .control, .option]).isEmpty else { return false } + switch Self.extensionConfigKeyAction(keyCode: event.keyCode) { + case .back: + // Wherever this page was opened from, which is the Settings list + // as often as the browser. + closeExtensionConfig() + case .moveSelection(let delta): + nav.extensionConfig?.moveSelection(by: delta) + case .activateSelection: + switch nav.extensionConfig?.selection { + case .field: nav.extensionConfig?.focusSelectedField() + case .save: nav.extensionConfig?.save() + case .remove: nav.extensionConfig?.onRemove() + case .back: closeExtensionConfig() + case nil: break + } + case .swallow: + break } - let plain = mods.intersection([.command, .control, .option, .shift]).isEmpty - guard plain, event.keyCode == KeyCode.escape else { return false } - // Wherever this page was opened from, which is the Settings list as - // often as the browser. - nav.mode = nav.extensionConfig?.origin ?? .extensions return true } @@ -3835,7 +4028,7 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, configureExtension(row) } return true - // Leave every other ⌘ combination to the app — ⌘Q and friends. + // Leave every other ⌘ combination to the app: ⌘Q and friends. default: return false } } @@ -3844,7 +4037,7 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, switch Self.extensionsKeyAction(keyCode: event.keyCode, characters: event.charactersIgnoringModifiers, queryIsEmpty: extensionCatalog.query.isEmpty) { - case .back: nav.mode = .settings + case .back: closeExtensionsBrowser() case .clearQuery: extensionCatalog.query = "" case .focusSearch: extensionCatalog.focusSearch() case .appendToQuery(let typed): @@ -3871,6 +4064,10 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, return true } + // Phrases mode: ↑/↓ navigate every row (defaults + custom), + // Space toggles the selected default, ⌫ removes the selected + // custom, Esc returns to Settings. Typing / Tab / Enter for + // adding still fall through to SwiftUI's TextField. if nav.mode == .phrases { let plain = mods.intersection([.command, .control, .option, .shift]).isEmpty guard plain else { return false } @@ -4193,6 +4390,42 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, case swallow } + // The config form's level one, where no field has focus. A page of text + // fields cannot walk itself with ↑↓: a focused field is first responder + // and takes them before NSWindow.keyDown runs, so the traversal lives here + // and ⏎ is what hands over. + enum ExtensionConfigKeyAction: Equatable { + case back + case moveSelection(Int) + // What that does depends on where the selection is, which the controller + // resolves: focus a field, save, remove, or walk back out. + case activateSelection + // Swallowed rather than passed on, the same rule every sub-page follows: + // a stray key must not reach the Events bindings and answer a permission + // prompt on a tab this page isn't showing. + case swallow + } + + // Tab joins ⏎ because it is what a macOS form is entered with, and at level + // one nothing else claims it. Neither is gated on the page having fields: + // the selection walks the back chevron, Save and Remove as well, so there is + // always something for ⏎ to act on, and what that is belongs to the model + // rather than to a key table. + static func extensionConfigKeyAction(keyCode: UInt16) -> ExtensionConfigKeyAction { + switch keyCode { + case KeyCode.escape: + return .back + case KeyCode.upArrow: + return .moveSelection(-1) + case KeyCode.downArrow: + return .moveSelection(1) + case KeyCode.returnKey, KeyCode.numpadEnter, KeyCode.tab: + return .activateSelection + default: + return .swallow + } + } + static func extensionsKeyAction(keyCode: UInt16, characters: String?, queryIsEmpty: Bool) -> ExtensionsKeyAction { @@ -4406,6 +4639,19 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, top ? nav.selectPrevCategory() : nav.selectNextCategory() case .phrases: top ? phrases.selectFirst() : phrases.selectLast() + case .extensions: + extensionCatalog.selectEdge( + among: ExtensionCatalog.matching( + ExtensionCatalog.rows(catalogue: extensionCatalog.entries, + installed: extensions.manifests, + refused: extensions.refused), + query: extensionCatalog.query), + top: top) + case .extensionConfig: + // Always somewhere to go: the back chevron and Remove are targets on + // every extension's page, including one declaring no config keys. + guard let config = nav.extensionConfig else { return false } + config.selectEdge(top: top) default: return false // modal / single-purpose screens have nothing to jump } diff --git a/panel/PanelNav.swift b/panel/PanelNav.swift index 08f0290..c1dc3b7 100644 --- a/panel/PanelNav.swift +++ b/panel/PanelNav.swift @@ -1003,6 +1003,17 @@ final class PanelNav: ObservableObject { // values. @Published var extensionConfig: ExtensionConfigModel? + // The row Settings should land on when it is re-entered from one of its own + // sub-pages, consumed once by SettingsView.onAppear. + // + // That view resets to the category sidebar on every appearance, which is + // right for a fresh visit and wrong for a return: the Extensions category + // now holds a card per installed extension, so opening the third one and + // pressing Esc put the keyboard back on the sidebar, four keystrokes from + // where it had just been. Configuring two extensions in a row is exactly + // the flow that happens in. + var settingsResumeRow: SettingsRow? + @Published var extensionTabs: [ExtensionTab] = [] { didSet { reconcileModeWithTabs() } } @@ -1252,6 +1263,26 @@ final class PanelNav: ObservableObject { selectedSettingIndex = settingsAttentionRows.count } + // What SettingsView.onAppear calls instead of resetting outright. With no + // row pending this is the old behaviour exactly; with one it re-enters the + // detail on that row, switching category if the row lives in another one so + // a resume can't silently land on index 0, which is an attention banner + // whenever there is one, and ⏎ on that runs the updater. + func resumeSettingsSelection() { + let pending = settingsResumeRow + settingsResumeRow = nil + guard let pending, + let category = SettingsCategory.allCases.first(where: { rows(in: $0).contains(pending) }) + else { + settingsDetailFocused = false + selectFirstCategoryRow() + return + } + settingsCategory = category + settingsDetailFocused = true + selectedSettingIndex = index(of: pending) + } + // Put the selection back on the row it was on, wherever that row has moved // to. Called whenever the attention rows change, because they are what shift // every index behind them. diff --git a/panel/Settings.swift b/panel/Settings.swift index 91b7d4a..582fa86 100644 --- a/panel/Settings.swift +++ b/panel/Settings.swift @@ -64,9 +64,10 @@ struct SettingsView: View { .onAppear { // Matching UsageView: never land back inside the detail from a // previous visit, where ↑↓ move rows rather than categories and the - // attention-row count may have changed while away. - nav.settingsDetailFocused = false - nav.selectFirstCategoryRow() + // attention-row count may have changed while away, unless this is + // a return from one of this page's own sub-pages, which resumes on + // the row that opened it. See PanelNav.resumeSettingsSelection. + nav.resumeSettingsSelection() nav.loadFromConfig() nav.refreshVoiceModelCached() if nav.voiceModelCached, nav.voicesAvailable.isEmpty {