Skip to content

Commit 1df6455

Browse files
authored
feat(datagrid): move the cell editor to the row above or below with the arrow keys (#2573)
1 parent f48d202 commit 1df6455

9 files changed

Lines changed: 617 additions & 34 deletions

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- Per-connection MongoDB shell state, so a variable or function survives from one statement to the next.
1414
- Cursor method autocomplete after `find()` and `aggregate()`.
1515
- Copy To and Duplicate Database in the sidebar and the Database menu, carrying structure, data or both to any connection. (#2487)
16+
- `Up` and `Down` while editing a cell, moving the editor to the same column of the row above or below. (#2569)
1617
- Tab rows in Settings > General > Tabs, wrapping the strip instead of scrolling it. (#2438)
1718
- Autoscrolling while dragging a tab, so a tab can be moved past the run currently on screen. (#2438)
1819

@@ -31,6 +32,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3132
- Tab drag released on a neighbour's exact centre leaving the order unchanged. (#2438)
3233
- Compare & Sync unable to drop an overloaded PostgreSQL routine, or any trigger.
3334
- PostgreSQL sequence DDL naming the schema it was read from, in SQL export and the structure editor.
35+
- Half-composed input method text saved and left behind when `Tab` moved the cell editor.
36+
- Cell editor opening off screen when `Tab` wrapped onto a row below the visible ones.
37+
- Cell cursor left on the old column after `Tab` carried the editor to the next one.
38+
- Every data grid switching to its accessibility layout after one `Tab` press, with no assistive app attached.
3439

3540
## [0.69.0] - 2026-08-27
3641

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//
2+
// CellEditorMovement.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
8+
/// Which way an inline cell editor was left.
9+
///
10+
/// The cases are AppKit's own vocabulary for leaving one field for the next: `NSTextMovement`
11+
/// declares `up` and `down` beside `tab` and `backtab` as "movement codes for movement between
12+
/// fields". The overlay editor is a standalone text view rather than a field editor, so it reads
13+
/// the four selectors itself and reports them here.
14+
enum CellEditorMovement {
15+
case tab
16+
case backtab
17+
case up
18+
case down
19+
}
20+
21+
/// Whether Up or Down in an inline cell editor moves the caret or leaves the cell.
22+
///
23+
/// A cell value can hold line breaks, so the arrow keys belong to the value's own lines first and
24+
/// the editor is left only from the line at that end. A value on one line has no line to move to
25+
/// in either direction, so both arrows leave it, including straight after the editor opens with
26+
/// the whole value selected.
27+
///
28+
/// A line break is the only thing that starts a line here, because the overlay never wraps text
29+
/// (`CellOverlayBase.applyCellTextLayout`, pinned by `CellOverlayTextLayoutTests`).
30+
struct CellEditorArrowExit {
31+
let canExitUp: Bool
32+
let canExitDown: Bool
33+
34+
init(text: NSString, selection: NSRange) {
35+
let length = text.length
36+
let start = min(max(selection.location, 0), length)
37+
let end = start + min(max(selection.length, 0), length - start)
38+
let breakAbove = Self.containsLineBreak(text, NSRange(location: 0, length: start))
39+
let breakBelow = Self.containsLineBreak(text, NSRange(location: end, length: length - end))
40+
41+
guard start != end else {
42+
canExitUp = !breakAbove
43+
canExitDown = !breakBelow
44+
return
45+
}
46+
47+
let isSingleLine = !breakAbove && !breakBelow
48+
&& !Self.containsLineBreak(text, NSRange(location: start, length: end - start))
49+
canExitUp = isSingleLine
50+
canExitDown = isSingleLine
51+
}
52+
53+
private static func containsLineBreak(_ text: NSString, _ range: NSRange) -> Bool {
54+
guard range.length > 0 else { return false }
55+
return text.rangeOfCharacter(from: .newlines, range: range).location != NSNotFound
56+
}
57+
}

TablePro/Views/Results/CellOverlayEditor.swift

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate {
1111
private var initialValue: String = ""
1212

1313
var onCommit: ((_ row: Int, _ columnIndex: Int, _ newValue: String) -> Void)?
14-
var onTabNavigation: ((_ row: Int, _ column: Int, _ forward: Bool) -> Void)?
14+
var onMovement: ((_ row: Int, _ column: Int, _ movement: CellEditorMovement) -> Void)?
1515

1616
func show(
1717
in tableView: NSTableView,
@@ -90,21 +90,51 @@ final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate {
9090
}
9191

9292
if commandSelector == #selector(NSResponder.insertTab(_:)) {
93-
let dismissRow = row, dismissColumn = column
94-
dismiss(commit: true)
95-
onTabNavigation?(dismissRow, dismissColumn, true)
96-
return true
93+
return leave(with: .tab, from: textView)
9794
}
9895

9996
if commandSelector == #selector(NSResponder.insertBacktab(_:)) {
100-
let dismissRow = row, dismissColumn = column
101-
dismiss(commit: true)
102-
onTabNavigation?(dismissRow, dismissColumn, false)
103-
return true
97+
return leave(with: .backtab, from: textView)
98+
}
99+
100+
if commandSelector == #selector(NSResponder.moveUp(_:)) {
101+
return leaveVertically(.up, from: textView)
102+
}
103+
104+
if commandSelector == #selector(NSResponder.moveDown(_:)) {
105+
return leaveVertically(.down, from: textView)
104106
}
105107

106108
return false
107109
}
110+
111+
/// Only the plain arrows are read. Shift, Option and Command each map to a selector of their
112+
/// own, so extending a selection or jumping to the end of the value keeps its native meaning.
113+
///
114+
/// An unhandled arrow moves the caret inside marked text, which is what it is for, so a
115+
/// composition takes it back rather than having it swallowed.
116+
private func leaveVertically(_ movement: CellEditorMovement, from textView: NSTextView) -> Bool {
117+
guard !textView.hasMarkedText() else { return false }
118+
let exit = CellEditorArrowExit(
119+
text: textView.string as NSString,
120+
selection: textView.selectedRange()
121+
)
122+
let leaves = movement == .up ? exit.canExitUp : exit.canExitDown
123+
guard leaves else { return false }
124+
return leave(with: movement, from: textView)
125+
}
126+
127+
/// A composition in progress owns the keystroke. Until the input method commits it the text
128+
/// view holds provisional text, and leaving the cell would save that half-composed value and
129+
/// carry the editor off it. The key is swallowed rather than passed back, because a literal
130+
/// tab in a cell is not what Tab was pressed for.
131+
private func leave(with movement: CellEditorMovement, from textView: NSTextView) -> Bool {
132+
guard !textView.hasMarkedText() else { return true }
133+
let dismissRow = row, dismissColumn = column
134+
dismiss(commit: true)
135+
onMovement?(dismissRow, dismissColumn, movement)
136+
return true
137+
}
108138
}
109139

110140
private final class OverlayTextView: NSTextView {

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

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ extension TableViewCoordinator {
9797
editor.onCommit = { [weak self] row, columnIndex, newValue in
9898
self?.commitCellEdit(row: row, columnIndex: columnIndex, newValue: newValue)
9999
}
100-
editor.onTabNavigation = { [weak self] row, column, forward in
101-
self?.handleOverlayTabNavigation(row: row, column: column, forward: forward)
100+
editor.onMovement = { [weak self] row, column, movement in
101+
self?.handleOverlayMovement(row: row, column: column, movement: movement)
102102
}
103103
overlayViewer?.dismiss()
104104
editor.show(in: tableView, row: row, column: column, columnIndex: columnIndex, value: value)
@@ -116,30 +116,31 @@ extension TableViewCoordinator {
116116
viewer.show(in: tableView, row: row, column: column, columnIndex: columnIndex, value: value)
117117
}
118118

119-
func handleOverlayTabNavigation(row: Int, column: Int, forward: Bool) {
120-
guard let tableView = tableView,
121-
let target = tabNavigationTarget(from: (row, column), forward: forward, in: tableView)
119+
/// The cell cursor moves with the editor, through the same `focusCell` the grid's own Tab uses.
120+
/// Selecting the row alone left the cursor on the column the editor came from, so closing the
121+
/// editor put it back where the editing was not, and it never scrolled the target row into
122+
/// view, so a wrap onto the row below the last visible one opened the editor off screen.
123+
func handleOverlayMovement(row: Int, column: Int, movement: CellEditorMovement) {
124+
guard let tableView = tableView as? KeyHandlingTableView,
125+
let target = movementTarget(from: (row, column), movement: movement, in: tableView)
122126
else { return }
123127

124-
let nextRow = target.row
125-
let nextColumn = target.column
126-
tableView.selectRowIndexes(IndexSet(integer: nextRow), byExtendingSelection: false)
127-
scrollColumnToVisible(tableColumnIndex: nextColumn)
128+
tableView.focusCell(row: target.row, column: target.column)
128129

129-
guard let nextColumnIndex = DataGridView.dataColumnIndex(
130-
for: nextColumn,
130+
guard let targetColumnIndex = DataGridView.dataColumnIndex(
131+
for: target.column,
131132
in: tableView,
132133
schema: identitySchema
133134
),
134-
nextColumnIndex >= 0,
135-
case .editable(let value) = editEligibility(row: nextRow, columnIndex: nextColumnIndex)
135+
targetColumnIndex >= 0,
136+
case .editable(let value) = editEligibility(row: target.row, columnIndex: targetColumnIndex)
136137
else { return }
137138

138139
showOverlayEditor(
139140
tableView: tableView,
140-
row: nextRow,
141-
column: nextColumn,
142-
columnIndex: nextColumnIndex,
141+
row: target.row,
142+
column: target.column,
143+
columnIndex: targetColumnIndex,
143144
value: value
144145
)
145146
}
@@ -148,22 +149,34 @@ extension TableViewCoordinator {
148149
/// previous row's last. Both ends are resolved rather than assumed: the window's spacers and
149150
/// the pool's surplus slots are attached columns too, so neither end of `tableColumns` holds a
150151
/// data column and a fixed position lands on a spacer that swallows the keystroke.
151-
private func tabNavigationTarget(
152+
///
153+
/// Up and Down hold the column and step one row, and neither wraps: a column is a column of one
154+
/// kind of value, so carrying the editor from the last row round to the first is a jump the
155+
/// user did not ask for.
156+
func movementTarget(
152157
from cell: (row: Int, column: Int),
153-
forward: Bool,
158+
movement: CellEditorMovement,
154159
in tableView: NSTableView
155160
) -> (row: Int, column: Int)? {
156-
if forward {
161+
switch movement {
162+
case .tab:
157163
if let next = nextPresentedColumnIndex(after: cell.column) {
158164
return (cell.row, next)
159165
}
160166
guard cell.row + 1 < tableView.numberOfRows, let first = firstPresentedColumnIndex() else { return nil }
161167
return (cell.row + 1, first)
168+
case .backtab:
169+
if let previous = previousPresentedColumnIndex(before: cell.column) {
170+
return (cell.row, previous)
171+
}
172+
guard cell.row > 0, let last = lastPresentedColumnIndex() else { return nil }
173+
return (cell.row - 1, last)
174+
case .up:
175+
guard cell.row > 0 else { return nil }
176+
return (cell.row - 1, cell.column)
177+
case .down:
178+
guard cell.row + 1 < tableView.numberOfRows else { return nil }
179+
return (cell.row + 1, cell.column)
162180
}
163-
if let previous = previousPresentedColumnIndex(before: cell.column) {
164-
return (cell.row, previous)
165-
}
166-
guard cell.row > 0, let last = lastPresentedColumnIndex() else { return nil }
167-
return (cell.row - 1, last)
168181
}
169182
}

TablePro/Views/Results/KeyHandlingTableView.swift

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,13 @@ final class KeyHandlingTableView: NSTableView {
521521
///
522522
/// A cell is drawn rather than mounted, so the element comes from the row's own accessibility
523523
/// children rather than from a cell view.
524+
///
525+
/// Nothing is posted until a client has asked the grid something. The element does not exist
526+
/// before that, so the notification had nowhere to land, and asking for it was itself enough to
527+
/// mount a view per visible cell in every grid: the cost `#2381` removed, charged to a session
528+
/// that pressed Tab once.
524529
internal func postCellCursorMoved() {
530+
guard DataGridAccessibility.isActive else { return }
525531
guard selectedRow >= 0, presentsDataColumn(at: focusedColumn) else { return }
526532
guard let element = accessibilityCellElement(row: selectedRow, tableColumnIndex: focusedColumn) else { return }
527533
NSAccessibility.post(element: element, notification: .focusedUIElementChanged)
@@ -621,7 +627,9 @@ final class KeyHandlingTableView: NSTableView {
621627
return true
622628
}
623629

624-
private func focusCell(row: Int, column: Int) {
630+
/// The one way the cell cursor is moved by a keystroke, used by Tab inside the grid and by the
631+
/// inline editor's own Tab and arrow navigation.
632+
internal func focusCell(row: Int, column: Int) {
625633
selectRowIndexes(IndexSet(integer: row), byExtendingSelection: false)
626634
focusedRow = row
627635
focusedColumn = column
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
//
2+
// CellEditorArrowExitTests.swift
3+
// TableProTests
4+
//
5+
6+
import Foundation
7+
import Testing
8+
9+
@testable import TablePro
10+
11+
/// Up and Down carry the inline editor to the adjacent row, so a value that holds line breaks has
12+
/// to keep them for its own lines and give them up only at the line at that end (#2569).
13+
@Suite("Cell editor arrow exit")
14+
struct CellEditorArrowExitTests {
15+
private func exit(_ value: String, selection: NSRange) -> CellEditorArrowExit {
16+
CellEditorArrowExit(text: value as NSString, selection: selection)
17+
}
18+
19+
@Test("A single line leaves the cell in both directions")
20+
func singleLineLeavesInBothDirections() {
21+
let placement = exit("alpha", selection: NSRange(location: 2, length: 0))
22+
23+
#expect(placement.canExitUp)
24+
#expect(placement.canExitDown)
25+
}
26+
27+
/// The editor opens with the whole value selected, which is where the first arrow press lands.
28+
@Test("A fully selected single line still leaves the cell")
29+
func fullySelectedSingleLineStillLeaves() {
30+
let placement = exit("alpha", selection: NSRange(location: 0, length: 5))
31+
32+
#expect(placement.canExitUp)
33+
#expect(placement.canExitDown)
34+
}
35+
36+
@Test("An empty value leaves the cell in both directions")
37+
func emptyValueLeaves() {
38+
let placement = exit("", selection: NSRange(location: 0, length: 0))
39+
40+
#expect(placement.canExitUp)
41+
#expect(placement.canExitDown)
42+
}
43+
44+
@Test("The middle line of a multi-line value keeps both arrows")
45+
func middleLineKeepsBothArrows() {
46+
let placement = exit("a\nb\nc", selection: NSRange(location: 2, length: 0))
47+
48+
#expect(!placement.canExitUp)
49+
#expect(!placement.canExitDown)
50+
}
51+
52+
@Test("The first line of a multi-line value leaves upwards only")
53+
func firstLineLeavesUpwardsOnly() {
54+
let placement = exit("a\nb\nc", selection: NSRange(location: 0, length: 0))
55+
56+
#expect(placement.canExitUp)
57+
#expect(!placement.canExitDown)
58+
}
59+
60+
@Test("The last line of a multi-line value leaves downwards only")
61+
func lastLineLeavesDownwardsOnly() {
62+
let placement = exit("a\nb\nc", selection: NSRange(location: 5, length: 0))
63+
64+
#expect(!placement.canExitUp)
65+
#expect(placement.canExitDown)
66+
}
67+
68+
/// A caret sitting just before the closing break is still on the line above the empty one.
69+
@Test("A trailing break still counts as a line below")
70+
func trailingBreakCountsAsLineBelow() {
71+
let placement = exit("a\n", selection: NSRange(location: 1, length: 0))
72+
73+
#expect(placement.canExitUp)
74+
#expect(!placement.canExitDown)
75+
}
76+
77+
/// A selection is a text-editing gesture in its own right, so the arrow collapses it first and
78+
/// the press after that is the one that leaves.
79+
@Test("A selection inside a multi-line value keeps both arrows")
80+
func selectionInsideMultiLineKeepsBothArrows() {
81+
let placement = exit("a\nb\nc", selection: NSRange(location: 0, length: 5))
82+
83+
#expect(!placement.canExitUp)
84+
#expect(!placement.canExitDown)
85+
}
86+
87+
@Test("A carriage return starts a line the same way a newline does")
88+
func carriageReturnStartsALine() {
89+
let placement = exit("a\r\nb", selection: NSRange(location: 4, length: 0))
90+
91+
#expect(!placement.canExitUp)
92+
#expect(placement.canExitDown)
93+
}
94+
95+
/// The selection comes from the text view, so it is already inside the value, but a stale one
96+
/// must not read past the end.
97+
@Test("A selection past the end of the value is clamped")
98+
func selectionPastTheEndIsClamped() {
99+
let placement = exit("alpha", selection: NSRange(location: 40, length: 10))
100+
101+
#expect(placement.canExitUp)
102+
#expect(placement.canExitDown)
103+
}
104+
}

0 commit comments

Comments
 (0)