diff --git a/alt-tab-macos.xcodeproj/project.pbxproj b/alt-tab-macos.xcodeproj/project.pbxproj index 688b5bf1..ed764034 100644 --- a/alt-tab-macos.xcodeproj/project.pbxproj +++ b/alt-tab-macos.xcodeproj/project.pbxproj @@ -61,6 +61,8 @@ 5E2A571E000000000000A202 /* ApplicationState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5E2A571E000000000000A201 /* ApplicationState.swift */; }; 5E2A571E000000000000A203 /* ApplicationState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5E2A571E000000000000A201 /* ApplicationState.swift */; }; D04BAE2200000000ED004T11 /* SettingsSearchHighlightTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAE2200000000ED004T12 /* SettingsSearchHighlightTests.swift */; }; + D04BAE2200000000ED005T11 /* SettingsSectionSearchContentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAE2200000000ED005T12 /* SettingsSectionSearchContentTests.swift */; }; + D04BAE2200000000ED006T11 /* SidebarListTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAE2200000000ED006T12 /* SidebarListTests.swift */; }; 5F04852E2F09AC9700235D4A /* WindowCaptureEvents.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5F04852D2F09AC9700235D4A /* WindowCaptureEvents.swift */; }; 5F0A63032F82D97C00AE4C38 /* LicenseManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5F0A63012F82D97C00AE4C38 /* LicenseManager.swift */; }; 5F0A63042F82D97C00AE4C38 /* LicenseState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5F0A63022F82D97C00AE4C38 /* LicenseState.swift */; }; @@ -270,6 +272,8 @@ D04BAE2000000000ED002001 /* SettingsSearchIndex.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAE2000000000ED002002 /* SettingsSearchIndex.swift */; }; D04BAE2200000000ED004001 /* SettingsSearchHighlight.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAE2200000000ED004002 /* SettingsSearchHighlight.swift */; }; D04BAE2200000000ED004003 /* SettingsSearchHighlight.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAE2200000000ED004002 /* SettingsSearchHighlight.swift */; }; + D04BAE2200000000ED005001 /* SettingsSectionSearchContent.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAE2200000000ED005002 /* SettingsSectionSearchContent.swift */; }; + D04BAE2200000000ED005003 /* SettingsSectionSearchContent.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAE2200000000ED005002 /* SettingsSectionSearchContent.swift */; }; D04BAE2200000000ED004T01 /* TableGroupView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1C961FB3DCF4906FEEF4A23D /* TableGroupView.swift */; }; D04BAE2200000000ED004T03 /* SettingsSearch.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BA00000000000C0DE0001 /* SettingsSearch.swift */; }; D04BAAAF5CFA991D3B078DB8 /* KeyboardEvents.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BA4A621A8A15660F973C1 /* KeyboardEvents.swift */; }; @@ -305,6 +309,7 @@ D04BAFF30A98CF287F85DA1E /* ExceptionsTab.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BA27695D9A5824720BD7B /* ExceptionsTab.swift */; }; D04BAFF70A98CF287F85DA22 /* ExceptionEditorView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAFF60A98CF287F85DA21 /* ExceptionEditorView.swift */; }; D04BAFF90A98CF287F85DA24 /* SidebarList.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAFF80A98CF287F85DA23 /* SidebarList.swift */; }; + D04BAFF90A98CF287F85DA25 /* SidebarList.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BAFF80A98CF287F85DA23 /* SidebarList.swift */; }; D04BB101A1B2C3D4E5F60011 /* InputSourceEvents.swift in Sources */ = {isa = PBXBuildFile; fileRef = D04BB102A1B2C3D4E5F60011 /* InputSourceEvents.swift */; }; DC06402457ECCE6C902FEAE4 /* OverrideClickResolver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 69A2E38AF41E866D3CDE71BF /* OverrideClickResolver.swift */; }; DC8CF4F01C999497352736F1 /* OverrideClickResolverTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B4409E4B3D98C491503CE5D /* OverrideClickResolverTests.swift */; }; @@ -381,6 +386,8 @@ 5E2A571E000000000000A001 /* WindowState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowState.swift; sourceTree = ""; }; 5E2A571E000000000000A201 /* ApplicationState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ApplicationState.swift; sourceTree = ""; }; D04BAE2200000000ED004T12 /* SettingsSearchHighlightTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsSearchHighlightTests.swift; sourceTree = ""; }; + D04BAE2200000000ED005T12 /* SettingsSectionSearchContentTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsSectionSearchContentTests.swift; sourceTree = ""; }; + D04BAE2200000000ED006T12 /* SidebarListTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarListTests.swift; sourceTree = ""; }; 5F04852D2F09AC9700235D4A /* WindowCaptureEvents.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowCaptureEvents.swift; sourceTree = ""; }; 5F05D0E62F31441A004A95F9 /* AGENTS.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = AGENTS.md; sourceTree = ""; }; 5F05D0E72F31441A004A95F9 /* CLAUDE.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = CLAUDE.md; sourceTree = ""; }; @@ -563,6 +570,7 @@ D04BAE1F00000000ED001002 /* ShortcutEditor.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ShortcutEditor.swift; sourceTree = ""; }; D04BAE2000000000ED002002 /* SettingsSearchIndex.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SettingsSearchIndex.swift; sourceTree = ""; }; D04BAE2200000000ED004002 /* SettingsSearchHighlight.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SettingsSearchHighlight.swift; sourceTree = ""; }; + D04BAE2200000000ED005002 /* SettingsSectionSearchContent.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SettingsSectionSearchContent.swift; sourceTree = ""; }; D04BA1D80F4EEF2A91BAD29C /* release.config.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = release.config.js; sourceTree = ""; }; D04BA1DF8CAB2FAB7FE9244B /* TileFontIconView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TileFontIconView.swift; sourceTree = ""; }; D04BA2526DC6726E0F7ACF7C /* LabelAndControl.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LabelAndControl.swift; sourceTree = ""; }; @@ -1519,6 +1527,9 @@ D04BAE2000000000ED002002 /* SettingsSearchIndex.swift */, D04BAE2200000000ED004002 /* SettingsSearchHighlight.swift */, D04BAE2200000000ED004T12 /* SettingsSearchHighlightTests.swift */, + D04BAE2200000000ED005002 /* SettingsSectionSearchContent.swift */, + D04BAE2200000000ED005T12 /* SettingsSectionSearchContentTests.swift */, + D04BAE2200000000ED006T12 /* SidebarListTests.swift */, D04BAFF80A98CF287F85DA23 /* SidebarList.swift */, 1C961FB3DCF4906FEEF4A23D /* TableGroupView.swift */, ); @@ -1824,6 +1835,10 @@ 5E2A571E000000000000A203 /* ApplicationState.swift in Sources */, 5FC5980000000000000C0003 /* CGSSymbolicHotKey.swift in Sources */, D04BAE2200000000ED004T11 /* SettingsSearchHighlightTests.swift in Sources */, + D04BAE2200000000ED005T11 /* SettingsSectionSearchContentTests.swift in Sources */, + D04BAFF90A98CF287F85DA25 /* SidebarList.swift in Sources */, + D04BAE2200000000ED006T11 /* SidebarListTests.swift in Sources */, + D04BAE2200000000ED005003 /* SettingsSectionSearchContent.swift in Sources */, D04BAE2200000000ED004003 /* SettingsSearchHighlight.swift in Sources */, D04BAE2200000000ED004T01 /* TableGroupView.swift in Sources */, D04BAE2200000000ED004T03 /* SettingsSearch.swift in Sources */, @@ -1982,6 +1997,7 @@ D04BADE4DDF7597749FB13BD /* SettingsWindow.swift in Sources */, D04BAE2000000000ED002001 /* SettingsSearchIndex.swift in Sources */, D04BAE2200000000ED004001 /* SettingsSearchHighlight.swift in Sources */, + D04BAE2200000000ED005001 /* SettingsSectionSearchContent.swift in Sources */, BF0C8CC5057406014FD612CC /* ATShortcut.swift in Sources */, D04BA00000000000C0DE0012 /* ShortcutAction.swift in Sources */, D04BA00000000000C0DE0021 /* SwitcherSession.swift in Sources */, diff --git a/src/preferences/settings-window/SettingsSearchHighlightSpecs.md b/src/preferences/settings-window/SettingsSearchHighlightSpecs.md index 959dbe9e..8f1d5c79 100644 --- a/src/preferences/settings-window/SettingsSearchHighlightSpecs.md +++ b/src/preferences/settings-window/SettingsSearchHighlightSpecs.md @@ -21,7 +21,9 @@ that work and are pinned here: yields a nil target. - Highlighting applies the search color to exactly the matched range and fully reverts on clear. - `makeText` registers into whatever builder is active; called **outside** an indexed scope (no active - builder) it silently no-ops — safe to call from any context (e.g. `refreshShortcutRows`). + builder) it silently no-ops. Content rebuilt after the section's build scope (sidebar rows recreated + by `refreshShortcutRows`) is therefore re-published through `SettingsWindow.refreshSectionSearchContent`, + which re-opens an `indexed { ... }` scope — see [[SettingsSectionSearchContent]]. ## Test scenarios diff --git a/src/preferences/settings-window/SettingsSearchHighlightTests.swift b/src/preferences/settings-window/SettingsSearchHighlightTests.swift index 023e46fb..a581eb77 100644 --- a/src/preferences/settings-window/SettingsSearchHighlightTests.swift +++ b/src/preferences/settings-window/SettingsSearchHighlightTests.swift @@ -16,11 +16,8 @@ import Cocoa /// so a section's search actually drives it. (Tier B: factory ⇄ index wiring.) /// /// The sidebar rows feed `NSTextField` labels into `highlightTarget`; the rounded-section rows -/// feed `LightLabel`. Both overloads are covered. -/// -/// (`SidebarListRow` itself can't be compiled into this test target — it uses the 10.13-only -/// `NSAppearance.current` API which is a hard error under the test target's 13.1 deployment -/// floor + warnings-as-errors. So we pin the *mechanism* it depends on rather than the row type.) +/// feed `LightLabel`. Both overloads are covered. (`SidebarListRow` itself — and its Pro-badge +/// recycling — is covered separately in `SidebarListTests`; it compiles into this target fine.) final class SettingsSearchHighlightTests: XCTestCase { // MARK: - Tier A: NSTextField highlight target (what sidebar rows use) diff --git a/src/preferences/settings-window/SettingsSectionSearchContent.swift b/src/preferences/settings-window/SettingsSectionSearchContent.swift new file mode 100644 index 00000000..81c4f312 --- /dev/null +++ b/src/preferences/settings-window/SettingsSectionSearchContent.swift @@ -0,0 +1,54 @@ +import Cocoa + +/// A Settings section's searchable content, split into a fixed **base** captured during the +/// section's build-time `SettingsSearchIndex.indexed { }` scope and a replaceable **dynamic** part +/// for content that is rebuilt while the section is already live. +/// +/// Why the split exists: most controls are built once, so the push-based index harvested at build +/// time stays valid for the session. But ControlsTab's shortcut sidebar rows ("Shortcut 1", +/// "Shortcut 2", …) are torn down and recreated by `refreshShortcutRows` — which runs *outside* the +/// build scope (via `preferenceChanged`, the +/- buttons, input-source changes, the pro-lock +/// observer, …). Their inline `registerSearchContent` no-ops there (no active builder), and the +/// section's base targets keep pointing at the now-removed labels, so a query like "sho" stopped +/// lighting up the rebuilt rows. Sections that manage such rows skip them in the build-time walk +/// (so they never land in `base`) and re-publish them here via `setDynamic` after every rebuild — +/// a wholesale swap that leaves no stale targets behind and makes freshly-added rows searchable. +final class SettingsSectionSearchContent { + private let baseStrings: [String] + private let baseTargets: [SettingsSearchHighlightTarget] + private var dynamicStrings: [String] = [] + private var dynamicTargets: [SettingsSearchHighlightTarget] = [] + + init(strings: [String] = [], targets: [SettingsSearchHighlightTarget] = []) { + baseStrings = strings + baseTargets = targets + } + + /// Replace the dynamic part wholesale. Called after a section rebuilds its sidebar rows, with + /// the strings + highlight targets freshly registered from the *current* rows. Replacing (not + /// appending) is the whole point: targets for removed rows are dropped, so no stale highlight + /// targets accumulate and `searchableStrings` never reports rows that no longer exist. + func setDynamic(strings: [String], targets: [SettingsSearchHighlightTarget]) { + dynamicStrings = strings + dynamicTargets = targets + } + + var searchableStrings: [String] { baseStrings + dynamicStrings } + var highlightTargets: [SettingsSearchHighlightTarget] { baseTargets + dynamicTargets } + + /// True when the query is empty (everything matches) or any base/dynamic string or highlight + /// target matches it. Drives whether the section stays visible in filtered search results. + func matches(_ query: String) -> Bool { + if SettingsSearch.isQueryEmpty(query) { return true } + if searchableStrings.contains(where: { SettingsSearch.match(query, in: $0) != nil }) { return true } + return highlightTargets.contains { $0.hasMatch(query) } + } + + func highlightMatches(_ query: String) { + highlightTargets.forEach { $0.updateHighlight(query) } + } + + func clearHighlights() { + highlightTargets.forEach { $0.clear() } + } +} diff --git a/src/preferences/settings-window/SettingsSectionSearchContentSpecs.md b/src/preferences/settings-window/SettingsSectionSearchContentSpecs.md new file mode 100644 index 00000000..cc085865 --- /dev/null +++ b/src/preferences/settings-window/SettingsSectionSearchContentSpecs.md @@ -0,0 +1,37 @@ +# SettingsSectionSearchContent — Specs + +## Summary + +Holds a Settings section's searchable content as two parts: + +- **base** — captured once during the section's build-time `SettingsSearchIndex.indexed { }` scope + (section title, dropdowns, buttons, static labels). Immutable for the session. +- **dynamic** — content rebuilt while the section is live and re-published wholesale via `setDynamic`. + This is where a section parks sidebar rows that get torn down and recreated after the initial build + (ControlsTab's "Shortcut 1" / "Shortcut 2" rows). Those rows are deliberately skipped by the + build-time walk so they live *only* here, and every rebuild swaps the dynamic part out completely. + +This is the fix for the regression where typing "sho" stopped highlighting rebuilt shortcut rows: +the recreated rows' inline registration no-ops outside the build scope, and the base targets kept +pointing at the removed labels. Re-publishing the current rows into the dynamic part — replacing, +not appending — keeps highlight targets in sync with the live rows. + +## Behavior & edge cases + +- `matches` is true for an empty query, or when any base **or** dynamic string/target matches. +- `setDynamic` **replaces** the dynamic part: targets for removed rows are dropped (no stale + targets), and newly-added rows become searchable immediately. +- `highlightMatches` / `clearHighlights` drive both base and dynamic targets. +- A section with no dynamic content behaves exactly like its base. + +## Test scenarios + +Mirrors `SettingsSectionSearchContentTests.swift` 1:1. + +- **testEmptyQueryAlwaysMatches** — an empty/whitespace query matches regardless of content. +- **testBaseStringIsSearchable** — a base string drives a match. +- **testDynamicStringIsSearchable** — a string published via `setDynamic` drives a match. +- **testDynamicTargetReportsAndHighlightsMatch** — a dynamic highlight target both reports a match and gets highlighted by `highlightMatches`. +- **testSetDynamicReplacesAndDropsStaleTargets** — re-publishing recreated rows replaces the dynamic part: the new rows match and highlight, the previous rows are gone (the "rebuilt Shortcut N rows still highlight" regression). +- **testSetDynamicReplaceDoesNotAccumulate** — replacing dynamic content twice leaves only the latest set, never the union. +- **testClearHighlightsClearsDynamicTargets** — `clearHighlights` reverts dynamic targets too. diff --git a/src/preferences/settings-window/SettingsSectionSearchContentTests.swift b/src/preferences/settings-window/SettingsSectionSearchContentTests.swift new file mode 100644 index 00000000..32934338 --- /dev/null +++ b/src/preferences/settings-window/SettingsSectionSearchContentTests.swift @@ -0,0 +1,111 @@ +import XCTest +import Cocoa + +/// Coverage for `SettingsSectionSearchContent` — the per-section search store that splits a fixed +/// build-time "base" from a replaceable "dynamic" part. +/// +/// Why this suite exists: typing "sho" stopped highlighting ControlsTab's "Shortcut 1"/"Shortcut 2" +/// sidebar rows after they were rebuilt (the +/- buttons, a recorder edit, an input-source change, +/// the pro-lock observer). The rows are recreated *outside* the section's build-time `indexed { }` +/// scope, so their inline registration no-ops and the section's cached targets keep pointing at the +/// removed labels. The fix routes such rows through the dynamic part and re-publishes them wholesale +/// on every rebuild. These tests pin that the dynamic part is searchable, that re-publishing +/// *replaces* (never accumulates, never leaves a stale target), and that highlighting drives it. +/// +/// We exercise the section store with plain `NSTextField` targets — exactly what the rows feed it, +/// and the cleanest unit. (`ControlsTab` itself can't compile into the test target — its trigger +/// row + sheets drag in the whole settings window — so the live re-publish path is verified by +/// running the app, not here.) +final class SettingsSectionSearchContentTests: XCTestCase { + + private func target(_ title: String) -> SettingsSearchHighlightTarget { + SettingsSearchHighlight.highlightTarget(NSTextField(labelWithString: title))! + } + + private func hasMatchColor(_ textField: NSTextField) -> Bool { + let attr = textField.attributedStringValue + var found = false + attr.enumerateAttribute(.foregroundColor, in: NSRange(location: 0, length: attr.length)) { value, _, _ in + if value as? NSColor == Appearance.searchMatchForegroundColor { found = true } + } + return found + } + + func testEmptyQueryAlwaysMatches() { + let content = SettingsSectionSearchContent() + XCTAssertTrue(content.matches(""), "an empty query keeps every section visible") + XCTAssertTrue(content.matches(" "), "a whitespace-only query is treated as empty") + } + + func testBaseStringIsSearchable() { + let content = SettingsSectionSearchContent(strings: ["Appearance"], targets: []) + XCTAssertTrue(content.matches("appea"), "a base string must drive a match") + XCTAssertFalse(content.matches("zzzzz"), "a non-matching query must not match") + } + + func testDynamicStringIsSearchable() { + let content = SettingsSectionSearchContent() + content.setDynamic(strings: ["Shortcut 1"], targets: []) + XCTAssertTrue(content.matches("sho"), + "a string published via setDynamic must be searchable — this is the sidebar-row path") + } + + func testDynamicTargetReportsAndHighlightsMatch() { + let tf = NSTextField(labelWithString: "Shortcut 1") + let content = SettingsSectionSearchContent() + content.setDynamic(strings: [], targets: [SettingsSearchHighlight.highlightTarget(tf)!]) + XCTAssertTrue(content.matches("Sho"), "a dynamic highlight target alone must drive section visibility") + content.highlightMatches("Sho") + XCTAssertTrue(hasMatchColor(tf), "highlightMatches must light up the dynamic target's label") + } + + /// The exact regression: rebuilt "Shortcut N" rows must light up again. Publishing the recreated + /// rows replaces the dynamic part — the previous rows' targets are dropped (no stale highlight), + /// the previous rows stop being searchable, and the new rows match + highlight. + func testSetDynamicReplacesAndDropsStaleTargets() { + let content = SettingsSectionSearchContent() + let firstTf = NSTextField(labelWithString: "Shortcut 1") + let firstTarget = SettingsSearchHighlight.highlightTarget(firstTf)! + content.setDynamic(strings: ["Shortcut 1"], targets: [firstTarget]) + XCTAssertTrue(content.matches("Shortcut 1")) + + // Rebuild: refreshShortcutRows recreates the rows as fresh instances. + let rebuiltTf = NSTextField(labelWithString: "Shortcut 2") + let rebuiltTarget = SettingsSearchHighlight.highlightTarget(rebuiltTf)! + content.setDynamic(strings: ["Shortcut 2"], targets: [rebuiltTarget]) + + XCTAssertFalse(content.highlightTargets.contains { $0 === firstTarget }, + "the removed row's highlight target must be dropped, not left dangling") + XCTAssertTrue(content.highlightTargets.contains { $0 === rebuiltTarget }, + "the rebuilt row's highlight target must be registered") + XCTAssertFalse(content.matches("Shortcut 1"), + "the removed row must no longer be searchable") + XCTAssertTrue(content.matches("Shortcut 2"), + "the rebuilt row must be searchable") + content.highlightMatches("Sho") + XCTAssertTrue(hasMatchColor(rebuiltTf), + "regression: a rebuilt 'Shortcut N' row must light up when the user types 'sho'") + } + + func testSetDynamicReplaceDoesNotAccumulate() { + let content = SettingsSectionSearchContent(strings: ["Base"], targets: []) + content.setDynamic(strings: ["Alpha"], targets: [target("Alpha")]) + let second = target("Bravo") + content.setDynamic(strings: ["Bravo"], targets: [second]) + XCTAssertEqual(content.highlightTargets.count, 1, + "replacing dynamic content must not accumulate targets across rebuilds") + XCTAssertTrue(content.highlightTargets.contains { $0 === second }) + XCTAssertEqual(content.searchableStrings, ["Base", "Bravo"], + "searchableStrings is base + the latest dynamic strings only") + } + + func testClearHighlightsClearsDynamicTargets() { + let tf = NSTextField(labelWithString: "Shortcut 1") + let content = SettingsSectionSearchContent() + content.setDynamic(strings: [], targets: [SettingsSearchHighlight.highlightTarget(tf)!]) + content.highlightMatches("Sho") + XCTAssertTrue(hasMatchColor(tf)) + content.clearHighlights() + XCTAssertFalse(hasMatchColor(tf), "clearHighlights must revert dynamic targets too") + } +} diff --git a/src/preferences/settings-window/SettingsWindow.swift b/src/preferences/settings-window/SettingsWindow.swift index 92f3f5a3..0c2e4ed1 100644 --- a/src/preferences/settings-window/SettingsWindow.swift +++ b/src/preferences/settings-window/SettingsWindow.swift @@ -9,6 +9,13 @@ private struct SettingsSectionDefinition { /// highlight targets as the section's factories build their widgets. The closure is invoked /// exactly once, in `addSection`. let builder: () -> NSView + /// Optional hook for sections whose searchable content includes sidebar rows that are rebuilt + /// *after* the initial build (ControlsTab's shortcut rows). When set, `addSection` skips + /// `SidebarListRow`s in the build-time walk — they'd otherwise be captured as base targets that + /// go stale on the next rebuild — and this closure re-registers the current rows into the + /// section's *dynamic* search content (at build time, and again after each rebuild via + /// `refreshSectionSearchContent`). nil for sections with no such rows. + let registerDynamicSearchContent: (() -> Void)? } private final class SettingsSection { @@ -17,8 +24,10 @@ private final class SettingsSection { let icon: NSImage let container: NSView let anchor: NSView - let searchableStrings: [String] - let highlightTargets: [SettingsSearchHighlightTarget] + let search: SettingsSectionSearchContent + /// See `SettingsSectionDefinition.registerDynamicSearchContent`. Re-run inside a fresh indexed + /// scope by `refreshSectionSearchContent` to refresh this section's dynamic search content. + let registerDynamicSearchContent: (() -> Void)? let interSectionSpacingConstraint: NSLayoutConstraint let bottomSpacingConstraint: NSLayoutConstraint let titleTopConstraint: NSLayoutConstraint @@ -28,8 +37,8 @@ private final class SettingsSection { _ icon: NSImage, _ container: NSView, _ anchor: NSView, - _ searchableStrings: [String], - _ highlightTargets: [SettingsSearchHighlightTarget], + _ search: SettingsSectionSearchContent, + _ registerDynamicSearchContent: (() -> Void)?, _ interSectionSpacingConstraint: NSLayoutConstraint, _ bottomSpacingConstraint: NSLayoutConstraint, _ titleTopConstraint: NSLayoutConstraint) { @@ -38,25 +47,27 @@ private final class SettingsSection { self.icon = icon self.container = container self.anchor = anchor - self.searchableStrings = searchableStrings - self.highlightTargets = highlightTargets + self.search = search + self.registerDynamicSearchContent = registerDynamicSearchContent self.interSectionSpacingConstraint = interSectionSpacingConstraint self.bottomSpacingConstraint = bottomSpacingConstraint self.titleTopConstraint = titleTopConstraint } + func setDynamicSearchContent(_ strings: [String], _ targets: [SettingsSearchHighlightTarget]) { + search.setDynamic(strings: strings, targets: targets) + } + func matches(_ query: String) -> Bool { - if SettingsSearch.isQueryEmpty(query) { return true } - if searchableStrings.contains(where: { SettingsSearch.match(query, in: $0) != nil }) { return true } - return highlightTargets.contains { $0.hasMatch(query) } + search.matches(query) } func highlightMatches(_ query: String) { - highlightTargets.forEach { $0.updateHighlight(query) } + search.highlightMatches(query) } func clearHighlights() { - highlightTargets.forEach { $0.clear() } + search.clearHighlights() } } @@ -362,6 +373,10 @@ class SettingsWindow: NSWindow { setupContentPane() sectionDefinitions().forEach { addSection($0) } refreshControlsFromSettings() + // Publish each section's dynamic search content (its rebuilt-after-build sidebar rows) now + // that the section objects exist. Uses `self` directly — `SettingsWindow.shared` isn't + // assigned until `init` finishes, so ControlsTab's own refresh calls no-op until then. + sections.forEach { refreshSectionSearchContent($0.id) } applySearch("") } @@ -525,10 +540,10 @@ class SettingsWindow: NSWindow { private func sectionDefinitions() -> [SettingsSectionDefinition] { [ - SettingsSectionDefinition(id: "appearance", title: NSLocalizedString("Appearance", comment: ""), symbol: .paintpalette, builder: AppearanceTab.initTab), - SettingsSectionDefinition(id: "controls", title: NSLocalizedString("Controls", comment: ""), symbol: .command, builder: ControlsTab.initTab), - SettingsSectionDefinition(id: "general", title: NSLocalizedString("General", comment: ""), symbol: .gearshape, builder: GeneralTab.initTab), - SettingsSectionDefinition(id: "exceptions", title: NSLocalizedString("Exceptions", comment: ""), symbol: .handRaised, builder: ExceptionsTab.initTab), + SettingsSectionDefinition(id: "appearance", title: NSLocalizedString("Appearance", comment: ""), symbol: .paintpalette, builder: AppearanceTab.initTab, registerDynamicSearchContent: nil), + SettingsSectionDefinition(id: "controls", title: NSLocalizedString("Controls", comment: ""), symbol: .command, builder: ControlsTab.initTab, registerDynamicSearchContent: ControlsTab.registerSidebarRowsSearchContent), + SettingsSectionDefinition(id: "general", title: NSLocalizedString("General", comment: ""), symbol: .gearshape, builder: GeneralTab.initTab, registerDynamicSearchContent: nil), + SettingsSectionDefinition(id: "exceptions", title: NSLocalizedString("Exceptions", comment: ""), symbol: .handRaised, builder: ExceptionsTab.initTab, registerDynamicSearchContent: nil), ] } @@ -593,23 +608,44 @@ class SettingsWindow: NSWindow { // runs as a safety net to catch direct widget creation in tabs (NSTextField, NSButton // etc. created outside the LabelAndControl/TableGroupView factories) that hasn't been // migrated to inline registration yet. The walk's results get unioned in via `Set` below. - var (searchableStrings, highlightTargets) = collectSearchContent(sectionTitle, sectionView) + // Sections that manage rebuilt-after-build sidebar rows (`registerDynamicSearchContent`) + // skip those rows here and publish them as dynamic content instead — see that hook. + let skipSidebarRows = definition.registerDynamicSearchContent != nil + var (searchableStrings, highlightTargets) = collectSearchContent(sectionTitle, sectionView, skipSidebarRows: skipSidebarRows) searchableStrings.append(contentsOf: builder.strings) highlightTargets.append(contentsOf: builder.targets) - let uniqueStrings = Array(Set(searchableStrings)) + let search = SettingsSectionSearchContent(strings: Array(Set(searchableStrings)), targets: highlightTargets) let section = SettingsSection(definition.id, definition.title, sidebarImage(definition), container, sectionTitle, - uniqueStrings, - highlightTargets, + search, + definition.registerDynamicSearchContent, interSectionSpacingConstraint, spacerHeightConstraint, titleTopConstraint) sections.append(section) } + /// Re-publish a section's dynamic search content (its sidebar rows) into the index. A section's + /// rows are rebuilt *outside* the build-time `indexed { }` scope (e.g. ControlsTab's + /// `refreshShortcutRows` from the +/- buttons, a recorder edit, an input-source change, or the + /// pro-lock observer), so the rows' own `registerSearchContent` no-ops there. This re-opens a + /// scope, re-registers the *current* rows, and swaps them into the section wholesale — no stale + /// targets for removed rows, and freshly-added rows become searchable. Re-applies the active + /// query so rebuilt rows light up immediately. Called once per section at the end of + /// `setupView`, then by the owning tab after every rebuild. + func refreshSectionSearchContent(_ id: String) { + guard let section = sections.first(where: { $0.id == id }), + let register = section.registerDynamicSearchContent else { return } + let (_, builder) = SettingsSearchIndex.indexed { register() } + section.setDynamicSearchContent(builder.strings, builder.targets) + if !SettingsSearch.isQueryEmpty(searchField.stringValue) { + applySearch(searchField.stringValue) + } + } + private func updateVisibleSectionsSpacing() { visibleSections.enumerated().forEach { index, section in let isFirst = index == 0 @@ -620,20 +656,26 @@ class SettingsWindow: NSWindow { } } - private func collectSearchContent(_ sectionTitle: LightLabel, _ root: NSView) -> ([String], [SettingsSearchHighlightTarget]) { + private func collectSearchContent(_ sectionTitle: LightLabel, _ root: NSView, skipSidebarRows: Bool) -> ([String], [SettingsSearchHighlightTarget]) { var textValues = [String]() var highlightTargets = [SettingsSearchHighlightTarget]() textValues.append(sectionTitle.stringValue) if let target = SettingsSearchHighlight.highlightTarget(sectionTitle) { highlightTargets.append(target) } - collectSearchContent(root, &textValues, &highlightTargets) + collectSearchContent(root, &textValues, &highlightTargets, skipSidebarRows: skipSidebarRows) return (Array(Set(textValues)), highlightTargets) } private func collectSearchContent(_ root: NSView, _ textValues: inout [String], - _ highlightTargets: inout [SettingsSearchHighlightTarget]) { + _ highlightTargets: inout [SettingsSearchHighlightTarget], + skipSidebarRows: Bool) { + // Sections that rebuild their sidebar rows after construction publish those rows as dynamic + // search content (see `refreshSectionSearchContent`). Skipping `SidebarListRow`s here keeps + // targets for since-removed rows out of the section's fixed base — that staleness was the + // "typing 'sho' no longer highlights Shortcut N" bug. + if skipSidebarRows, root is SidebarListRow { return } if root is ProBadgeView { textValues.append(NSLocalizedString("Pro", comment: "")) return @@ -691,7 +733,7 @@ class SettingsWindow: NSWindow { textValues.append(value) } } - root.subviews.forEach { collectSearchContent($0, &textValues, &highlightTargets) } + root.subviews.forEach { collectSearchContent($0, &textValues, &highlightTargets, skipSidebarRows: skipSidebarRows) } } static func highlightTarget(_ popUpButton: NSPopUpButton) -> SettingsSearchHighlightTarget? { diff --git a/src/preferences/settings-window/SidebarList.swift b/src/preferences/settings-window/SidebarList.swift index 0f88c511..7d357b25 100644 --- a/src/preferences/settings-window/SidebarList.swift +++ b/src/preferences/settings-window/SidebarList.swift @@ -185,12 +185,14 @@ class SidebarListRow: ClickHoverStackView { /// (if any), along with highlight targets for both labels. Call this *after* `setContent` so the /// indexed strings match what the user sees. A no-op outside an `indexed { ... }` scope. /// - /// Why this exists: the fallback `SettingsWindow.collectSearchContent` walk *does* find these - /// labels via the generic `NSTextField` branch — but ControlsTab's sidebar rows are populated - /// by `refreshShortcutRows` (called from `initTab` *inside* the indexed scope, and again from - /// `preferenceChanged` *outside* it). The fallback walk only runs once per section, so rows - /// recreated after the section is added would silently drop out of the index without this - /// explicit registration. + /// Sidebar rows are rebuilt *after* the section's build-time `indexed` scope (ControlsTab's + /// `refreshShortcutRows`), so they can't register themselves at creation — there's no active + /// builder. Instead the owning tab re-publishes all its rows through + /// `SettingsWindow.refreshSectionSearchContent`, which re-opens an `indexed { ... }` scope and + /// calls this on each current row (at build time and after every rebuild). The build-time walk + /// deliberately skips `SidebarListRow`s so those rows live solely in the section's replaceable + /// dynamic content — no stale targets for since-removed rows. Targets read the labels' + /// `stringValue` live, so in-place content edits don't need re-registration. func registerSearchContent() { SettingsSearchIndex.registerString(titleLabel.stringValue) SettingsSearchIndex.registerString(summaryLabel.stringValue) @@ -240,7 +242,13 @@ class SidebarListRow: ClickHoverStackView { } func setProBadge(_ show: Bool) { - proBadge?.removeFromSuperview() + // No-op if already in the requested state. Rows are recycled across refreshes, so this is + // called repeatedly with the same value; without this guard each call would leave the old + // (now badge-less) `wrapper` in `titleRow` and append a new one — the wrappers pile up, + // each adding `titleRow.spacing`, progressively squeezing and truncating the title. + guard show != (proBadge != nil) else { return } + // Remove the whole wrapper from `titleRow`, not just the badge from the wrapper. + proBadge?.superview?.removeFromSuperview() proBadge = nil if show { let badge = ProBadgeView() diff --git a/src/preferences/settings-window/SidebarListSpecs.md b/src/preferences/settings-window/SidebarListSpecs.md new file mode 100644 index 00000000..cb74b8db --- /dev/null +++ b/src/preferences/settings-window/SidebarListSpecs.md @@ -0,0 +1,23 @@ +# SidebarList — Specs + +## Summary + +`SidebarListRow` is the recycled row widget for the shortcut sidebar (ControlsTab) — title, optional +summary, optional **Pro** badge. Because rows are reused across refreshes rather than rebuilt, the +badge mutators must be safe to call repeatedly on the same row. + +## Behavior & edge cases + +- `setProBadge(true)` is idempotent: calling it again on a row that already shows the badge adds + nothing. (The badge lives in a wrapper view inside the title row; a non-idempotent implementation + leaves empty wrappers behind, and their accumulated spacing truncates the title — the bug this + pins.) +- `setProBadge(false)` removes the whole wrapper, returning the row to its no-badge view count — not + just the badge while orphaning the wrapper. + +## Test scenarios + +Mirrors `SidebarListTests.swift` 1:1. + +- **testSetProBadgeDoesNotAccumulateWrappers** — repeated `setProBadge(true)` adds no views and leaves exactly one badge. +- **testSetProBadgeAddsThenFullyRemovesBadge** — `setProBadge(true)` then `setProBadge(false)` returns to the original view count with no badge. diff --git a/src/preferences/settings-window/SidebarListTests.swift b/src/preferences/settings-window/SidebarListTests.swift new file mode 100644 index 00000000..e3a49c8f --- /dev/null +++ b/src/preferences/settings-window/SidebarListTests.swift @@ -0,0 +1,50 @@ +import XCTest +import Cocoa + +/// Coverage for `SidebarListRow`'s Pro-badge management. +/// +/// Why this suite exists: the shortcut sidebar rows are recycled across refreshes (ControlsTab's +/// `refreshShortcutRows`), so `setProBadge` is called repeatedly on the *same* row instead of once +/// on a fresh one. The original implementation removed the badge from its wrapper but left the +/// now-empty wrapper in the title row, then appended a new wrapper — so the wrappers piled up, each +/// adding the title row's spacing, progressively squeezing and head-truncating "Shortcut 2" / +/// "Shortcut 3". These tests pin that `setProBadge` is idempotent and fully removes its wrapper. +/// +/// (`SidebarListRow` compiles into the test target: the deployment target is 10.13 — same as the +/// app — and `_test-support/Mocks.swift` stubs the few app-only symbols it touches, +/// `SettingsWindow.contentWidth` and `SettingsSearchIndex`.) +final class SidebarListTests: XCTestCase { + + private func descendantViews(_ view: NSView) -> [NSView] { + view.subviews + view.subviews.flatMap { descendantViews($0) } + } + + private func badgeCount(_ row: NSView) -> Int { + descendantViews(row).filter { $0 is ProBadgeView }.count + } + + func testSetProBadgeDoesNotAccumulateWrappers() { + let row = SidebarListRow() + row.setContent("Shortcut 2", "") + row.setProBadge(true) + let viewCountWithBadge = descendantViews(row).count + row.setProBadge(true) + row.setProBadge(true) + XCTAssertEqual(descendantViews(row).count, viewCountWithBadge, + "regression: repeated setProBadge(true) on a recycled row must not add views — leftover empty badge wrappers piling up in the title row is what progressively truncated 'Shortcut N'") + XCTAssertEqual(badgeCount(row), 1, + "exactly one badge must remain after repeated setProBadge(true)") + } + + func testSetProBadgeAddsThenFullyRemovesBadge() { + let row = SidebarListRow() + row.setContent("Shortcut 2", "") + let baselineViewCount = descendantViews(row).count + row.setProBadge(true) + XCTAssertEqual(badgeCount(row), 1, "setProBadge(true) adds the badge") + row.setProBadge(false) + XCTAssertEqual(badgeCount(row), 0, "setProBadge(false) removes the badge") + XCTAssertEqual(descendantViews(row).count, baselineViewCount, + "regression: setProBadge(false) must remove the whole wrapper (not leave an empty one behind), returning to the no-badge view count") + } +} diff --git a/src/preferences/settings-window/tabs/controls/ControlsTab.swift b/src/preferences/settings-window/tabs/controls/ControlsTab.swift index 41b9dca9..9ef7403f 100644 --- a/src/preferences/settings-window/tabs/controls/ControlsTab.swift +++ b/src/preferences/settings-window/tabs/controls/ControlsTab.swift @@ -40,7 +40,7 @@ class ControlsTab { // MARK: - Layout constants (unchanged from the old monolithic editor) - private static let shortcutSidebarWidth = CGFloat(175) + private static let shortcutSidebarWidth = CGFloat(180) private static let sidebarRowHeight = CGFloat(52) private static let sidebarHorizontalPadding = TableGroupView.padding private static let shortcutEditorTopBottomPadding = TableGroupView.padding @@ -282,6 +282,9 @@ class ControlsTab { rows.spacing = 0 rows.translatesAutoresizingMaskIntoConstraints = false shortcutRowsStackView = rows + // Start the recycled row pool empty and bound to this freshly-built stack view; + // `refreshShortcutRows` grows it on demand. (`cleanup` also clears it on window close.) + shortcutRows.removeAll() let rowsScrollView = ForwardingVerticalScrollView() rowsScrollView.translatesAutoresizingMaskIntoConstraints = false rowsScrollView.drawsBackground = false @@ -477,29 +480,30 @@ class ControlsTab { private static func refreshShortcutRows() { guard let rows = shortcutRowsStackView else { return } setHoveredShortcutRow(nil) + let count = Preferences.shortcutCount clearArrangedSubviews(rows) - shortcutRows.removeAll(keepingCapacity: true) - for index in 0.. count { + shortcutRows.removeLast(shortcutRows.count - count) + } + for index in 0..= 1 { - row.setProBadge(true) - } - row.onClick = { _, _ in selectShortcut(index) } - row.onMouseEntered = { _, view in setHoveredShortcutRow(view as? SidebarListRow) } - row.onMouseExited = { _, _ in setHoveredShortcutRow(nil) } + row.setProBadge(index >= 1) rows.addArrangedSubview(row) + // Re-create the row↔stack width constraint each layout: AppKit drops it when the row is + // removed from the stack by `clearArrangedSubviews`. The row's height constraint is + // row-internal, set once in `makeShortcutRow`, and survives the remove/re-add cycle. row.widthAnchor.constraint(equalTo: rows.widthAnchor).isActive = true - row.heightAnchor.constraint(equalToConstant: sidebarRowHeight).isActive = true - shortcutRows.append(row) - // Push the row's static title+summary into the active search index so a query that - // matches "Sho..." lights up the sidebar row. The fallback `collectSearchContent` - // walk *should* also catch these via the `NSTextField` cast, but the inline path is - // more robust against ordering (`refreshShortcutRows` may run on the second pass via - // `preferenceChanged`, after the section index has already been frozen). - row.registerSearchContent() - if index < Preferences.shortcutCount - 1 { + if index < count - 1 { let separator = sidebarSeparatorView() rows.addArrangedSubview(separator) separator.leadingAnchor.constraint(equalTo: rows.leadingAnchor, constant: TableGroupView.padding).isActive = true @@ -508,6 +512,21 @@ class ControlsTab { } } syncShortcutSidebarHoverState() + // The rows were (re)built outside the section's build-time `indexed { }` scope, so their own + // `registerSearchContent` can't reach an active builder. Ask the window to re-publish the + // Controls section's dynamic search content from the current rows so a "sho" query keeps + // highlighting them. No-ops until `SettingsWindow.shared` is set (initial build is covered + // by `SettingsWindow.setupView`). + SettingsWindow.shared?.refreshSectionSearchContent("controls") + } + + private static func makeShortcutRow(index: Int) -> SidebarListRow { + let row = SidebarListRow() + row.onClick = { _, _ in selectShortcut(index) } + row.onMouseEntered = { _, view in setHoveredShortcutRow(view as? SidebarListRow) } + row.onMouseExited = { _, _ in setHoveredShortcutRow(nil) } + row.heightAnchor.constraint(equalToConstant: sidebarRowHeight).isActive = true + return row } private static func refreshShortcutSelection() { @@ -631,9 +650,18 @@ class ControlsTab { guard let gestureSidebarRow else { return } gestureSidebarRow.setContent(gestureTitle(), gestureSummary()) gestureSidebarRow.setSelected(selectedShortcutIndex == gestureSelectionIndex) - // See `SidebarListRow.registerSearchContent` — the inline registration here is what - // lets the search query light up the Gesture sidebar row's label. - gestureSidebarRow.registerSearchContent() + } + + /// Register the current sidebar rows (the shortcut rows + the persistent gesture row) into the + /// active search-index builder. Invoked by `SettingsWindow.refreshSectionSearchContent("controls")` + /// inside a fresh `indexed { }` scope — at build time and after every `refreshShortcutRows`. The + /// rows can't self-register at creation because they're (re)built outside the build scope; this + /// is the single place that publishes them, so a "sho" query lights up "Shortcut 1"/"Shortcut 2" + /// and "Gesture". Targets read the labels' `stringValue` live, so in-place content edits don't + /// need re-registration. + static func registerSidebarRowsSearchContent() { + shortcutRows.forEach { $0.registerSearchContent() } + gestureSidebarRow?.registerSearchContent() } // MARK: - Hover state diff --git a/src/preferences/settings-window/tabs/controls/ShortcutEditor.swift b/src/preferences/settings-window/tabs/controls/ShortcutEditor.swift index 188fa432..87762e36 100644 --- a/src/preferences/settings-window/tabs/controls/ShortcutEditor.swift +++ b/src/preferences/settings-window/tabs/controls/ShortcutEditor.swift @@ -218,9 +218,10 @@ final class TriggerBinding { private let nextRecorder: CustomRecorderControl private let holdLabel = NSTextField(labelWithString: "") private let andPressLabel: NSTextField - private let selectNextLabel: NSTextField private static let holdLabelText = NSLocalizedString("Hold", comment: "") private static let andPressText = NSLocalizedString("and press", comment: "") + // Not shown as a trigger-row label anymore; kept as the next-window recorder's human-readable + // name in `shortcutControls`, used by ControlsTab's conflict dialogs (see `conflictLabel`). private static let selectNextText = NSLocalizedString("Select next window", comment: "") private var currentIndex: Int = -1 @@ -231,9 +232,8 @@ final class TriggerBinding { let holdLabelField = LabelAndControl.makeLabel(TriggerBinding.holdLabelText) andPressLabel = LabelAndControl.makeLabel(TriggerBinding.andPressText) - selectNextLabel = LabelAndControl.makeLabel(TriggerBinding.selectNextText) - let stack = NSStackView(views: [holdLabelField, holdRecorder, andPressLabel, nextRecorder, selectNextLabel]) + let stack = NSStackView(views: [holdLabelField, holdRecorder, andPressLabel, nextRecorder]) stack.orientation = .horizontal stack.alignment = .centerY stack.spacing = TableGroupView.spacing