Skip to content

Commit 5877d31

Browse files
datlechindatlechin
authored
refactor(datagrid): extract TableSelection value type from KeyHandlingTableView (#926)
Replace four stored properties (focusedRow, focusedColumn, selectionAnchor, selectionPivot) with a single TableSelection struct. Centralized didSet computes reload indices for transitioning focus, eliminating per-property reload logic. Backward-compat accessors keep external call sites unchanged. Phase A of the data grid clean-architecture refactor.
1 parent a8d2993 commit 5877d31

4 files changed

Lines changed: 242 additions & 25 deletions

File tree

‎TablePro/Views/Results/KeyHandlingTableView.swift‎

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,38 +23,35 @@ final class KeyHandlingTableView: NSTableView {
2323
true
2424
}
2525

26-
/// Currently focused row index (-1 = no focus)
27-
var focusedRow: Int = -1 {
26+
var selection = TableSelection() {
2827
didSet {
29-
if oldValue != focusedRow && oldValue >= 0 {
30-
if focusedColumn >= 0 && focusedColumn < numberOfColumns && oldValue < numberOfRows {
31-
reloadData(forRowIndexes: IndexSet(integer: oldValue),
32-
columnIndexes: IndexSet(integer: focusedColumn))
33-
}
34-
}
28+
guard let (rows, columns) = selection.reloadIndexes(from: oldValue) else { return }
29+
let validRows = rows.filteredIndexSet { $0 < numberOfRows }
30+
let validColumns = columns.filteredIndexSet { $0 < numberOfColumns }
31+
guard !validRows.isEmpty, !validColumns.isEmpty else { return }
32+
reloadData(forRowIndexes: validRows, columnIndexes: validColumns)
3533
}
3634
}
3735

38-
/// Currently focused column index (-1 = no focus, 0 = row number column)
39-
var focusedColumn: Int = -1 {
40-
didSet {
41-
guard oldValue != focusedColumn else { return }
42-
let row = focusedRow
43-
guard row >= 0 && row < numberOfRows else { return }
44-
var cols = IndexSet()
45-
if oldValue >= 0 && oldValue < numberOfColumns { cols.insert(oldValue) }
46-
if focusedColumn >= 0 && focusedColumn < numberOfColumns { cols.insert(focusedColumn) }
47-
if !cols.isEmpty {
48-
reloadData(forRowIndexes: IndexSet(integer: row), columnIndexes: cols)
49-
}
50-
}
36+
var focusedRow: Int {
37+
get { selection.focusedRow }
38+
set { selection.focusedRow = newValue }
5139
}
5240

53-
/// Anchor row for Shift+Arrow range selection (-1 = no anchor)
54-
var selectionAnchor: Int = -1
41+
var focusedColumn: Int {
42+
get { selection.focusedColumn }
43+
set { selection.focusedColumn = newValue }
44+
}
45+
46+
var selectionAnchor: Int {
47+
get { selection.anchor }
48+
set { selection.anchor = newValue }
49+
}
5550

56-
/// Current pivot row for Shift+Arrow navigation
57-
var selectionPivot: Int = -1
51+
var selectionPivot: Int {
52+
get { selection.pivot }
53+
set { selection.pivot = newValue }
54+
}
5855

5956
// MARK: - TablePlus-Style Cell Focus
6057

‎TablePro/Views/Results/TableSelection.swift‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
//
2+
// TableSelection.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
8+
struct TableSelection: Equatable {
9+
var focusedRow: Int = -1
10+
var focusedColumn: Int = -1
11+
var anchor: Int = -1
12+
var pivot: Int = -1
13+
14+
var hasFocus: Bool { focusedRow >= 0 && focusedColumn >= 0 }
15+
16+
static let empty = TableSelection()
17+
18+
mutating func clearFocus() {
19+
focusedRow = -1
20+
focusedColumn = -1
21+
}
22+
23+
mutating func setFocus(row: Int, column: Int) {
24+
focusedRow = row
25+
focusedColumn = column
26+
}
27+
28+
mutating func resetAnchor(at row: Int) {
29+
anchor = row
30+
pivot = row
31+
}
32+
33+
mutating func clearAnchor() {
34+
anchor = -1
35+
pivot = -1
36+
}
37+
38+
func reloadIndexes(from previous: TableSelection) -> (rows: IndexSet, columns: IndexSet)? {
39+
guard previous.focusedRow != focusedRow || previous.focusedColumn != focusedColumn else {
40+
return nil
41+
}
42+
43+
var rows = IndexSet()
44+
var columns = IndexSet()
45+
46+
if previous.focusedRow >= 0 { rows.insert(previous.focusedRow) }
47+
if previous.focusedColumn >= 0 { columns.insert(previous.focusedColumn) }
48+
if focusedRow >= 0 { rows.insert(focusedRow) }
49+
if focusedColumn >= 0 { columns.insert(focusedColumn) }
50+
51+
guard !rows.isEmpty, !columns.isEmpty else { return nil }
52+
return (rows, columns)
53+
}
54+
}

‎TableProTests/Views/Main/Child/DataTabGridDelegateTests.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ private final class FakeRowDeltaApplier: RowDeltaApplying {
1313
var insertedCalls: [IndexSet] = []
1414
var removedCalls: [IndexSet] = []
1515
var fullReplaceCount: Int = 0
16+
var invalidateCount: Int = 0
1617

1718
func applyInsertedRows(_ indices: IndexSet) {
1819
insertedCalls.append(indices)
@@ -25,6 +26,10 @@ private final class FakeRowDeltaApplier: RowDeltaApplying {
2526
func applyFullReplace() {
2627
fullReplaceCount += 1
2728
}
29+
30+
func invalidateCachesForUndoRedo() {
31+
invalidateCount += 1
32+
}
2833
}
2934

3035
@Suite("DataTabGridDelegate row-delta forwarding")

‎TableProTests/Views/Results/TableSelectionTests.swift‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
//
2+
// TableSelectionTests.swift
3+
// TableProTests
4+
//
5+
6+
import Foundation
7+
@testable import TablePro
8+
import Testing
9+
10+
@Suite("TableSelection")
11+
struct TableSelectionTests {
12+
@Test("Default selection is empty")
13+
func defaultIsEmpty() {
14+
let selection = TableSelection()
15+
#expect(selection.focusedRow == -1)
16+
#expect(selection.focusedColumn == -1)
17+
#expect(selection.anchor == -1)
18+
#expect(selection.pivot == -1)
19+
#expect(selection.hasFocus == false)
20+
}
21+
22+
@Test("hasFocus requires both row and column")
23+
func hasFocusRequiresBoth() {
24+
var selection = TableSelection()
25+
selection.focusedRow = 5
26+
#expect(selection.hasFocus == false)
27+
selection.focusedColumn = 2
28+
#expect(selection.hasFocus == true)
29+
selection.focusedRow = -1
30+
#expect(selection.hasFocus == false)
31+
}
32+
33+
@Test("clearFocus resets focus only")
34+
func clearFocusKeepsAnchor() {
35+
var selection = TableSelection()
36+
selection.setFocus(row: 5, column: 2)
37+
selection.resetAnchor(at: 5)
38+
selection.clearFocus()
39+
#expect(selection.focusedRow == -1)
40+
#expect(selection.focusedColumn == -1)
41+
#expect(selection.anchor == 5)
42+
#expect(selection.pivot == 5)
43+
}
44+
45+
@Test("setFocus assigns row and column")
46+
func setFocus() {
47+
var selection = TableSelection()
48+
selection.setFocus(row: 7, column: 3)
49+
#expect(selection.focusedRow == 7)
50+
#expect(selection.focusedColumn == 3)
51+
}
52+
53+
@Test("resetAnchor sets both anchor and pivot")
54+
func resetAnchor() {
55+
var selection = TableSelection()
56+
selection.resetAnchor(at: 4)
57+
#expect(selection.anchor == 4)
58+
#expect(selection.pivot == 4)
59+
}
60+
61+
@Test("clearAnchor resets anchor and pivot")
62+
func clearAnchor() {
63+
var selection = TableSelection()
64+
selection.resetAnchor(at: 4)
65+
selection.clearAnchor()
66+
#expect(selection.anchor == -1)
67+
#expect(selection.pivot == -1)
68+
}
69+
70+
@Test("Equatable compares all four fields")
71+
func equatable() {
72+
var a = TableSelection()
73+
a.setFocus(row: 1, column: 2)
74+
a.resetAnchor(at: 1)
75+
var b = a
76+
#expect(a == b)
77+
b.focusedRow = 2
78+
#expect(a != b)
79+
}
80+
}
81+
82+
@Suite("TableSelection.reloadIndexes")
83+
struct TableSelectionReloadIndexesTests {
84+
@Test("No change returns nil")
85+
func noChange() {
86+
var selection = TableSelection()
87+
selection.setFocus(row: 5, column: 2)
88+
let same = selection
89+
#expect(selection.reloadIndexes(from: same) == nil)
90+
}
91+
92+
@Test("Anchor change without focus change returns nil")
93+
func anchorOnlyChange() {
94+
var previous = TableSelection()
95+
previous.setFocus(row: 5, column: 2)
96+
var current = previous
97+
current.resetAnchor(at: 8)
98+
#expect(current.reloadIndexes(from: previous) == nil)
99+
}
100+
101+
@Test("Initial focus from empty includes new cell only")
102+
func initialFocus() {
103+
let previous = TableSelection()
104+
var current = previous
105+
current.setFocus(row: 3, column: 1)
106+
let result = current.reloadIndexes(from: previous)
107+
#expect(result?.rows == IndexSet([3]))
108+
#expect(result?.columns == IndexSet([1]))
109+
}
110+
111+
@Test("Clearing focus includes old cell only")
112+
func clearFocusFromActive() {
113+
var previous = TableSelection()
114+
previous.setFocus(row: 3, column: 1)
115+
var current = previous
116+
current.clearFocus()
117+
let result = current.reloadIndexes(from: previous)
118+
#expect(result?.rows == IndexSet([3]))
119+
#expect(result?.columns == IndexSet([1]))
120+
}
121+
122+
@Test("Row change at same column reloads both rows")
123+
func rowChange() {
124+
var previous = TableSelection()
125+
previous.setFocus(row: 3, column: 2)
126+
var current = previous
127+
current.focusedRow = 4
128+
let result = current.reloadIndexes(from: previous)
129+
#expect(result?.rows == IndexSet([3, 4]))
130+
#expect(result?.columns == IndexSet([2]))
131+
}
132+
133+
@Test("Column change at same row reloads both columns")
134+
func columnChange() {
135+
var previous = TableSelection()
136+
previous.setFocus(row: 3, column: 2)
137+
var current = previous
138+
current.focusedColumn = 5
139+
let result = current.reloadIndexes(from: previous)
140+
#expect(result?.rows == IndexSet([3]))
141+
#expect(result?.columns == IndexSet([2, 5]))
142+
}
143+
144+
@Test("Both change reloads both rows and both columns")
145+
func bothChange() {
146+
var previous = TableSelection()
147+
previous.setFocus(row: 3, column: 2)
148+
var current = previous
149+
current.setFocus(row: 7, column: 5)
150+
let result = current.reloadIndexes(from: previous)
151+
#expect(result?.rows == IndexSet([3, 7]))
152+
#expect(result?.columns == IndexSet([2, 5]))
153+
}
154+
155+
@Test("Clearing focus from no-focus state returns nil")
156+
func clearFromEmpty() {
157+
let previous = TableSelection()
158+
let current = previous
159+
#expect(current.reloadIndexes(from: previous) == nil)
160+
}
161+
}

0 commit comments

Comments
 (0)