Skip to content

Commit a8d2993

Browse files
datlechindatlechin
authored
refactor(datagrid): interaction fixes, window UndoManager, multi-cell paste (#924)
* refactor(datagrid): keyboard fixes, tabular clipboard, Shift+Tab navigation * refactor(datagrid): use window UndoManager instead of private instance * feat(datagrid): multi-cell paste from TSV clipboard with undo grouping * docs: update CHANGELOG for datagrid phase 3 * fix(datagrid): restore Ctrl+H/J/K/L navigation, invalidate display cache on undo * debug(datagrid): add OSLog tracing for undo CPU investigation * fix(datagrid): drop full reload on undo, rely on SwiftUI partial reload * chore(datagrid): remove undo tracing instrumentation
1 parent eb27b2b commit a8d2993

17 files changed

Lines changed: 219 additions & 145 deletions

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
- Click a focused cell to start editing without a second click
1616
- Data grid focus ring follows the system accent color and contrast settings
1717
- Data grid cells expose accessibility row and column index ranges to VoiceOver on all dataset sizes
18+
- Multi-cell paste: paste TSV data from the clipboard into the grid starting from the focused cell, grouped as a single undo action
19+
- Shift+Tab navigates to the previous cell in the data grid
20+
- Copy rows writes TSV, HTML table, and plain text to the clipboard for richer paste in spreadsheet apps
21+
- Row drag adds TSV and HTML representations alongside the internal drag type
1822

1923
### Changed
2024

@@ -34,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3438
- Row data lives in a per-coordinator RowDataStore keyed by tab.id rather than on QueryTab itself, so SwiftUI's @Observable tracking on tabManager.tabs no longer fires for row writes.
3539
- DataGridConfiguration is Equatable; DataGridIdentity covers tabType, tableName, and primaryKeyColumns so updateNSView short-circuits when nothing structural changed. DataTabGridDelegate properties are wired in onAppear / onChange instead of in the body.
3640
- Date picker popover font follows the data grid font setting
41+
- Data grid undo/redo uses the window's UndoManager instead of a private instance, unifying Cmd+Z across editor and grid
42+
- Right-click during cell editing shows the native text context menu instead of the row menu
3743

3844
### Fixed
3945

‎TablePro/Core/ChangeTracking/DataChangeManager.swift‎

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -101,18 +101,21 @@ final class DataChangeManager: ChangeManaging {
101101
return lo
102102
}
103103

104-
private let undoManager: UndoManager = {
105-
let manager = UndoManager()
106-
manager.levelsOfUndo = 100
107-
return manager
108-
}()
104+
var undoManagerProvider: (() -> UndoManager?)?
105+
var onUndoApplied: ((UndoResult) -> Void)?
109106

110107
private var lastUndoResult: UndoResult?
111108

112109
// MARK: - Undo/Redo Properties
113110

114-
var canUndo: Bool { undoManager.canUndo }
115-
var canRedo: Bool { undoManager.canRedo }
111+
var canUndo: Bool { undoManagerProvider?()?.canUndo ?? false }
112+
var canRedo: Bool { undoManagerProvider?()?.canRedo ?? false }
113+
114+
private func registerUndo(actionName: String, _ handler: @escaping (DataChangeManager) -> Void) {
115+
guard let undoManager = undoManagerProvider?() else { return }
116+
undoManager.registerUndo(withTarget: self, handler: handler)
117+
undoManager.setActionName(actionName)
118+
}
116119

117120
// MARK: - Helper Methods
118121

@@ -138,7 +141,7 @@ final class DataChangeManager: ChangeManaging {
138141

139142
func clearChangesAndUndoHistory() {
140143
clearChanges()
141-
undoManager.removeAllActions()
144+
undoManagerProvider?()?.removeAllActions(withTarget: self)
142145
}
143146

144147
func configureForTable(
@@ -159,7 +162,7 @@ final class DataChangeManager: ChangeManaging {
159162
modifiedCells.removeAll()
160163
insertedRowData.removeAll()
161164
changedRowIndices.removeAll()
162-
undoManager.removeAllActions()
165+
undoManagerProvider?()?.removeAllActions(withTarget: self)
163166

164167
changes.removeAll()
165168
hasChanges = false
@@ -232,13 +235,12 @@ final class DataChangeManager: ChangeManaging {
232235
newValue: newValue
233236
))
234237
}
235-
undoManager.registerUndo(withTarget: self) { target in
238+
registerUndo(actionName: String(localized: "Edit Cell")) { target in
236239
target.applyDataUndo(.cellEdit(
237240
rowIndex: rowIndex, columnIndex: columnIndex, columnName: columnName,
238241
previousValue: oldValue, newValue: newValue, originalRow: nil
239242
))
240243
}
241-
undoManager.setActionName(String(localized: "Edit Cell"))
242244
changedRowIndices.insert(rowIndex)
243245
hasChanges = !changes.isEmpty
244246
reloadVersion += 1
@@ -287,13 +289,12 @@ final class DataChangeManager: ChangeManaging {
287289
changedRowIndices.insert(rowIndex)
288290
}
289291

290-
undoManager.registerUndo(withTarget: self) { target in
292+
registerUndo(actionName: String(localized: "Edit Cell")) { target in
291293
target.applyDataUndo(.cellEdit(
292294
rowIndex: rowIndex, columnIndex: columnIndex, columnName: columnName,
293295
previousValue: oldValue, newValue: newValue, originalRow: originalRow
294296
))
295297
}
296-
undoManager.setActionName(String(localized: "Edit Cell"))
297298
hasChanges = !changes.isEmpty
298299
reloadVersion += 1
299300
}
@@ -307,10 +308,9 @@ final class DataChangeManager: ChangeManaging {
307308
changeIndex[RowChangeKey(rowIndex: rowIndex, type: .delete)] = changes.count - 1
308309
deletedRowIndices.insert(rowIndex)
309310
changedRowIndices.insert(rowIndex)
310-
undoManager.registerUndo(withTarget: self) { target in
311+
registerUndo(actionName: String(localized: "Delete Row")) { target in
311312
target.applyDataUndo(.rowDeletion(rowIndex: rowIndex, originalRow: originalRow))
312313
}
313-
undoManager.setActionName(String(localized: "Delete Row"))
314314
hasChanges = true
315315
reloadVersion += 1
316316
}
@@ -336,10 +336,9 @@ final class DataChangeManager: ChangeManaging {
336336
changedRowIndices.insert(rowIndex)
337337
batchData.append((rowIndex: rowIndex, originalRow: originalRow))
338338
}
339-
undoManager.registerUndo(withTarget: self) { target in
339+
registerUndo(actionName: String(localized: "Delete Rows")) { target in
340340
target.applyDataUndo(.batchRowDeletion(rows: batchData))
341341
}
342-
undoManager.setActionName(String(localized: "Delete Rows"))
343342
hasChanges = true
344343
reloadVersion += 1
345344
}
@@ -351,10 +350,9 @@ final class DataChangeManager: ChangeManaging {
351350
changeIndex[RowChangeKey(rowIndex: rowIndex, type: .insert)] = changes.count - 1
352351
insertedRowIndices.insert(rowIndex)
353352
changedRowIndices.insert(rowIndex)
354-
undoManager.registerUndo(withTarget: self) { target in
353+
registerUndo(actionName: String(localized: "Insert Row")) { target in
355354
target.applyDataUndo(.rowInsertion(rowIndex: rowIndex))
356355
}
357-
undoManager.setActionName(String(localized: "Insert Row"))
358356
hasChanges = true
359357
reloadVersion += 1
360358
}
@@ -476,10 +474,9 @@ final class DataChangeManager: ChangeManaging {
476474
insertedRowData.removeValue(forKey: rowIndex)
477475
}
478476

479-
undoManager.registerUndo(withTarget: self) { target in
477+
registerUndo(actionName: String(localized: "Insert Rows")) { target in
480478
target.applyDataUndo(.batchRowInsertion(rowIndices: validRows, rowValues: rowValues))
481479
}
482-
undoManager.setActionName(String(localized: "Insert Rows"))
483480

484481
let sortedDeleted = validRows.sorted()
485482

@@ -506,13 +503,12 @@ final class DataChangeManager: ChangeManaging {
506503
private func applyDataUndo(_ action: UndoAction) {
507504
switch action {
508505
case .cellEdit(let rowIndex, let columnIndex, let columnName, let previousValue, let newValue, let originalRow):
509-
undoManager.registerUndo(withTarget: self) { target in
506+
registerUndo(actionName: String(localized: "Edit Cell")) { target in
510507
target.applyDataUndo(.cellEdit(
511508
rowIndex: rowIndex, columnIndex: columnIndex, columnName: columnName,
512509
previousValue: newValue, newValue: previousValue, originalRow: originalRow
513510
))
514511
}
515-
undoManager.setActionName(String(localized: "Edit Cell"))
516512

517513
let matchedIndex = changeIndex[RowChangeKey(rowIndex: rowIndex, type: .update)]
518514
?? changeIndex[RowChangeKey(rowIndex: rowIndex, type: .insert)]
@@ -567,13 +563,12 @@ final class DataChangeManager: ChangeManaging {
567563

568564
case .rowInsertion(let rowIndex):
569565
let savedValues = insertedRowData[rowIndex]
570-
undoManager.registerUndo(withTarget: self) { [savedValues] target in
566+
registerUndo(actionName: String(localized: "Insert Row")) { [savedValues] target in
571567
if let savedValues {
572568
target.insertedRowData[rowIndex] = savedValues
573569
}
574570
target.applyDataUndo(.rowInsertion(rowIndex: rowIndex))
575571
}
576-
undoManager.setActionName(String(localized: "Insert Row"))
577572

578573
if insertedRowIndices.contains(rowIndex) {
579574
undoRowInsertion(rowIndex: rowIndex)
@@ -606,10 +601,9 @@ final class DataChangeManager: ChangeManaging {
606601
}
607602

608603
case .rowDeletion(let rowIndex, let originalRow):
609-
undoManager.registerUndo(withTarget: self) { target in
604+
registerUndo(actionName: String(localized: "Delete Row")) { target in
610605
target.applyDataUndo(.rowDeletion(rowIndex: rowIndex, originalRow: originalRow))
611606
}
612-
undoManager.setActionName(String(localized: "Delete Row"))
613607

614608
if deletedRowIndices.contains(rowIndex) {
615609
undoRowDeletion(rowIndex: rowIndex)
@@ -626,10 +620,9 @@ final class DataChangeManager: ChangeManaging {
626620
}
627621

628622
case .batchRowDeletion(let rows):
629-
undoManager.registerUndo(withTarget: self) { target in
623+
registerUndo(actionName: String(localized: "Delete Rows")) { target in
630624
target.applyDataUndo(.batchRowDeletion(rows: rows))
631625
}
632-
undoManager.setActionName(String(localized: "Delete Rows"))
633626

634627
let isUndo = rows.contains { deletedRowIndices.contains($0.rowIndex) }
635628
if isUndo {
@@ -651,10 +644,9 @@ final class DataChangeManager: ChangeManaging {
651644
}
652645

653646
case .batchRowInsertion(let rowIndices, let rowValues):
654-
undoManager.registerUndo(withTarget: self) { target in
647+
registerUndo(actionName: String(localized: "Insert Rows")) { target in
655648
target.applyDataUndo(.batchRowInsertion(rowIndices: rowIndices, rowValues: rowValues))
656649
}
657-
undoManager.setActionName(String(localized: "Insert Rows"))
658650

659651
let firstInserted = rowIndices.first.map { insertedRowIndices.contains($0) } ?? false
660652
if firstInserted {
@@ -701,9 +693,12 @@ final class DataChangeManager: ChangeManaging {
701693

702694
hasChanges = !changes.isEmpty
703695
reloadVersion += 1
696+
697+
if let result = lastUndoResult {
698+
onUndoApplied?(result)
699+
}
704700
}
705701

706-
/// Re-apply a cell edit during redo without registering a duplicate undo
707702
private func recordCellChangeForRedo(
708703
rowIndex: Int,
709704
columnIndex: Int,
@@ -784,16 +779,16 @@ final class DataChangeManager: ChangeManaging {
784779
// MARK: - Undo/Redo Public API
785780

786781
func undoLastChange() -> UndoResult? {
787-
guard undoManager.canUndo else { return nil }
782+
guard let um = undoManagerProvider?(), um.canUndo else { return nil }
788783
lastUndoResult = nil
789-
undoManager.undo()
784+
um.undo()
790785
return lastUndoResult
791786
}
792787

793788
func redoLastChange() -> UndoResult? {
794-
guard undoManager.canRedo else { return nil }
789+
guard let um = undoManagerProvider?(), um.canRedo else { return nil }
795790
lastUndoResult = nil
796-
undoManager.redo()
791+
um.redo()
797792
return lastUndoResult
798793
}
799794

‎TablePro/Core/Services/Infrastructure/ClipboardService.swift‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,16 @@
99
import AppKit
1010
import UniformTypeIdentifiers
1111

12-
/// Protocol for clipboard operations
13-
/// Abstraction allows for mocking in tests
1412
protocol ClipboardProvider {
15-
/// Read text content from clipboard
16-
/// - Returns: Text string if available, nil otherwise
1713
func readText() -> String?
18-
19-
/// Write text content to clipboard
20-
/// - Parameter text: Text to write
2114
func writeText(_ text: String)
22-
23-
/// Check if clipboard contains text data
15+
func writeTabular(tsv: String, html: String)
2416
var hasText: Bool { get }
2517
}
2618

27-
/// Concrete implementation using NSPasteboard
2819
struct NSPasteboardClipboardProvider: ClipboardProvider {
20+
private static let tsvType = NSPasteboard.PasteboardType("public.utf8-tab-separated-values-text")
21+
2922
func readText() -> String? {
3023
NSPasteboard.general.string(forType: .string)
3124
}
@@ -37,12 +30,19 @@ struct NSPasteboardClipboardProvider: ClipboardProvider {
3730
pb.setString(text, forType: NSPasteboard.PasteboardType(UTType.utf8PlainText.identifier))
3831
}
3932

33+
func writeTabular(tsv: String, html: String) {
34+
let pb = NSPasteboard.general
35+
pb.clearContents()
36+
pb.setString(tsv, forType: .string)
37+
pb.setString(tsv, forType: Self.tsvType)
38+
pb.setString(html, forType: .html)
39+
}
40+
4041
var hasText: Bool {
4142
NSPasteboard.general.string(forType: .string) != nil
4243
}
4344
}
4445

45-
/// Shared clipboard service instance
4646
@MainActor
4747
enum ClipboardService {
4848
static var shared: ClipboardProvider = NSPasteboardClipboardProvider()

‎TablePro/Core/Services/Infrastructure/HtmlTableEncoder.swift‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
//
2+
// HtmlTableEncoder.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
8+
enum HtmlTableEncoder {
9+
static func encode(rows: [[String]], headers: [String]? = nil) -> String {
10+
var html = "<table>"
11+
if let headers {
12+
html += "<tr>"
13+
for header in headers {
14+
html += "<th>\(escape(header))</th>"
15+
}
16+
html += "</tr>"
17+
}
18+
for row in rows {
19+
html += "<tr>"
20+
for cell in row {
21+
html += "<td>\(escape(cell))</td>"
22+
}
23+
html += "</tr>"
24+
}
25+
html += "</table>"
26+
return html
27+
}
28+
29+
static func escape(_ string: String) -> String {
30+
string
31+
.replacingOccurrences(of: "&", with: "&amp;")
32+
.replacingOccurrences(of: "<", with: "&lt;")
33+
.replacingOccurrences(of: ">", with: "&gt;")
34+
.replacingOccurrences(of: "\"", with: "&quot;")
35+
}
36+
}

‎TablePro/Core/Services/Query/RowOperationsManager.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ final class RowOperationsManager {
176176
return applyUndoResult(result, resultRows: &resultRows)
177177
}
178178

179-
private func applyUndoResult(_ result: UndoResult, resultRows: inout [[String?]]) -> Set<Int>? {
179+
func applyUndoResult(_ result: UndoResult, resultRows: inout [[String?]]) -> Set<Int>? {
180180
switch result.action {
181181
case .cellEdit(let rowIndex, let columnIndex, _, let previousValue, _, _):
182182
if rowIndex < resultRows.count {

‎TablePro/Resources/Localizable.xcstrings‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29536,6 +29536,9 @@
2953629536
}
2953729537
}
2953829538
}
29539+
},
29540+
"Paste Cells" : {
29541+
2953929542
},
2954029543
"Paste your CREATE TABLE statement below:" : {
2954129544
"extractionState" : "stale",

‎TablePro/Views/Main/Child/DataTabGridDelegate.swift‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,9 @@ final class DataTabGridDelegate: DataGridViewDelegate {
7575
NotificationCenter.default.post(name: .exportQueryResults, object: nil)
7676
}
7777

78-
func dataGridUndo() {
79-
coordinator?.undoLastChange()
80-
}
78+
func dataGridUndo() {}
8179

82-
func dataGridRedo() {
83-
coordinator?.redoLastChange()
84-
}
80+
func dataGridRedo() {}
8581

8682
func dataGridNavigateFK(value: String, fkInfo: ForeignKeyInfo) {
8783
coordinator?.navigateToFKReference(value: value, fkInfo: fkInfo)

0 commit comments

Comments
 (0)