Skip to content

Commit e5490e4

Browse files
shouwang0527claude
authored and
zhaojianyong
committed
fix(datagrid): address review feedback for cell-range selection
- Separate copyRowsAsTSV validation from copy (only rows, not cells) - Write TSV format to clipboard so paste into Excel/Numbers works - Restrict Cmd+click column header to exclude Shift (Cmd+Shift = multi-sort) - Set implicit anchor on column select so Shift+click can extend - Add unit tests for CellSelection and TableSelection - Update keyboard-shortcuts docs with new selection shortcuts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 871cbf8 commit e5490e4

5 files changed

Lines changed: 181 additions & 5 deletions

File tree

‎TablePro/Views/Results/DataGridView+RowActions.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ extension TableViewCoordinator {
312312
func selectColumn(_ dataColumnIndex: Int) {
313313
guard let keyTableView = tableView as? KeyHandlingTableView else { return }
314314
keyTableView.selection.cellSelection = .column(dataColumnIndex)
315-
keyTableView.cellSelectionAnchor = nil
315+
keyTableView.cellSelectionAnchor = CellPosition(row: 0, column: dataColumnIndex)
316316
keyTableView.deselectAll(nil)
317317
}
318318

@@ -346,7 +346,7 @@ extension TableViewCoordinator {
346346
lines.append(text)
347347
}
348348

349-
ClipboardService.shared.writeText(lines.joined(separator: "\n"))
349+
ClipboardService.shared.writeRows(tsv: lines.joined(separator: "\n"), html: nil)
350350
}
351351

352352
private func copyCellPositions(_ positions: Set<CellPosition>) {
@@ -366,6 +366,6 @@ extension TableViewCoordinator {
366366
lines.append(text)
367367
}
368368

369-
ClipboardService.shared.writeText(lines.joined(separator: "\n"))
369+
ClipboardService.shared.writeRows(tsv: lines.joined(separator: "\n"), html: nil)
370370
}
371371
}

