fix: preserve switcher selection across reorders (closes #5665)
Async AX events (focused-window changed, app activated) can fire during the switcher's show path and reorder Windows.list under the panel. The old updateSelectedWindow short-circuited to the initial-pick path whenever the lowest-lastFocusOrder window changed mid-show, throwing away the user's selectedTarget and re-picking the default. Visible slot swap plus highlight jump; if the user released keys early they landed on the wrong window. Extract a pure-function kernel (SelectionResolver) covering the full selection state machine and replace updateSelectedWindow with a thin snapshot/decide/apply wrapper. The actual fix is dropping the focusedWindowChangedWhileShowing arm of shouldSelectFromScratch: the list still reorders (Photoshop finishing launch is fresh state the user wants reflected), but the existing target-by-id lookup re-resolves the user's pick at its new index instead of being skipped. 28 new SelectionResolverTests lock down every selection path: initial pick rules, target preservation across reorders (#5665 regressions), target removal, search-mode flags, edge cases. Also adds minimal Appearance and SettingsSearchIndex stubs to Mocks.swift so the unit-test target builds. ProBadgeView and LightLabel referenced those types without the underlying source being in the test target.
This commit is contained in:
lwouis committed
1 parent
9a47672920
commit
5bd746d342
5 files changed
+557
-92
No files matched your search
@@ -36,6 +36,9 @@
|
||||
4807A6C623A9CD190052A53E /* SkyLight.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 4807A6C523A9CD190052A53E /* SkyLight.framework */; };
|
||||
48F3E16224EC0D8800A1C64B /* Localizable.strings in Resources */ = {isa = PBXBuildFile; fileRef = D04BABDA79F8EF76E3ACDD5F /* Localizable.strings */; };
|
||||
5883F0A829A5182C0071DB65 /* TrackpadEvents.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5883F0A729A5182C0071DB65 /* TrackpadEvents.swift */; };
|
||||
5E1EC110000000000000C002 /* SelectionResolver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5E1EC110000000000000C001 /* SelectionResolver.swift */; };
|
||||
5E1EC110000000000000C003 /* SelectionResolver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5E1EC110000000000000C001 /* SelectionResolver.swift */; };
|
||||
5E1EC110000000000000C102 /* SelectionResolverTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5E1EC110000000000000C101 /* SelectionResolverTests.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 */; };
|
||||
@@ -336,6 +339,8 @@
|
||||
481FE54624D2D387001032F1 /* alt-tab-macos-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "alt-tab-macos-Bridging-Header.h"; sourceTree = "<group>"; };
|
||||
4B4409E4B3D98C491503CE5D /* OverrideClickResolverTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OverrideClickResolverTests.swift; sourceTree = "<group>"; };
|
||||
5883F0A729A5182C0071DB65 /* TrackpadEvents.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TrackpadEvents.swift; sourceTree = "<group>"; };
|
||||
5E1EC110000000000000C001 /* SelectionResolver.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SelectionResolver.swift; sourceTree = "<group>"; };
|
||||
5E1EC110000000000000C101 /* SelectionResolverTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SelectionResolverTests.swift; sourceTree = "<group>"; };
|
||||
5F04852D2F09AC9700235D4A /* WindowCaptureEvents.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowCaptureEvents.swift; sourceTree = "<group>"; };
|
||||
5F05D0E62F31441A004A95F9 /* AGENTS.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = AGENTS.md; sourceTree = "<group>"; };
|
||||
5F05D0E72F31441A004A95F9 /* CLAUDE.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = CLAUDE.md; sourceTree = "<group>"; };
|
||||
@@ -835,6 +840,7 @@
|
||||
5F791E4A2F1C65E100B6419C /* ApplicationDiscriminator.swift */,
|
||||
5F906A332F16A18000B99924 /* Screens.swift */,
|
||||
D04BA7CF9C2D1BEC7C05AB24 /* Spaces.swift */,
|
||||
5E1EC110000000000000C001 /* SelectionResolver.swift */,
|
||||
);
|
||||
path = state;
|
||||
sourceTree = "<group>";
|
||||
@@ -905,6 +911,7 @@
|
||||
BF0CBBBB0000000000000001 /* OnActionExtensionTests.swift */,
|
||||
25966EE691FC6871588D796B /* ProBadgeViewSegmentTests.swift */,
|
||||
4B4409E4B3D98C491503CE5D /* OverrideClickResolverTests.swift */,
|
||||
5E1EC110000000000000C101 /* SelectionResolverTests.swift */,
|
||||
BF0C829EAA93A4C4ADB42CF8 /* Mocks.swift */,
|
||||
BF0C8C6810FF50908E637A48 /* ConcurrentScreenshots.swift */,
|
||||
BF0C855CD9DBCAEFE48F9EA5 /* AXUIElementTests.swift */,
|
||||
@@ -1748,6 +1755,8 @@
|
||||
A053BAD6681629878E72B1F7 /* ProBadgeViewSegmentTests.swift in Sources */,
|
||||
DC8CF4F01C999497352736F1 /* OverrideClickResolverTests.swift in Sources */,
|
||||
DC06402457ECCE6C902FEAE4 /* OverrideClickResolver.swift in Sources */,
|
||||
5E1EC110000000000000C003 /* SelectionResolver.swift in Sources */,
|
||||
5E1EC110000000000000C102 /* SelectionResolverTests.swift in Sources */,
|
||||
8B4A0B173EEB0648424219B5 /* ProBadgeView.swift in Sources */,
|
||||
6D42BE46F595D567EDBF559A /* DynamicColorTextField.swift in Sources */,
|
||||
BF0C8C5FA4B401A8C0F75B7A /* KeyboardEventsTestable.swift in Sources */,
|
||||
@@ -1789,6 +1798,7 @@
|
||||
5F21C9E22C6E94410091F72F /* AdditionalControlsSheet.swift in Sources */,
|
||||
D04BA8480A8FF466CA89DA5B /* main.swift in Sources */,
|
||||
D04BAFB973C3D28718FAEB87 /* Windows.swift in Sources */,
|
||||
5E1EC110000000000000C002 /* SelectionResolver.swift in Sources */,
|
||||
F00DCAFE0000000000001002 /* Search.swift in Sources */,
|
||||
F00DCAFE0000000000001003 /* SearchTestable.swift in Sources */,
|
||||
D04BAC011A71E0418154F8CD /* Preferences.swift in Sources */,
|
||||
|
||||
@@ -0,0 +1,157 @@
|
||||
import Foundation
|
||||
|
||||
/// One window as seen by the selection kernel — just the fields selection logic needs.
|
||||
/// Decouples the pure decision from the `Window` AppKit class so this is unit-testable.
|
||||
struct SelectionWindow: Equatable {
|
||||
let id: String
|
||||
let visible: Bool
|
||||
let lastFocusOrder: Int
|
||||
let isMinimized: Bool
|
||||
let isWindowlessApp: Bool
|
||||
}
|
||||
|
||||
/// Snapshot of everything the kernel needs to pick the next selection. No globals, no AppKit.
|
||||
struct SelectionInputs: Equatable {
|
||||
let list: [SelectionWindow]
|
||||
let selectedIndex: Int
|
||||
let selectedTarget: String?
|
||||
/// True iff `Preferences.windowOrder[shortcutIndex] != .recentlyFocused`
|
||||
/// AND `Applications.frontmostPid != nil`. Gates the alpha/space-ordering initial-pick path.
|
||||
let useLastFocusedRule: Bool
|
||||
let restoreDefaultOnSearchClear: Bool
|
||||
let bestMatchOnSearchChange: Bool
|
||||
}
|
||||
|
||||
/// What the kernel recommends. Wrapper translates this into side effects (highlight redraws,
|
||||
/// scroll-to-visible, thumbnail preview, etc.).
|
||||
enum SelectionDecision: Equatable {
|
||||
/// Post-firstVisible-guard empty case. Wrapper: clear `selectedTarget` and `hoveredIndex`.
|
||||
case clearTargetAndHover
|
||||
|
||||
/// "From scratch" initial-pick path with a valid pick. Wrapper: clear hovered + reset
|
||||
/// `selectedTarget` first, then move selection to `index`.
|
||||
case resetThenSelect(Int)
|
||||
|
||||
/// "From scratch" but list has nothing to land on (e.g. search-clear with no visible).
|
||||
/// Wrapper: clear hovered + reset `selectedTarget`; leave `selectedIndex`.
|
||||
case resetWithoutSelection
|
||||
|
||||
/// Move selection to `index`. `selectedTarget` follows to `list[index].id`.
|
||||
case selectAt(Int)
|
||||
|
||||
/// `selectedIndex` is fine; just ensure `selectedTarget == list[index].id` (backfill).
|
||||
case ensureTargetSet(Int)
|
||||
}
|
||||
|
||||
enum SelectionResolver {
|
||||
/// Pure port of `Windows.updateSelectedWindow`. Branching order matches the original so the
|
||||
/// behavior-preserving extraction can be verified against the running app before we touch
|
||||
/// the logic.
|
||||
static func decide(_ i: SelectionInputs) -> SelectionDecision {
|
||||
// 1) Search-clear path takes precedence — runs even when no visible windows.
|
||||
if i.restoreDefaultOnSearchClear {
|
||||
return resetInitialPick(i)
|
||||
}
|
||||
// 2) Empty visible list.
|
||||
let visibleIndexes = i.list.indices.filter { i.list[$0].visible }
|
||||
guard let firstVisibleIndex = visibleIndexes.first else {
|
||||
return .clearTargetAndHover
|
||||
}
|
||||
// 3) Search-best-match path.
|
||||
if i.bestMatchOnSearchChange {
|
||||
return .selectAt(firstVisibleIndex)
|
||||
}
|
||||
// 4) "From scratch" only when there's no user selection yet — first refresh of the
|
||||
// session. Previously this also fired whenever the MRU-focused window changed mid-show,
|
||||
// which short-circuited past target preservation and made the highlight jump (#5665).
|
||||
// Removed: AX events that reorder the list during display now fall through to
|
||||
// `findTarget` below, keeping the user's pick stable.
|
||||
if i.selectedTarget == nil {
|
||||
return resetInitialPick(i)
|
||||
}
|
||||
// 5) Try to restore the user's chosen target by id.
|
||||
if let targetIndex = findTarget(i.list, i.selectedTarget) {
|
||||
return .selectAt(targetIndex)
|
||||
}
|
||||
// 6) Target gone — adapt to the closest visible.
|
||||
return adapt(i, visibleIndexes: visibleIndexes, firstVisible: firstVisibleIndex, lastVisible: visibleIndexes.last!)
|
||||
}
|
||||
|
||||
/// Mirrors `setInitialSelectedAndHoveredWindowIndex` — picks the index, defers reset to wrapper.
|
||||
static func initialPickIndex(_ i: SelectionInputs) -> Int? {
|
||||
if i.useLastFocusedRule, let idx = getLastFocusedOrderWindowIndex(i.list) {
|
||||
return idx
|
||||
}
|
||||
// Edge case: top two windows both minimized — land on index 0 rather than cycling past.
|
||||
if i.list.count >= 2 && i.list[0].isMinimized && i.list[1].isMinimized {
|
||||
return i.list[0].visible ? 0 : nil
|
||||
}
|
||||
return cycleFromZero(i.list)
|
||||
}
|
||||
|
||||
/// Cycles from index 0 by step +1, wrapping around the list, stopping at the first visible
|
||||
/// window. Returns 0 if only index 0 is visible (cycle wraps back). Returns nil if no
|
||||
/// visible window exists.
|
||||
static func cycleFromZero(_ list: [SelectionWindow]) -> Int? {
|
||||
guard !list.isEmpty, list.contains(where: { $0.visible }) else { return nil }
|
||||
var idx = 0
|
||||
for _ in 0..<list.count {
|
||||
idx = (idx + 1) % list.count
|
||||
if list[idx].visible {
|
||||
return idx
|
||||
}
|
||||
}
|
||||
return 0 // only list[0] is visible — cycle wrapped back
|
||||
}
|
||||
|
||||
/// Returns the index of the visible non-windowless window with the lowest `lastFocusOrder`.
|
||||
/// Mirrors `Windows.getLastFocusedOrderWindowIndex`.
|
||||
static func getLastFocusedOrderWindowIndex(_ list: [SelectionWindow]) -> Int? {
|
||||
var bestIndex: Int? = nil
|
||||
var bestOrder = Int.max
|
||||
for (idx, w) in list.enumerated() {
|
||||
if !w.isWindowlessApp && w.visible && w.lastFocusOrder < bestOrder {
|
||||
bestOrder = w.lastFocusOrder
|
||||
bestIndex = idx
|
||||
}
|
||||
}
|
||||
return bestIndex
|
||||
}
|
||||
|
||||
/// Find the user's chosen window by id, returning its current index if visible.
|
||||
/// Mirrors the lookup in the old `Windows.restoreSelectionTargetIfVisible`.
|
||||
static func findTarget(_ list: [SelectionWindow], _ targetId: String?) -> Int? {
|
||||
guard let targetId else { return nil }
|
||||
return list.firstIndex { $0.id == targetId && $0.visible }
|
||||
}
|
||||
|
||||
// MARK: - Internal
|
||||
|
||||
private static func resetInitialPick(_ i: SelectionInputs) -> SelectionDecision {
|
||||
if let idx = initialPickIndex(i) {
|
||||
return .resetThenSelect(idx)
|
||||
}
|
||||
return .resetWithoutSelection
|
||||
}
|
||||
|
||||
/// Mirrors `adaptSelectionToVisibleIndexes`. `visibleIndexes` is non-empty by caller's guard.
|
||||
private static func adapt(_ i: SelectionInputs, visibleIndexes: [Int], firstVisible: Int, lastVisible: Int) -> SelectionDecision {
|
||||
if !visibleIndexes.contains(i.selectedIndex) {
|
||||
let closest = visibleIndexes.last(where: { $0 < i.selectedIndex }) ?? lastVisible
|
||||
return .selectAt(closest)
|
||||
}
|
||||
if i.selectedIndex > lastVisible {
|
||||
return .selectAt(lastVisible)
|
||||
}
|
||||
if i.selectedIndex < firstVisible {
|
||||
return .selectAt(firstVisible)
|
||||
}
|
||||
if i.selectedTarget == nil {
|
||||
return .ensureTargetSet(i.selectedIndex)
|
||||
}
|
||||
// selectedIndex valid AND target already set — nothing to do.
|
||||
// We still return `ensureTargetSet` (idempotent: same target value) to keep the enum
|
||||
// surface minimal. The wrapper treats a no-change as a no-op.
|
||||
return .ensureTargetSet(i.selectedIndex)
|
||||
}
|
||||
}
|
||||
@@ -3,8 +3,6 @@ import Cocoa
|
||||
class Windows {
|
||||
static var list = [Window]()
|
||||
private(set) static var byWindowId = [CGWindowID: Window]()
|
||||
// we use this to track if the focused window changed while alt-tab was open
|
||||
private static var lastFocusedWindowTarget: String?
|
||||
private static var lastWindowActivityType = WindowActivityType.none
|
||||
private static var shouldSelectBestMatchOnSearchChange = false
|
||||
private static var shouldRestoreDefaultSelectionOnSearchClear = false
|
||||
@@ -187,6 +185,69 @@ class Windows {
|
||||
|
||||
static func setInitialSelectedAndHoveredWindowIndex() {
|
||||
guard let session = SwitcherSession.current else { return }
|
||||
let snapshot = selectionSnapshot()
|
||||
let inputs = makeSelectionInputs(snapshot, session: session)
|
||||
let pickIndex = SelectionResolver.initialPickIndex(inputs)
|
||||
resetForInitialPick(session)
|
||||
if let idx = pickIndex {
|
||||
updateSelectedAndHoveredWindowIndex(idx)
|
||||
}
|
||||
}
|
||||
|
||||
static func updateSelectedWindow() {
|
||||
guard let session = SwitcherSession.current else { return }
|
||||
let snapshot = selectionSnapshot()
|
||||
let inputs = makeSelectionInputs(snapshot, session: session)
|
||||
let decision = SelectionResolver.decide(inputs)
|
||||
shouldRestoreDefaultSelectionOnSearchClear = false
|
||||
shouldSelectBestMatchOnSearchChange = false
|
||||
applySelectionDecision(decision, session: session)
|
||||
}
|
||||
|
||||
/// Project `list` into the kernel's window view (just the fields selection needs).
|
||||
private static func selectionSnapshot() -> [SelectionWindow] {
|
||||
list.map {
|
||||
SelectionWindow(id: $0.id,
|
||||
visible: shouldDisplay($0),
|
||||
lastFocusOrder: $0.lastFocusOrder,
|
||||
isMinimized: $0.isMinimized,
|
||||
isWindowlessApp: $0.isWindowlessApp)
|
||||
}
|
||||
}
|
||||
|
||||
private static func makeSelectionInputs(_ snapshot: [SelectionWindow], session: SwitcherSession) -> SelectionInputs {
|
||||
SelectionInputs(
|
||||
list: snapshot,
|
||||
selectedIndex: session.selectedIndex,
|
||||
selectedTarget: session.selectedTarget,
|
||||
useLastFocusedRule: Applications.frontmostPid != nil
|
||||
&& Preferences.windowOrder[session.shortcutIndex] != .recentlyFocused,
|
||||
restoreDefaultOnSearchClear: shouldRestoreDefaultSelectionOnSearchClear,
|
||||
bestMatchOnSearchChange: shouldSelectBestMatchOnSearchChange)
|
||||
}
|
||||
|
||||
private static func applySelectionDecision(_ decision: SelectionDecision, session: SwitcherSession) {
|
||||
switch decision {
|
||||
case .clearTargetAndHover:
|
||||
session.selectedTarget = nil
|
||||
session.hoveredIndex = nil
|
||||
case .resetThenSelect(let idx):
|
||||
resetForInitialPick(session)
|
||||
updateSelectedAndHoveredWindowIndex(idx)
|
||||
case .resetWithoutSelection:
|
||||
resetForInitialPick(session)
|
||||
case .selectAt(let idx):
|
||||
updateSelectedAndHoveredWindowIndex(idx)
|
||||
case .ensureTargetSet(let idx):
|
||||
if session.selectedTarget == nil && idx < list.count {
|
||||
session.selectedTarget = list[idx].id
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Wrapper-side reset that mirrors the first half of the old `setInitialSelectedAndHoveredWindowIndex`:
|
||||
/// clear `selectedTarget`, reset `selectedIndex` to 0, redraw the old highlight, drop hover.
|
||||
private static func resetForInitialPick(_ session: SwitcherSession) {
|
||||
let oldIndex = session.selectedIndex
|
||||
session.selectedIndex = 0
|
||||
session.selectedTarget = nil
|
||||
@@ -195,95 +256,6 @@ class Windows {
|
||||
session.hoveredIndex = nil
|
||||
TilesView.highlight(oldHovered)
|
||||
}
|
||||
if Applications.frontmostPid != nil,
|
||||
Preferences.windowOrder[session.shortcutIndex] != .recentlyFocused,
|
||||
let lastFocusedOrderWindowIndex = getLastFocusedOrderWindowIndex() {
|
||||
updateSelectedAndHoveredWindowIndex(lastFocusedOrderWindowIndex)
|
||||
} else {
|
||||
// edge-case: when the 2 most recently focused windows are both minimized, select the first
|
||||
if list.count >= 2 && list[0].isMinimized && list[1].isMinimized {
|
||||
updateSelectedAndHoveredWindowIndex(0)
|
||||
} else {
|
||||
cycleSelectedWindowIndex(1)
|
||||
if session.selectedIndex == 0 {
|
||||
updateSelectedAndHoveredWindowIndex(0)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static func updateSelectedWindow() {
|
||||
guard let session = SwitcherSession.current else { return }
|
||||
let focusedWindowTarget = currentFocusedWindowTarget()
|
||||
defer { lastFocusedWindowTarget = focusedWindowTarget }
|
||||
if shouldRestoreDefaultSelectionOnSearchClear {
|
||||
shouldRestoreDefaultSelectionOnSearchClear = false
|
||||
setInitialSelectedAndHoveredWindowIndex()
|
||||
return
|
||||
}
|
||||
let visibleIndexes = visibleWindowIndexes()
|
||||
guard let firstVisibleIndex = visibleIndexes.first else {
|
||||
session.selectedTarget = nil
|
||||
session.hoveredIndex = nil
|
||||
return
|
||||
}
|
||||
if shouldSelectBestMatchOnSearchChange {
|
||||
shouldSelectBestMatchOnSearchChange = false
|
||||
updateSelectedAndHoveredWindowIndex(firstVisibleIndex)
|
||||
return
|
||||
}
|
||||
if shouldSelectFromScratch(focusedWindowTarget) {
|
||||
setInitialSelectedAndHoveredWindowIndex()
|
||||
return
|
||||
}
|
||||
if restoreSelectionTargetIfVisible() { return }
|
||||
adaptSelectionToVisibleIndexes(visibleIndexes, firstVisibleIndex)
|
||||
}
|
||||
|
||||
private static func visibleWindowIndexes() -> [Int] {
|
||||
list.indices.filter { shouldDisplay(list[$0]) }
|
||||
}
|
||||
|
||||
private static func currentFocusedWindowTarget() -> String? {
|
||||
getLastFocusedOrderWindowIndex().map { list[$0].id }
|
||||
}
|
||||
|
||||
private static func shouldSelectFromScratch(_ focusedWindowTarget: String?) -> Bool {
|
||||
SwitcherSession.current?.selectedTarget == nil || focusedWindowChangedWhileShowing(focusedWindowTarget)
|
||||
}
|
||||
|
||||
private static func focusedWindowChangedWhileShowing(_ focusedWindowTarget: String?) -> Bool {
|
||||
guard let session = SwitcherSession.current, Search.normalizedQuery(session.searchQuery).isEmpty else { return false }
|
||||
guard let lastFocusedWindowTarget, let focusedWindowTarget else { return false }
|
||||
return focusedWindowTarget != lastFocusedWindowTarget
|
||||
}
|
||||
|
||||
private static func restoreSelectionTargetIfVisible() -> Bool {
|
||||
guard let session = SwitcherSession.current, let target = session.selectedTarget else { return false }
|
||||
guard let index = list.firstIndex(where: { $0.id == target && shouldDisplay($0) }) else { return false }
|
||||
if index == session.selectedIndex { return true }
|
||||
updateSelectedAndHoveredWindowIndex(index)
|
||||
return true
|
||||
}
|
||||
|
||||
private static func adaptSelectionToVisibleIndexes(_ visibleIndexes: [Int], _ firstVisibleIndex: Int) {
|
||||
guard let session = SwitcherSession.current, let lastVisibleIndex = visibleIndexes.last else { return }
|
||||
if !visibleIndexes.contains(session.selectedIndex) {
|
||||
let closest = visibleIndexes.last(where: { $0 < session.selectedIndex }) ?? lastVisibleIndex
|
||||
updateSelectedAndHoveredWindowIndex(closest)
|
||||
return
|
||||
}
|
||||
if session.selectedIndex > lastVisibleIndex {
|
||||
updateSelectedAndHoveredWindowIndex(lastVisibleIndex)
|
||||
return
|
||||
}
|
||||
if session.selectedIndex < firstVisibleIndex {
|
||||
updateSelectedAndHoveredWindowIndex(firstVisibleIndex)
|
||||
return
|
||||
}
|
||||
if session.selectedTarget == nil {
|
||||
session.selectedTarget = list[session.selectedIndex].id
|
||||
}
|
||||
}
|
||||
|
||||
static func updateSelectedAndHoveredWindowIndex(_ newIndex: Int, _ fromMouse: Bool = false) {
|
||||
@@ -545,7 +517,6 @@ class Windows {
|
||||
if addWindowlessWindowIfNeeded {
|
||||
windows.forEach { $0.application.addWindowlessWindowIfNeeded() }
|
||||
}
|
||||
lastFocusedWindowTarget = getLastFocusedOrderWindowIndex().map { list[$0].id }
|
||||
App.refreshOpenUiAfterExternalEvent([], windowRemoved: true)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +19,18 @@ extension NSImage {
|
||||
}
|
||||
}
|
||||
|
||||
// `LightLabel` and `ProBadgeView` live in the test target's source membership (they're pulled
|
||||
// in by `ProBadgeViewSegmentTests`) but reference `Appearance` and `SettingsSearchIndex` —
|
||||
// types not in the test target. Minimal stubs so the test target compiles. Tests never exercise
|
||||
// these code paths; the stubs just satisfy the type checker.
|
||||
enum SettingsSearchIndex {
|
||||
static func registerString(_ s: String?) {}
|
||||
}
|
||||
|
||||
class Appearance {
|
||||
static var searchMatchHighlightColor: NSColor = .yellow
|
||||
}
|
||||
|
||||
enum SearchKeyResult {
|
||||
case handled
|
||||
case passToField
|
||||
|
||||
@@ -0,0 +1,315 @@
|
||||
import XCTest
|
||||
|
||||
/// Scenarios pinning the selection-state machine that drives which tile is highlighted while
|
||||
/// the switcher is open. Each test builds a fully explicit `SelectionInputs` and asserts on the
|
||||
/// `SelectionDecision` the kernel returns — no globals, no AppKit, no async. Future refactors
|
||||
/// of selection must keep this suite green.
|
||||
///
|
||||
/// Test method names follow `test<Group><Scenario>`. Groups:
|
||||
/// - InitialPick (A): first call after the switcher opens (`selectedTarget == nil`)
|
||||
/// - PreserveTarget (B): list reorders but the user's chosen tile stays selected (#5665)
|
||||
/// - TargetRemoved (C): user's window closed / filtered out
|
||||
/// - SearchMode (D): search filter / clear interactions
|
||||
/// - EdgeCases (E): corruption, single-window flips, multi-step sequences
|
||||
final class SelectionResolverTests: XCTestCase {
|
||||
|
||||
// MARK: - Builders
|
||||
|
||||
/// Concise window builder. Defaults model the common case: visible, non-minimized, non-windowless.
|
||||
private func w(_ id: String, focusOrder: Int = 0, visible: Bool = true,
|
||||
minimized: Bool = false, windowless: Bool = false) -> SelectionWindow {
|
||||
SelectionWindow(id: id, visible: visible, lastFocusOrder: focusOrder,
|
||||
isMinimized: minimized, isWindowlessApp: windowless)
|
||||
}
|
||||
|
||||
/// Mid-session refresh with most knobs in their default position. The test customizes only
|
||||
/// what its scenario cares about.
|
||||
private func inputs(list: [SelectionWindow],
|
||||
selectedIndex: Int = 0,
|
||||
selectedTarget: String? = nil,
|
||||
useLastFocusedRule: Bool = false,
|
||||
restoreDefaultOnSearchClear: Bool = false,
|
||||
bestMatchOnSearchChange: Bool = false) -> SelectionInputs {
|
||||
SelectionInputs(list: list,
|
||||
selectedIndex: selectedIndex,
|
||||
selectedTarget: selectedTarget,
|
||||
useLastFocusedRule: useLastFocusedRule,
|
||||
restoreDefaultOnSearchClear: restoreDefaultOnSearchClear,
|
||||
bestMatchOnSearchChange: bestMatchOnSearchChange)
|
||||
}
|
||||
|
||||
// MARK: - A. Initial pick (`selectedTarget == nil`)
|
||||
|
||||
/// A1. No windows at all — wrapper clears `selectedTarget` / `hoveredIndex`.
|
||||
func testInitialPickEmptyList() {
|
||||
let i = inputs(list: [])
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .clearTargetAndHover)
|
||||
}
|
||||
|
||||
/// A2. Single visible window — cycle from 0 wraps back to 0; that's the only choice.
|
||||
func testInitialPickSingleVisible() {
|
||||
let i = inputs(list: [w("a")])
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .resetThenSelect(0))
|
||||
}
|
||||
|
||||
/// A3. Default Cmd-Tab behavior: cycle off slot 0 to slot 1 ("previous app").
|
||||
func testInitialPickTwoVisibleDefaultRules() {
|
||||
let i = inputs(list: [w("a"), w("b")])
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .resetThenSelect(1))
|
||||
}
|
||||
|
||||
/// A4. Edge case at Windows.swift:204 — both top windows minimized, pick index 0 instead
|
||||
/// of cycling. Behavior preserved from original (#5665 doesn't touch this).
|
||||
func testInitialPickTopTwoMinimized() {
|
||||
let i = inputs(list: [w("a", minimized: true), w("b", minimized: true), w("c")])
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .resetThenSelect(0))
|
||||
}
|
||||
|
||||
/// A5. Alpha/space ordering with frontmost set — pick the lowest-`lastFocusOrder` visible
|
||||
/// non-windowless window, regardless of its slot position in the alpha-sorted list.
|
||||
func testInitialPickUseLastFocusedRule() {
|
||||
let list = [w("a", focusOrder: 5), w("b", focusOrder: 0), w("c", focusOrder: 3)]
|
||||
let i = inputs(list: list, useLastFocusedRule: true)
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .resetThenSelect(1))
|
||||
}
|
||||
|
||||
/// A6. All windows filtered out (e.g. by search). Same as A1 — empty visible list.
|
||||
func testInitialPickAllInvisible() {
|
||||
let list = [w("a", visible: false), w("b", visible: false), w("c", visible: false)]
|
||||
let i = inputs(list: list)
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .clearTargetAndHover)
|
||||
}
|
||||
|
||||
/// A5 corollary (was E4 in the plan): windowless app entries are skipped when scanning for
|
||||
/// the lowest `lastFocusOrder` — they're not real windows the user can pick.
|
||||
func testInitialPickSkipsWindowlessInLastFocusedRule() {
|
||||
let list = [
|
||||
w("dock", focusOrder: 0, windowless: true),
|
||||
w("real", focusOrder: 1),
|
||||
w("other", focusOrder: 2),
|
||||
]
|
||||
let i = inputs(list: list, useLastFocusedRule: true)
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .resetThenSelect(1))
|
||||
}
|
||||
|
||||
// MARK: - B. Preserve target across reorders — the #5665 regression cluster
|
||||
|
||||
/// B1. Trivial preservation — target's still at its index after a benign refresh.
|
||||
func testPreserveTargetSameIndex() {
|
||||
let list = [w("a", focusOrder: 0), w("b", focusOrder: 1)]
|
||||
let i = inputs(list: list, selectedIndex: 1, selectedTarget: "b")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(1))
|
||||
}
|
||||
|
||||
/// B2. #5665 regression — user picked "b" at slot 1, then Photoshop ("p") finished launching
|
||||
/// and stole focus. List reorders so p=0, a=1, b=2. The user's selection must follow "b" to
|
||||
/// slot 2. Pre-fix, the kernel re-picked from scratch instead, jumping the highlight.
|
||||
func testPreserveTargetMovedToHigherIndexAfterPhotoshopLaunch() {
|
||||
let listAfterReorder = [
|
||||
w("p", focusOrder: 0), // Photoshop just launched, top of MRU
|
||||
w("a", focusOrder: 1), // was the front app
|
||||
w("b", focusOrder: 2), // user's pick, slid down
|
||||
]
|
||||
let i = inputs(list: listAfterReorder, selectedIndex: 1, selectedTarget: "b")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(2),
|
||||
"regression for #5665 — the user picked 'b'; the highlight must follow 'b' " +
|
||||
"to its new slot, not jump back to a 'previous app' re-pick.")
|
||||
}
|
||||
|
||||
/// B3. Same as B2 but the target moved DOWN the list — e.g. an app's window closed,
|
||||
/// shifting indices up.
|
||||
func testPreserveTargetMovedToLowerIndex() {
|
||||
// Before refresh: list was [x, a, b, c, target]. Mid-session: "x" closed.
|
||||
// Now: [a, b, target, c]. Target was at index 4, is now at index 2.
|
||||
let list = [w("a", focusOrder: 0), w("b", focusOrder: 1),
|
||||
w("target", focusOrder: 2), w("c", focusOrder: 3)]
|
||||
let i = inputs(list: list, selectedIndex: 4, selectedTarget: "target")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(2),
|
||||
"regression for #5665 — target moved to a different slot; highlight must follow.")
|
||||
}
|
||||
|
||||
/// B4. The list churns but the target happens to land at the same index.
|
||||
/// (Defensive — kernel shouldn't notice or care.)
|
||||
func testPreserveTargetIndexUnchangedByCoincidence() {
|
||||
let list = [w("x", focusOrder: 0), w("target", focusOrder: 1), w("y", focusOrder: 2)]
|
||||
let i = inputs(list: list, selectedIndex: 1, selectedTarget: "target")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(1))
|
||||
}
|
||||
|
||||
/// B5. New window appears at the END of the list — target slot is unchanged.
|
||||
func testPreserveTargetNewWindowAppended() {
|
||||
let list = [
|
||||
w("a", focusOrder: 0),
|
||||
w("b", focusOrder: 1),
|
||||
w("newly-launched", focusOrder: 99), // appended via Windows.appendWindow
|
||||
]
|
||||
let i = inputs(list: list, selectedIndex: 1, selectedTarget: "b")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(1))
|
||||
}
|
||||
|
||||
/// B6. Multiple sequential reorders during one session — the target tracks correctly across
|
||||
/// every refresh. Simulates a chatty Electron app firing focus events repeatedly.
|
||||
func testPreserveTargetAcrossMultipleReorders() {
|
||||
// Step 1: user picks "b" at slot 1.
|
||||
let step1 = inputs(
|
||||
list: [w("a", focusOrder: 0), w("b", focusOrder: 1), w("c", focusOrder: 2)],
|
||||
selectedIndex: 1, selectedTarget: "b")
|
||||
XCTAssertEqual(SelectionResolver.decide(step1), .selectAt(1))
|
||||
// Step 2: c steals focus. List reorders: c=0, a=1, b=2. Target now at index 2.
|
||||
let step2 = inputs(
|
||||
list: [w("c", focusOrder: 0), w("a", focusOrder: 1), w("b", focusOrder: 2)],
|
||||
selectedIndex: 1, selectedTarget: "b")
|
||||
XCTAssertEqual(SelectionResolver.decide(step2), .selectAt(2))
|
||||
// Step 3: a steals focus back. List reorders: a=0, c=1, b=2. Target stays at index 2.
|
||||
let step3 = inputs(
|
||||
list: [w("a", focusOrder: 0), w("c", focusOrder: 1), w("b", focusOrder: 2)],
|
||||
selectedIndex: 2, selectedTarget: "b")
|
||||
XCTAssertEqual(SelectionResolver.decide(step3), .selectAt(2))
|
||||
}
|
||||
|
||||
// MARK: - C. Target removed / no longer visible
|
||||
|
||||
/// C1. User's picked window closed externally. The id is no longer in the list. Fall through
|
||||
/// to `adapt` and end on the previous `selectedIndex` (target backfill).
|
||||
func testTargetRemovedAdaptToClosestBelow() {
|
||||
// Originally: [a, b, c, d]; user picked "c" at index 2. Then "c" closed:
|
||||
let list = [w("a", focusOrder: 0), w("b", focusOrder: 1), w("d", focusOrder: 3)]
|
||||
let i = inputs(list: list, selectedIndex: 2, selectedTarget: "c")
|
||||
// visibleIndexes = [0, 1, 2]. selectedIndex (2) is in range and equals lastVisible,
|
||||
// selectedTarget != nil but lookup fails. Tail branch: ensureTargetSet(2) — list[2] is "d"
|
||||
// and the wrapper backfills the target to "d".
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .ensureTargetSet(2))
|
||||
}
|
||||
|
||||
/// C1 variant: original `selectedIndex` is now out of bounds for the smaller list.
|
||||
func testTargetRemovedSelectedIndexOutOfBounds() {
|
||||
// Originally 4 windows, selectedIndex=3. Then targets+others closed: only [a, b] left.
|
||||
let list = [w("a", focusOrder: 0), w("b", focusOrder: 1)]
|
||||
let i = inputs(list: list, selectedIndex: 3, selectedTarget: "d")
|
||||
// adapt: visibleIndexes=[0,1] doesn't contain 3 → closest visible < 3 is 1 → selectAt(1).
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(1))
|
||||
}
|
||||
|
||||
/// C2. Target still in `list` but filtered out by `visible == false` (e.g. search match miss
|
||||
/// or app went to a non-visible space). Same flow as C1 — target lookup excludes invisible.
|
||||
func testTargetBecameInvisible() {
|
||||
let list = [w("a"), w("b", visible: false), w("c")]
|
||||
let i = inputs(list: list, selectedIndex: 1, selectedTarget: "b")
|
||||
// visibleIndexes = [0, 2]. selectedIndex (1) not in [0,2] → closest visible < 1 is 0.
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(0))
|
||||
}
|
||||
|
||||
/// C3. Target removed AND list emptied — `clearTargetAndHover` path takes over.
|
||||
func testTargetRemovedAndListEmptied() {
|
||||
let i = inputs(list: [], selectedIndex: 0, selectedTarget: "gone")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .clearTargetAndHover)
|
||||
}
|
||||
|
||||
/// C4. Target was at slot 0, closed; only one visible window remains.
|
||||
func testTargetRemovedOnlyOneLeft() {
|
||||
// Originally [other, target]; target closed. Now: [other].
|
||||
let list = [w("other")]
|
||||
let i = inputs(list: list, selectedIndex: 0, selectedTarget: "target")
|
||||
// visibleIndexes=[0]. selectedIndex=0 in range. selectedTarget != nil but lookup fails.
|
||||
// Tail returns ensureTargetSet(0); wrapper backfills target to "other".
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .ensureTargetSet(0))
|
||||
}
|
||||
|
||||
// MARK: - D. Search-mode interactions
|
||||
|
||||
/// D1. User types a search query that produces a new best match — jump to firstVisible.
|
||||
func testSearchBestMatchOnSearchChange() {
|
||||
let list = [w("a", visible: false), w("b"), w("c")]
|
||||
let i = inputs(list: list, selectedIndex: 0, selectedTarget: "a",
|
||||
bestMatchOnSearchChange: true)
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(1))
|
||||
}
|
||||
|
||||
/// D2. User cleared a non-empty search query — restore the default initial pick.
|
||||
func testSearchRestoreDefaultOnClear() {
|
||||
let list = [w("a"), w("b"), w("c")]
|
||||
let i = inputs(list: list, selectedIndex: 0, selectedTarget: "a",
|
||||
restoreDefaultOnSearchClear: true)
|
||||
// Same flow as A3 — default rules, picks index 1.
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .resetThenSelect(1))
|
||||
}
|
||||
|
||||
/// D3. Target preservation works the same whether or not search is active. (Pre-fix this
|
||||
/// scenario was specifically interesting because `focusedWindowChangedWhileShowing` had a
|
||||
/// search-empty guard; with the fix the guard is irrelevant.)
|
||||
func testTargetPreservedInSearchMode() {
|
||||
let list = [w("p", focusOrder: 0), w("a", focusOrder: 1), w("b", focusOrder: 2)]
|
||||
let i = inputs(list: list, selectedIndex: 2, selectedTarget: "b")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(2))
|
||||
}
|
||||
|
||||
/// D4. Search filters out the user's pick, but other matches remain — adapt to closest.
|
||||
func testSearchTargetFilteredOutWithOthersMatching() {
|
||||
let list = [w("a"), w("b", visible: false), w("c")]
|
||||
let i = inputs(list: list, selectedIndex: 1, selectedTarget: "b")
|
||||
// visibleIndexes = [0, 2]. selectedIndex=1 not in [0,2] → closest below is 0.
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(0))
|
||||
}
|
||||
|
||||
// MARK: - E. Edge cases
|
||||
|
||||
/// E1. Single window in the list that becomes invisible mid-session — clear selection.
|
||||
func testEdgeSingleWindowBecomesInvisible() {
|
||||
let list = [w("only", visible: false)]
|
||||
let i = inputs(list: list, selectedIndex: 0, selectedTarget: "only")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .clearTargetAndHover)
|
||||
}
|
||||
|
||||
/// E2. Target was at index 0; a new window inserts ahead, pushing it to 1. Highlight follows.
|
||||
func testEdgeNewWindowPushesTargetDown() {
|
||||
// Before: [target]. Now: [new, target].
|
||||
let list = [w("new", focusOrder: 0), w("target", focusOrder: 1)]
|
||||
let i = inputs(list: list, selectedIndex: 0, selectedTarget: "target")
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .selectAt(1))
|
||||
}
|
||||
|
||||
/// E3. `selectedTarget` points to an id that was never in the list (corrupt state from a
|
||||
/// stale session, or a window destroyed before refresh ran).
|
||||
func testEdgeStaleSelectedTarget() {
|
||||
let list = [w("a"), w("b"), w("c")]
|
||||
let i = inputs(list: list, selectedIndex: 1, selectedTarget: "ghost")
|
||||
// target lookup fails → adapt → selectedIndex=1 in [0,1,2], in range, target was non-nil
|
||||
// so the no-target-set branch doesn't trigger; tail returns ensureTargetSet(1).
|
||||
XCTAssertEqual(SelectionResolver.decide(i), .ensureTargetSet(1))
|
||||
}
|
||||
|
||||
// MARK: - Helper kernel functions
|
||||
|
||||
/// `getLastFocusedOrderWindowIndex` ignores windowless apps and invisible windows.
|
||||
func testGetLastFocusedOrderWindowIndexIgnoresWindowlessAndInvisible() {
|
||||
let list = [
|
||||
w("dock", focusOrder: 0, windowless: true), // windowless: skip
|
||||
w("a", focusOrder: 1, visible: false), // invisible: skip
|
||||
w("b", focusOrder: 2), // candidate
|
||||
w("c", focusOrder: 3), // candidate (loses on order)
|
||||
]
|
||||
XCTAssertEqual(SelectionResolver.getLastFocusedOrderWindowIndex(list), 2)
|
||||
}
|
||||
|
||||
/// `cycleFromZero` empty / single / multi-visible behavior.
|
||||
func testCycleFromZeroBehavior() {
|
||||
XCTAssertNil(SelectionResolver.cycleFromZero([]))
|
||||
XCTAssertNil(SelectionResolver.cycleFromZero([w("a", visible: false)]))
|
||||
// Single visible — wraps back to 0.
|
||||
XCTAssertEqual(SelectionResolver.cycleFromZero([w("a")]), 0)
|
||||
// Two visible — advances to 1.
|
||||
XCTAssertEqual(SelectionResolver.cycleFromZero([w("a"), w("b")]), 1)
|
||||
// First invisible — skips to next visible.
|
||||
XCTAssertEqual(SelectionResolver.cycleFromZero([w("a", visible: false), w("b")]), 1)
|
||||
}
|
||||
|
||||
/// `findTarget` excludes invisible matches even when the id is present.
|
||||
func testFindTargetSkipsInvisibleMatches() {
|
||||
let list = [w("a"), w("b", visible: false), w("c")]
|
||||
XCTAssertEqual(SelectionResolver.findTarget(list, "a"), 0)
|
||||
XCTAssertNil(SelectionResolver.findTarget(list, "b"))
|
||||
XCTAssertEqual(SelectionResolver.findTarget(list, "c"), 2)
|
||||
XCTAssertNil(SelectionResolver.findTarget(list, nil))
|
||||
XCTAssertNil(SelectionResolver.findTarget(list, "missing"))
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user