From f75ec33723dbe8979e54bc0bb8cc8a5e37912806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Tue, 28 Apr 2026 15:01:12 +0700 Subject: [PATCH] refactor(datagrid): extract TableSelection value type from KeyHandlingTableView 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. --- .../Views/Results/KeyHandlingTableView.swift | 47 +++-- TablePro/Views/Results/TableSelection.swift | 54 ++++++ .../Main/Child/DataTabGridDelegateTests.swift | 5 + .../Views/Results/TableSelectionTests.swift | 161 ++++++++++++++++++ 4 files changed, 242 insertions(+), 25 deletions(-) create mode 100644 TablePro/Views/Results/TableSelection.swift create mode 100644 TableProTests/Views/Results/TableSelectionTests.swift diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index c1f62d60d..a62f4db54 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -23,38 +23,35 @@ final class KeyHandlingTableView: NSTableView { true } - /// Currently focused row index (-1 = no focus) - var focusedRow: Int = -1 { + var selection = TableSelection() { didSet { - if oldValue != focusedRow && oldValue >= 0 { - if focusedColumn >= 0 && focusedColumn < numberOfColumns && oldValue < numberOfRows { - reloadData(forRowIndexes: IndexSet(integer: oldValue), - columnIndexes: IndexSet(integer: focusedColumn)) - } - } + guard let (rows, columns) = selection.reloadIndexes(from: oldValue) else { return } + let validRows = rows.filteredIndexSet { $0 < numberOfRows } + let validColumns = columns.filteredIndexSet { $0 < numberOfColumns } + guard !validRows.isEmpty, !validColumns.isEmpty else { return } + reloadData(forRowIndexes: validRows, columnIndexes: validColumns) } } - /// Currently focused column index (-1 = no focus, 0 = row number column) - var focusedColumn: Int = -1 { - didSet { - guard oldValue != focusedColumn else { return } - let row = focusedRow - guard row >= 0 && row < numberOfRows else { return } - var cols = IndexSet() - if oldValue >= 0 && oldValue < numberOfColumns { cols.insert(oldValue) } - if focusedColumn >= 0 && focusedColumn < numberOfColumns { cols.insert(focusedColumn) } - if !cols.isEmpty { - reloadData(forRowIndexes: IndexSet(integer: row), columnIndexes: cols) - } - } + var focusedRow: Int { + get { selection.focusedRow } + set { selection.focusedRow = newValue } } - /// Anchor row for Shift+Arrow range selection (-1 = no anchor) - var selectionAnchor: Int = -1 + var focusedColumn: Int { + get { selection.focusedColumn } + set { selection.focusedColumn = newValue } + } + + var selectionAnchor: Int { + get { selection.anchor } + set { selection.anchor = newValue } + } - /// Current pivot row for Shift+Arrow navigation - var selectionPivot: Int = -1 + var selectionPivot: Int { + get { selection.pivot } + set { selection.pivot = newValue } + } // MARK: - TablePlus-Style Cell Focus diff --git a/TablePro/Views/Results/TableSelection.swift b/TablePro/Views/Results/TableSelection.swift new file mode 100644 index 000000000..cca56fdae --- /dev/null +++ b/TablePro/Views/Results/TableSelection.swift @@ -0,0 +1,54 @@ +// +// TableSelection.swift +// TablePro +// + +import Foundation + +struct TableSelection: Equatable { + var focusedRow: Int = -1 + var focusedColumn: Int = -1 + var anchor: Int = -1 + var pivot: Int = -1 + + var hasFocus: Bool { focusedRow >= 0 && focusedColumn >= 0 } + + static let empty = TableSelection() + + mutating func clearFocus() { + focusedRow = -1 + focusedColumn = -1 + } + + mutating func setFocus(row: Int, column: Int) { + focusedRow = row + focusedColumn = column + } + + mutating func resetAnchor(at row: Int) { + anchor = row + pivot = row + } + + mutating func clearAnchor() { + anchor = -1 + pivot = -1 + } + + func reloadIndexes(from previous: TableSelection) -> (rows: IndexSet, columns: IndexSet)? { + guard previous.focusedRow != focusedRow || previous.focusedColumn != focusedColumn else { + return nil + } + + var rows = IndexSet() + var columns = IndexSet() + + if previous.focusedRow >= 0 { rows.insert(previous.focusedRow) } + if previous.focusedColumn >= 0 { columns.insert(previous.focusedColumn) } + if focusedRow >= 0 { rows.insert(focusedRow) } + if focusedColumn >= 0 { columns.insert(focusedColumn) } + + guard !rows.isEmpty, !columns.isEmpty else { return nil } + return (rows, columns) + } +} diff --git a/TableProTests/Views/Main/Child/DataTabGridDelegateTests.swift b/TableProTests/Views/Main/Child/DataTabGridDelegateTests.swift index 3fd071b56..a23ed8a05 100644 --- a/TableProTests/Views/Main/Child/DataTabGridDelegateTests.swift +++ b/TableProTests/Views/Main/Child/DataTabGridDelegateTests.swift @@ -13,6 +13,7 @@ private final class FakeRowDeltaApplier: RowDeltaApplying { var insertedCalls: [IndexSet] = [] var removedCalls: [IndexSet] = [] var fullReplaceCount: Int = 0 + var invalidateCount: Int = 0 func applyInsertedRows(_ indices: IndexSet) { insertedCalls.append(indices) @@ -25,6 +26,10 @@ private final class FakeRowDeltaApplier: RowDeltaApplying { func applyFullReplace() { fullReplaceCount += 1 } + + func invalidateCachesForUndoRedo() { + invalidateCount += 1 + } } @Suite("DataTabGridDelegate row-delta forwarding") diff --git a/TableProTests/Views/Results/TableSelectionTests.swift b/TableProTests/Views/Results/TableSelectionTests.swift new file mode 100644 index 000000000..e5cc30363 --- /dev/null +++ b/TableProTests/Views/Results/TableSelectionTests.swift @@ -0,0 +1,161 @@ +// +// TableSelectionTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("TableSelection") +struct TableSelectionTests { + @Test("Default selection is empty") + func defaultIsEmpty() { + let selection = TableSelection() + #expect(selection.focusedRow == -1) + #expect(selection.focusedColumn == -1) + #expect(selection.anchor == -1) + #expect(selection.pivot == -1) + #expect(selection.hasFocus == false) + } + + @Test("hasFocus requires both row and column") + func hasFocusRequiresBoth() { + var selection = TableSelection() + selection.focusedRow = 5 + #expect(selection.hasFocus == false) + selection.focusedColumn = 2 + #expect(selection.hasFocus == true) + selection.focusedRow = -1 + #expect(selection.hasFocus == false) + } + + @Test("clearFocus resets focus only") + func clearFocusKeepsAnchor() { + var selection = TableSelection() + selection.setFocus(row: 5, column: 2) + selection.resetAnchor(at: 5) + selection.clearFocus() + #expect(selection.focusedRow == -1) + #expect(selection.focusedColumn == -1) + #expect(selection.anchor == 5) + #expect(selection.pivot == 5) + } + + @Test("setFocus assigns row and column") + func setFocus() { + var selection = TableSelection() + selection.setFocus(row: 7, column: 3) + #expect(selection.focusedRow == 7) + #expect(selection.focusedColumn == 3) + } + + @Test("resetAnchor sets both anchor and pivot") + func resetAnchor() { + var selection = TableSelection() + selection.resetAnchor(at: 4) + #expect(selection.anchor == 4) + #expect(selection.pivot == 4) + } + + @Test("clearAnchor resets anchor and pivot") + func clearAnchor() { + var selection = TableSelection() + selection.resetAnchor(at: 4) + selection.clearAnchor() + #expect(selection.anchor == -1) + #expect(selection.pivot == -1) + } + + @Test("Equatable compares all four fields") + func equatable() { + var a = TableSelection() + a.setFocus(row: 1, column: 2) + a.resetAnchor(at: 1) + var b = a + #expect(a == b) + b.focusedRow = 2 + #expect(a != b) + } +} + +@Suite("TableSelection.reloadIndexes") +struct TableSelectionReloadIndexesTests { + @Test("No change returns nil") + func noChange() { + var selection = TableSelection() + selection.setFocus(row: 5, column: 2) + let same = selection + #expect(selection.reloadIndexes(from: same) == nil) + } + + @Test("Anchor change without focus change returns nil") + func anchorOnlyChange() { + var previous = TableSelection() + previous.setFocus(row: 5, column: 2) + var current = previous + current.resetAnchor(at: 8) + #expect(current.reloadIndexes(from: previous) == nil) + } + + @Test("Initial focus from empty includes new cell only") + func initialFocus() { + let previous = TableSelection() + var current = previous + current.setFocus(row: 3, column: 1) + let result = current.reloadIndexes(from: previous) + #expect(result?.rows == IndexSet([3])) + #expect(result?.columns == IndexSet([1])) + } + + @Test("Clearing focus includes old cell only") + func clearFocusFromActive() { + var previous = TableSelection() + previous.setFocus(row: 3, column: 1) + var current = previous + current.clearFocus() + let result = current.reloadIndexes(from: previous) + #expect(result?.rows == IndexSet([3])) + #expect(result?.columns == IndexSet([1])) + } + + @Test("Row change at same column reloads both rows") + func rowChange() { + var previous = TableSelection() + previous.setFocus(row: 3, column: 2) + var current = previous + current.focusedRow = 4 + let result = current.reloadIndexes(from: previous) + #expect(result?.rows == IndexSet([3, 4])) + #expect(result?.columns == IndexSet([2])) + } + + @Test("Column change at same row reloads both columns") + func columnChange() { + var previous = TableSelection() + previous.setFocus(row: 3, column: 2) + var current = previous + current.focusedColumn = 5 + let result = current.reloadIndexes(from: previous) + #expect(result?.rows == IndexSet([3])) + #expect(result?.columns == IndexSet([2, 5])) + } + + @Test("Both change reloads both rows and both columns") + func bothChange() { + var previous = TableSelection() + previous.setFocus(row: 3, column: 2) + var current = previous + current.setFocus(row: 7, column: 5) + let result = current.reloadIndexes(from: previous) + #expect(result?.rows == IndexSet([3, 7])) + #expect(result?.columns == IndexSet([2, 5])) + } + + @Test("Clearing focus from no-focus state returns nil") + func clearFromEmpty() { + let previous = TableSelection() + let current = previous + #expect(current.reloadIndexes(from: previous) == nil) + } +}