‎TablePro/Views/Results/KeyHandlingTableView.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,10 @@ final class KeyHandlingTableView: NSTableView {
244244
switch item.action {
245245
case #selector(delete(_:)), #selector(deleteBackward(_:)):
246246
return coordinator?.isEditable == true && !selectedRowIndexes.isEmpty
247-
case #selector(copy(_:)), #selector(copyRowsAsTSV(_:)):
247+
case #selector(copy(_:)):
248248
return !selection.cellSelection.isEmpty || !selectedRowIndexes.isEmpty
249+
case #selector(copyRowsAsTSV(_:)):
250+
return !selectedRowIndexes.isEmpty
249251
case #selector(paste(_:)):
250252
return coordinator?.isEditable == true && coordinator?.delegate != nil
251253
case #selector(insertNewline(_:)):

‎TablePro/Views/Results/SortableHeaderView.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ final class SortableHeaderView: NSTableHeaderView {
185185

186186
let modifiers = event.modifierFlags.intersection(.deviceIndependentFlagsMask)
187187

188-
if modifiers.contains(.command) {
188+
if modifiers.contains(.command) && !modifiers.contains(.shift) {
189189
coordinator.selectColumn(dataIndex)
190190
return
191191
}

‎TableProTests/Views/Results/CellSelectionTests.swift‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
import Foundation
2+
@testable import TablePro
3+
import Testing
4+
5+
@Suite("CellSelection")
6+
struct CellSelectionTests {
7+
8+
// MARK: - contains
9+
10+
@Test("none contains nothing")
11+
func noneContainsNothing() {
12+
let sel = CellSelection.none
13+
#expect(!sel.contains(row: 0, column: 0))
14+
#expect(!sel.contains(row: 5, column: 3))
15+
}
16+
17+
@Test("column contains any row in that column")
18+
func columnContainsMatchingColumn() {
19+
let sel = CellSelection.column(2)
20+
#expect(sel.contains(row: 0, column: 2))
21+
#expect(sel.contains(row: 999, column: 2))
22+
#expect(!sel.contains(row: 0, column: 1))
23+
#expect(!sel.contains(row: 0, column: 3))
24+
}
25+
26+
@Test("range contains rows within bounds in correct column")
27+
func rangeContainsWithinBounds() {
28+
let sel = CellSelection.range(column: 1, rows: 3...7)
29+
#expect(sel.contains(row: 3, column: 1))
30+
#expect(sel.contains(row: 5, column: 1))
31+
#expect(sel.contains(row: 7, column: 1))
32+
#expect(!sel.contains(row: 2, column: 1))
33+
#expect(!sel.contains(row: 8, column: 1))
34+
#expect(!sel.contains(row: 5, column: 0))
35+
}
36+
37+
@Test("cells contains only listed positions")
38+
func cellsContainsExactPositions() {
39+
let sel = CellSelection.cells([
40+
CellPosition(row: 1, column: 2),
41+
CellPosition(row: 5, column: 3)
42+
])
43+
#expect(sel.contains(row: 1, column: 2))
44+
#expect(sel.contains(row: 5, column: 3))
45+
#expect(!sel.contains(row: 1, column: 3))
46+
#expect(!sel.contains(row: 2, column: 2))
47+
}
48+
49+
// MARK: - affectedColumns
50+
51+
@Test("none has no affected columns")
52+
func noneAffectedColumns() {
53+
#expect(CellSelection.none.affectedColumns.isEmpty)
54+
}
55+
56+
@Test("column reports single affected column")
57+
func columnAffectedColumns() {
58+
let sel = CellSelection.column(4)
59+
#expect(sel.affectedColumns == IndexSet(integer: 4))
60+
}
61+
62+
@Test("range reports single affected column")
63+
func rangeAffectedColumns() {
64+
let sel = CellSelection.range(column: 2, rows: 0...10)
65+
#expect(sel.affectedColumns == IndexSet(integer: 2))
66+
}
67+
68+
@Test("cells reports all unique columns")
69+
func cellsAffectedColumns() {
70+
let sel = CellSelection.cells([
71+
CellPosition(row: 0, column: 1),
72+
CellPosition(row: 2, column: 3),
73+
CellPosition(row: 4, column: 1)
74+
])
75+
#expect(sel.affectedColumns == IndexSet([1, 3]))
76+
}
77+
78+
// MARK: - affectedRows
79+
80+
@Test("column has no affected rows")
81+
func columnAffectedRows() {
82+
#expect(CellSelection.column(0).affectedRows.isEmpty)
83+
}
84+
85+
@Test("range reports all rows in bounds")
86+
func rangeAffectedRows() {
87+
let sel = CellSelection.range(column: 0, rows: 2...5)
88+
#expect(sel.affectedRows == IndexSet(integersIn: 2...5))
89+
}
90+
91+
@Test("cells reports all unique rows")
92+
func cellsAffectedRows() {
93+
let sel = CellSelection.cells([
94+
CellPosition(row: 1, column: 0),
95+
CellPosition(row: 3, column: 2),
96+
CellPosition(row: 1, column: 5)
97+
])
98+
#expect(sel.affectedRows == IndexSet([1, 3]))
99+
}
100+
101+
// MARK: - isEmpty
102+
103+
@Test("none is empty")
104+
func noneIsEmpty() {
105+
#expect(CellSelection.none.isEmpty)
106+
}
107+
108+
@Test("column is not empty")
109+
func columnIsNotEmpty() {
110+
#expect(!CellSelection.column(0).isEmpty)
111+
}
112+
113+
@Test("range is not empty")
114+
func rangeIsNotEmpty() {
115+
#expect(!CellSelection.range(column: 0, rows: 0...0).isEmpty)
116+
}
117+
118+
@Test("cells with elements is not empty")
119+
func cellsNotEmpty() {
120+
#expect(!CellSelection.cells([CellPosition(row: 0, column: 0)]).isEmpty)
121+
}
122+
123+
@Test("cells with empty set is empty")
124+
func cellsEmptySetIsEmpty() {
125+
#expect(CellSelection.cells(Set()).isEmpty)
126+
}
127+
}
128+
129+
@Suite("TableSelection.reloadIndexes with cellSelection")
130+
struct TableSelectionCellSelectionTests {
131+
132+
@Test("returns nil when nothing changed")
133+
func noChangeReturnsNil() {
134+
let sel = TableSelection(focusedRow: 1, focusedColumn: 2, cellSelection: .column(3))
135+
#expect(sel.reloadIndexes(from: sel) == nil)
136+
}
137+
138+
@Test("returns affected indexes when cellSelection changes from none to range")
139+
func noneToRangeReturnsIndexes() {
140+
let old = TableSelection(focusedRow: -1, focusedColumn: -1, cellSelection: .none)
141+
let new = TableSelection(focusedRow: -1, focusedColumn: -1, cellSelection: .range(column: 2, rows: 3...5))
142+
let result = new.reloadIndexes(from: old)
143+
#expect(result != nil)
144+
#expect(result?.rows == IndexSet(integersIn: 3...5))
145+
#expect(result?.columns == IndexSet(integer: 2))
146+
}
147+
148+
@Test("returns nil for column-only change because affectedRows is empty")
149+
func columnChangeReturnsNilDueToEmptyRows() {
150+
let old = TableSelection(focusedRow: -1, focusedColumn: -1, cellSelection: .none)
151+
let new = TableSelection(focusedRow: -1, focusedColumn: -1, cellSelection: .column(1))
152+
let result = new.reloadIndexes(from: old)
153+
#expect(result == nil)
154+
}
155+
156+
@Test("focus change combined with cell selection returns union of indexes")
157+
func focusAndCellSelectionChange() {
158+
let old = TableSelection(focusedRow: 0, focusedColumn: 0, cellSelection: .cells([CellPosition(row: 2, column: 1)]))
159+
let new = TableSelection(focusedRow: 3, focusedColumn: 1, cellSelection: .cells([CellPosition(row: 4, column: 2)]))
160+
let result = new.reloadIndexes(from: old)
161+
#expect(result != nil)
162+
#expect(result!.rows.contains(0))
163+
#expect(result!.rows.contains(2))
164+
#expect(result!.rows.contains(3))
165+
#expect(result!.rows.contains(4))
166+
#expect(result!.columns.contains(0))
167+
#expect(result!.columns.contains(1))
168+
#expect(result!.columns.contains(2))
169+
}
170+
}

‎docs/features/keyboard-shortcuts.mdx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ TablePro is keyboard-driven. Most actions have shortcuts, and most menu shortcut
125125
| Select cell | Click |
126126
| Select row | Click row number |
127127
| Select multiple cells | Click + drag |
128+
| Select entire column | `Cmd` + click column header |
129+
| Select cell range in column | Click cell, then `Shift` + click another cell in same column |
130+
| Toggle cell in selection | `Cmd` + click cell |
131+
| Clear cell selection | `Escape` |
128132
| Extend selection | Shift + click |
129133
| Add to selection | Cmd + click |
130134
| Extend selection by row | `Shift+Up` / `Shift+Down` |

0 commit comments

Comments
 (0)