Skip to content

Commit eb27b2b

Browse files
authored
refactor(datagrid): native focus ring, single-click edit, a11y fixes (#921)
* refactor(datagrid): use native focus ring instead of CALayer border * fix(a11y): always set cell labels and row/column index ranges * refactor(datagrid): use themed font size for inline date picker * feat(datagrid): render boolean columns as native NSButton checkboxes * feat(datagrid): single-click on focused cell starts inline edit * refactor(datagrid): revert boolean checkbox to text+dropdown, deduplicate edit eligibility * refactor(datagrid): restore final on DataGridCellView, guard drag-from-focused-cell * fix(datagrid): use adaptive border color for focused cell instead of system focus ring
1 parent 7df36b6 commit eb27b2b

8 files changed

Lines changed: 119 additions & 105 deletions

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Connection URL parsing: SSH `user:password@host` split, `safeModeLevel` from TablePlus URLs, case-insensitive query params
1313
- Connection URL export: SSH password, Redis database index, MongoDB auth params (`authSource`, `authMechanism`, `replicaSet`), and multi-host
1414
- SSH Private Key auth resolves keys from `~/.ssh/config` and default locations (`id_ed25519`, `id_rsa`, `id_ecdsa`) when no explicit key path is set
15+
- Click a focused cell to start editing without a second click
16+
- Data grid focus ring follows the system accent color and contrast settings
17+
- Data grid cells expose accessibility row and column index ranges to VoiceOver on all dataset sizes
1518

1619
### Changed
1720

@@ -30,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3033
- QueryTab.resultVersion split: schemaVersion (column shape) on QueryTab, row mutations through delegate deltas, sort completion through a single delegate replace call. Pin toggle, sort completion, and applyMultiStatementResults no longer fan out a redundant reload signal.
3134
- 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.
3235
- 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.
36+
- Date picker popover font follows the data grid font setting
3337

3438
### Fixed
3539

TablePro/Views/Results/CellOverlayEditor.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ final class CellOverlayEditor: NSObject, NSTextViewDelegate {
109109
newPanel.contentView = scrollView
110110
newPanel.contentView?.wantsLayer = true
111111
newPanel.contentView?.layer?.borderWidth = 2
112-
newPanel.contentView?.layer?.borderColor = NSColor.selectedControlColor.safeCGColor
112+
newPanel.contentView?.layer?.borderColor = NSColor.keyboardFocusIndicatorColor.safeCGColor
113113
newPanel.contentView?.layer?.cornerRadius = 2
114114
newPanel.contentView?.layer?.masksToBounds = true
115115

TablePro/Views/Results/DataGridCellFactory.swift

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -23,40 +23,16 @@ final class CellChevronButton: NSButton {
2323
var cellColumnIndex: Int = -1
2424
}
2525

26-
/// Factory for creating data grid cell views
2726
@MainActor
2827
final class DataGridCellFactory {
2928
private let cellIdentifier = NSUserInterfaceItemIdentifier("DataCell")
3029
private let rowNumberCellIdentifier = NSUserInterfaceItemIdentifier("RowNumberCell")
31-
32-
/// Large dataset threshold - above this, disable expensive visual features
3330
private let largeDatasetThreshold = 5_000
3431

35-
// MARK: - Cached Settings
36-
37-
/// Cached NULL display string (updated via settings notification)
3832
private var nullDisplayString: String = AppSettingsManager.shared.dataGrid.nullDisplay
3933
private var settingsObserver: NSObjectProtocol?
4034

41-
// MARK: - Cached VoiceOver State
42-
43-
private static var cachedVoiceOverEnabled: Bool = NSWorkspace.shared.isVoiceOverEnabled
44-
// Observer lives for app lifetime — no removal needed since DataGridCellFactory is a static singleton cache
45-
private static let voiceOverObserver: NSObjectProtocol? = {
46-
NotificationCenter.default.addObserver(
47-
forName: NSWorkspace.accessibilityDisplayOptionsDidChangeNotification,
48-
object: nil,
49-
queue: .main
50-
) { _ in
51-
Task { @MainActor in
52-
DataGridCellFactory.cachedVoiceOverEnabled = NSWorkspace.shared.isVoiceOverEnabled
53-
}
54-
}
55-
}()
56-
5735
init() {
58-
_ = Self.voiceOverObserver
59-
6036
settingsObserver = NotificationCenter.default.addObserver(
6137
forName: .dataGridSettingsDidChange,
6238
object: nil,
@@ -119,9 +95,8 @@ final class DataGridCellFactory {
11995

12096
cell.stringValue = "\(row + 1)"
12197
cell.textColor = visualState.isDeleted ? ThemeEngine.shared.colors.dataGrid.deletedText : .secondaryLabelColor
122-
if Self.cachedVoiceOverEnabled {
123-
cellView.setAccessibilityLabel(String(format: String(localized: "Row %d"), row + 1))
124-
}
98+
cellView.setAccessibilityLabel(String(format: String(localized: "Row %d"), row + 1))
99+
cellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1))
125100

126101
return cellView
127102
}
@@ -259,23 +234,16 @@ final class DataGridCellFactory {
259234
gridCellView.changeBackgroundColor = nil
260235
}
261236

262-
if isLargeDataset {
263-
gridCellView.layer?.borderWidth = 0
264-
} else if isFocused {
265-
gridCellView.layer?.borderWidth = 2
266-
gridCellView.layer?.borderColor = ThemeEngine.shared.colors.dataGrid.focusBorderCG
267-
} else {
268-
gridCellView.layer?.borderWidth = 0
269-
}
237+
gridCellView.isFocusedCell = isFocused
270238

271239
CATransaction.commit()
272240

273-
if Self.cachedVoiceOverEnabled {
274-
let accessibilityValue = rawValue ?? String(localized: "NULL")
275-
cell.setAccessibilityLabel(
276-
String(format: String(localized: "Row %d, column %d: %@"), row + 1, columnIndex + 1, accessibilityValue)
277-
)
278-
}
241+
let accessibilityValue = rawValue ?? String(localized: "NULL")
242+
cell.setAccessibilityLabel(
243+
String(format: String(localized: "Row %d, column %d: %@"), row + 1, columnIndex + 1, accessibilityValue)
244+
)
245+
gridCellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1))
246+
gridCellView.setAccessibilityColumnIndexRange(NSRange(location: columnIndex, length: 1))
279247

280248
return gridCellView
281249
}

TablePro/Views/Results/DataGridCellView.swift

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@
55

66
import AppKit
77

8-
/// Custom cell view that uses a background subview for change-state coloring.
9-
/// AppKit's `NSTableRowView` sets `backgroundStyle` to `.emphasized` when the
10-
/// row is selected — we hide the background view so the native selection highlight
11-
/// shows through.
128
final class DataGridCellView: NSTableCellView {
139
var fkArrowButton: FKArrowButton?
1410
var chevronButton: CellChevronButton?
1511
var textFieldTrailing: NSLayoutConstraint?
1612

13+
var isFocusedCell: Bool = false {
14+
didSet {
15+
guard oldValue != isFocusedCell else { return }
16+
updateFocusBorder()
17+
}
18+
}
19+
1720
private lazy var backgroundView: NSView = {
1821
let view = NSView()
1922
view.wantsLayer = true
@@ -43,6 +46,18 @@ final class DataGridCellView: NSTableCellView {
4346
override var backgroundStyle: NSView.BackgroundStyle {
4447
didSet {
4548
backgroundView.isHidden = (backgroundStyle == .emphasized) || (changeBackgroundColor == nil)
49+
if isFocusedCell { updateFocusBorder() }
50+
}
51+
}
52+
53+
private func updateFocusBorder() {
54+
if isFocusedCell {
55+
layer?.borderWidth = 2
56+
layer?.borderColor = backgroundStyle == .emphasized
57+
? NSColor.white.withAlphaComponent(0.8).cgColor
58+
: NSColor.keyboardFocusIndicatorColor.cgColor
59+
} else {
60+
layer?.borderWidth = 0
4661
}
4762
}
4863
}

TablePro/Views/Results/DatePickerCellEditor.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ final class DatePickerCellEditor: NSDatePicker {
7171
private func setupUI() {
7272
datePickerStyle = .textFieldAndStepper
7373
datePickerElements = [.yearMonthDay, .hourMinuteSecond]
74-
font = .monospacedSystemFont(ofSize: 13, weight: .regular)
74+
let pointSize = ThemeEngine.shared.dataGridFonts.regular.pointSize
75+
font = .monospacedSystemFont(ofSize: pointSize, weight: .regular)
7576
isBezeled = false
7677
isBordered = false
7778
drawsBackground = false

TablePro/Views/Results/Extensions/DataGridView+Editing.swift

Lines changed: 51 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -7,57 +7,67 @@ import AppKit
77
import SwiftUI
88

99
extension TableViewCoordinator {
10-
func tableView(_ tableView: NSTableView, shouldEdit tableColumn: NSTableColumn?, row: Int) -> Bool {
11-
guard isEditable,
12-
let tableColumn = tableColumn else { return false }
10+
enum InlineEditEligibility {
11+
case eligible
12+
case needsOverlayEditor(value: String)
13+
case blocked
14+
}
1315

14-
let columnId = tableColumn.identifier.rawValue
15-
guard columnId != "__rowNumber__",
16-
!changeManager.isRowDeleted(row) else { return false }
16+
func inlineEditEligibility(row: Int, columnIndex: Int) -> InlineEditEligibility {
17+
guard isEditable else { return .blocked }
18+
guard row >= 0, columnIndex >= 0, columnIndex < rowProvider.columns.count else { return .blocked }
19+
guard !changeManager.isRowDeleted(row) else { return .blocked }
1720

1821
let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? []
19-
if !immutable.isEmpty,
20-
let columnIndex = DataGridView.dataColumnIndex(from: tableColumn.identifier),
21-
columnIndex < rowProvider.columns.count,
22-
immutable.contains(rowProvider.columns[columnIndex]) {
23-
return false
22+
if immutable.contains(rowProvider.columns[columnIndex]) {
23+
return .blocked
2424
}
2525

26-
// Popover-editor columns (date/FK/JSON) are only editable via
27-
// double-click (handleDoubleClick). Block inline editing for them.
28-
if let columnIndex = DataGridView.dataColumnIndex(from: tableColumn.identifier) {
29-
if columnIndex < rowProvider.columns.count {
30-
let columnName = rowProvider.columns[columnIndex]
31-
if rowProvider.columnForeignKeys[columnName] != nil { return false }
32-
}
33-
if columnIndex < rowProvider.columnTypes.count {
34-
let ct = rowProvider.columnTypes[columnIndex]
35-
if ct.isDateType || ct.isJsonType || ct.isEnumType || ct.isSetType || ct.isBlobType || ct.isBooleanType { return false }
36-
}
37-
if let dropdownCols = dropdownColumns, dropdownCols.contains(columnIndex) {
38-
return false
39-
}
40-
if let typePickerCols = typePickerColumns, typePickerCols.contains(columnIndex) {
41-
return false
42-
}
26+
let columnName = rowProvider.columns[columnIndex]
27+
if rowProvider.columnForeignKeys[columnName] != nil { return .blocked }
4328

44-
// Text columns containing JSON use JSON editor popover
45-
if let value = rowProvider.value(atRow: row, column: columnIndex),
46-
value.looksLikeJson {
47-
return false
29+
if columnIndex < rowProvider.columnTypes.count {
30+
let ct = rowProvider.columnTypes[columnIndex]
31+
if ct.isBooleanType || ct.isDateType || ct.isJsonType
32+
|| ct.isBlobType || ct.isEnumType || ct.isSetType {
33+
return .blocked
4834
}
35+
}
4936

50-
// Multiline values use overlay editor — block inline field editor
51-
if let value = rowProvider.value(atRow: row, column: columnIndex),
52-
value.containsLineBreak {
53-
let tableColumnIdx = tableView.column(withIdentifier: tableColumn.identifier)
54-
guard tableColumnIdx >= 0 else { return false }
55-
showOverlayEditor(tableView: tableView, row: row, column: tableColumnIdx, columnIndex: columnIndex, value: value)
56-
return false
57-
}
37+
if dropdownColumns?.contains(columnIndex) == true { return .blocked }
38+
if typePickerColumns?.contains(columnIndex) == true { return .blocked }
39+
40+
if let value = rowProvider.value(atRow: row, column: columnIndex) {
41+
if value.containsLineBreak { return .needsOverlayEditor(value: value) }
42+
if value.looksLikeJson { return .blocked }
5843
}
5944

60-
return true
45+
return .eligible
46+
}
47+
48+
func canStartInlineEdit(row: Int, columnIndex: Int) -> Bool {
49+
if case .eligible = inlineEditEligibility(row: row, columnIndex: columnIndex) {
50+
return true
51+
}
52+
return false
53+
}
54+
55+
func tableView(_ tableView: NSTableView, shouldEdit tableColumn: NSTableColumn?, row: Int) -> Bool {
56+
guard let tableColumn else { return false }
57+
guard tableColumn.identifier.rawValue != "__rowNumber__" else { return false }
58+
guard let columnIndex = DataGridView.dataColumnIndex(from: tableColumn.identifier) else { return false }
59+
60+
switch inlineEditEligibility(row: row, columnIndex: columnIndex) {
61+
case .eligible:
62+
return true
63+
case .needsOverlayEditor(let value):
64+
let tableColumnIdx = tableView.column(withIdentifier: tableColumn.identifier)
65+
guard tableColumnIdx >= 0 else { return false }
66+
showOverlayEditor(tableView: tableView, row: row, column: tableColumnIdx, columnIndex: columnIndex, value: value)
67+
return false
68+
case .blocked:
69+
return false
70+
}
6171
}
6272

6373
// MARK: - Overlay Editor (Multiline)

TablePro/Views/Results/KeyHandlingTableView.swift

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ final class KeyHandlingTableView: NSTableView {
5959
// MARK: - TablePlus-Style Cell Focus
6060

6161
override func mouseDown(with event: NSEvent) {
62-
// Become first responder to capture keyboard events (especially Delete key)
6362
window?.makeFirstResponder(self)
6463

6564
let point = convert(event.locationInWindow, from: nil)
@@ -71,32 +70,40 @@ final class KeyHandlingTableView: NSTableView {
7170
return
7271
}
7372

74-
// Reset anchor/pivot when clicking without Shift
7573
if clickedRow >= 0 && !event.modifierFlags.contains(.shift) {
7674
selectionAnchor = clickedRow
7775
selectionPivot = clickedRow
7876
}
7977

78+
let alreadyFocusedHere = clickedRow >= 0
79+
&& clickedColumn >= 0
80+
&& clickedRow == focusedRow
81+
&& clickedColumn == focusedColumn
82+
8083
super.mouseDown(with: event)
8184

82-
// Only handle editing for valid clicks on data cells (not row number column)
8385
guard clickedRow >= 0,
8486
clickedColumn >= 0,
8587
clickedColumn < numberOfColumns else {
8688
return
8789
}
8890

89-
// Skip row number column
9091
let column = tableColumns[clickedColumn]
9192
if column.identifier.rawValue == "__rowNumber__" {
9293
focusedRow = -1
9394
focusedColumn = -1
9495
return
9596
}
9697

97-
// Update focus (edit mode is triggered by double-click, not single click)
9898
focusedRow = clickedRow
9999
focusedColumn = clickedColumn
100+
101+
if alreadyFocusedHere && event.clickCount == 1 && selectedRowIndexes.count == 1 {
102+
let dataColumnIndex = DataGridView.dataColumnIndex(for: clickedColumn)
103+
if coordinator?.canStartInlineEdit(row: clickedRow, columnIndex: dataColumnIndex) == true {
104+
editColumn(clickedColumn, row: clickedRow, with: nil, select: true)
105+
}
106+
}
100107
}
101108

102109
// MARK: - Standard Edit Menu Actions

TablePro/Views/Results/TableRowViewWithMenu.swift

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ final class TableRowViewWithMenu: NSTableRowView {
130130
menu.addItem(NSMenuItem.separator())
131131
}
132132

133-
// Set Value (editable + column clicked)
134133
if coordinator.isEditable && dataColumnIndex >= 0 {
135134
let setValueMenu = NSMenu()
136135

@@ -140,17 +139,27 @@ final class TableRowViewWithMenu: NSTableRowView {
140139
emptyItem.target = self
141140
setValueMenu.addItem(emptyItem)
142141

143-
let nullItem = NSMenuItem(
144-
title: String(localized: "NULL"), action: #selector(setNullValue(_:)), keyEquivalent: "")
145-
nullItem.representedObject = dataColumnIndex
146-
nullItem.target = self
147-
setValueMenu.addItem(nullItem)
148-
149-
let defaultItem = NSMenuItem(
150-
title: String(localized: "Default"), action: #selector(setDefaultValue(_:)), keyEquivalent: "")
151-
defaultItem.representedObject = dataColumnIndex
152-
defaultItem.target = self
153-
setValueMenu.addItem(defaultItem)
142+
let columnName = dataColumnIndex < coordinator.rowProvider.columns.count
143+
? coordinator.rowProvider.columns[dataColumnIndex]
144+
: nil
145+
146+
let isNullable = columnName.flatMap { coordinator.rowProvider.columnNullable[$0] } ?? true
147+
if isNullable {
148+
let nullItem = NSMenuItem(
149+
title: String(localized: "NULL"), action: #selector(setNullValue(_:)), keyEquivalent: "")
150+
nullItem.representedObject = dataColumnIndex
151+
nullItem.target = self
152+
setValueMenu.addItem(nullItem)
153+
}
154+
155+
let hasDefault = columnName.flatMap({ coordinator.rowProvider.columnDefaults[$0] ?? nil }) != nil
156+
if hasDefault {
157+
let defaultItem = NSMenuItem(
158+
title: String(localized: "Default"), action: #selector(setDefaultValue(_:)), keyEquivalent: "")
159+
defaultItem.representedObject = dataColumnIndex
160+
defaultItem.target = self
161+
setValueMenu.addItem(defaultItem)
162+
}
154163

155164
let setValueItem = NSMenuItem(title: String(localized: "Set Value"), action: nil, keyEquivalent: "")
156165
setValueItem.submenu = setValueMenu

0 commit comments

Comments
 (0)