diff --git a/CHANGELOG.md b/CHANGELOG.md index b032124f9..97b2309d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Shift+Arrow in the data grid now starts and extends a cell selection from the focused cell, instead of doing nothing until a range already existed. Cmd+Shift+Arrow extends to the row or column edge. - Delete key now respects cell-range selection in the data grid, removing all rows covered by the selection instead of ignoring it. - Right-clicking inside a multi-row or cell-range selection no longer collapses the selection before the context menu appears. - Oracle connections no longer crash the app during connect. A short or unexpected handshake packet from the server (such as session-setup metadata or an error) now surfaces the error or continues instead of trapping. (#1683) diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index 365d0c7a9..d9b92c23d 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -321,24 +321,16 @@ final class KeyHandlingTableView: NSTableView { switch key { case .leftArrow: - handleHorizontalArrow(direction: .left, modifiers: modifiers, currentRow: row) + handleArrow(.left, modifiers: modifiers, currentRow: row, event: event) return case .rightArrow: - handleHorizontalArrow(direction: .right, modifiers: modifiers, currentRow: row) + handleArrow(.right, modifiers: modifiers, currentRow: row, event: event) return case .upArrow: - if modifiers.contains(.shift) { - gridSelection?.extendActiveCell(direction: .up, jumpToEdge: modifiers.contains(.command), totalRows: totalRows(), totalColumns: totalDataColumns()) - return - } - super.keyDown(with: event) + handleArrow(.up, modifiers: modifiers, currentRow: row, event: event) return case .downArrow: - if modifiers.contains(.shift) { - gridSelection?.extendActiveCell(direction: .down, jumpToEdge: modifiers.contains(.command), totalRows: totalRows(), totalColumns: totalDataColumns()) - return - } - super.keyDown(with: event) + handleArrow(.down, modifiers: modifiers, currentRow: row, event: event) return case .home, .end, .pageUp, .pageDown: super.keyDown(with: event) @@ -378,18 +370,41 @@ final class KeyHandlingTableView: NSTableView { return combo.matches(event) } - private func handleHorizontalArrow(direction: GridSelectionController.Direction, modifiers: NSEvent.ModifierFlags, currentRow: Int) { - if modifiers.contains(.shift), let controller = gridSelection, !controller.isEmpty { - controller.extendActiveCell(direction: direction, jumpToEdge: modifiers.contains(.command), totalRows: totalRows(), totalColumns: totalDataColumns()) + private func handleArrow(_ direction: GridSelectionController.Direction, modifiers: NSEvent.ModifierFlags, currentRow: Int, event: NSEvent) { + if modifiers.contains(.shift) { + if extendGridSelection(direction: direction, jumpToEdge: modifiers.contains(.command)) { + return + } + super.keyDown(with: event) return } + gridSelection?.clear() switch direction { case .left: handleLeftArrow(currentRow: currentRow) case .right: handleRightArrow(currentRow: currentRow) - default: break + case .up, .down: super.keyDown(with: event) } } + private func extendGridSelection(direction: GridSelectionController.Direction, jumpToEdge: Bool) -> Bool { + guard let controller = gridSelection else { return false } + let seed = controller.isEmpty ? focusedGridCoord() : nil + guard !controller.isEmpty || seed != nil else { return false } + controller.extendActiveCell( + from: seed, + direction: direction, + jumpToEdge: jumpToEdge, + totalRows: totalRows(), + totalColumns: totalDataColumns() + ) + return true + } + + private func focusedGridCoord() -> GridCoord? { + guard let cell = focusedDataCell() else { return nil } + return GridCoord(row: cell.row, column: cell.columnIndex) + } + @objc override func insertNewline(_ sender: Any?) { let row = selectedRow guard row >= 0, diff --git a/TablePro/Views/Results/Selection/GridSelectionController.swift b/TablePro/Views/Results/Selection/GridSelectionController.swift index aacff4dcd..ca300ea0c 100644 --- a/TablePro/Views/Results/Selection/GridSelectionController.swift +++ b/TablePro/Views/Results/Selection/GridSelectionController.swift @@ -159,10 +159,10 @@ final class GridSelectionController { update(.single(rect, anchor: anchor, active: anchor)) } - func extendActiveCell(direction: Direction, jumpToEdge: Bool, totalRows: Int, totalColumns: Int) { - guard let active = selection.activeCell else { return } + func extendActiveCell(from seed: GridCoord? = nil, direction: Direction, jumpToEdge: Bool, totalRows: Int, totalColumns: Int) { + guard let active = selection.activeCell ?? seed else { return } + let origin = selection.anchor ?? seed ?? active let next = step(from: active, direction: direction, jumpToEdge: jumpToEdge, totalRows: totalRows, totalColumns: totalColumns) - let origin = selection.anchor ?? active update(.single(GridRect.between(origin, next), anchor: origin, active: next)) } diff --git a/TableProTests/Views/Results/CellSelectionTests.swift b/TableProTests/Views/Results/CellSelectionTests.swift index cf83d74b1..980eb42eb 100644 --- a/TableProTests/Views/Results/CellSelectionTests.swift +++ b/TableProTests/Views/Results/CellSelectionTests.swift @@ -291,13 +291,84 @@ struct GridSelectionControllerTests { #expect(controller.selection.rectangles == [GridRect(rows: 2...2, columns: 2...9)]) } - @Test("extendActiveCell is a no-op when the selection is empty") + @Test("extendActiveCell without a seed is a no-op when the selection is empty") func extendActiveCellNoOpEmpty() { let controller = GridSelectionController() controller.extendActiveCell(direction: .down, jumpToEdge: false, totalRows: 10, totalColumns: 10) #expect(controller.selection.isEmpty) } + @Test("extendActiveCell with a seed begins a range anchored at the focused cell") + func extendActiveCellSeedsFromFocusedCell() { + let controller = GridSelectionController() + let focused = GridCoord(row: 3, column: 4) + + controller.extendActiveCell(from: focused, direction: .down, jumpToEdge: false, totalRows: 10, totalColumns: 10) + + #expect(controller.selection.rectangles == [GridRect(rows: 3...4, columns: 4...4)]) + #expect(controller.selection.anchor == focused) + #expect(controller.selection.activeCell == GridCoord(row: 4, column: 4)) + } + + @Test("a seeded extend grows by one cell in each direction from the focused cell") + func extendActiveCellSeedsInEveryDirection() { + let focused = GridCoord(row: 3, column: 3) + let cases: [(GridSelectionController.Direction, GridRect, GridCoord)] = [ + (.up, GridRect(rows: 2...3, columns: 3...3), GridCoord(row: 2, column: 3)), + (.down, GridRect(rows: 3...4, columns: 3...3), GridCoord(row: 4, column: 3)), + (.left, GridRect(rows: 3...3, columns: 2...3), GridCoord(row: 3, column: 2)), + (.right, GridRect(rows: 3...3, columns: 3...4), GridCoord(row: 3, column: 4)) + ] + for (direction, expectedRect, expectedActive) in cases { + let controller = GridSelectionController() + controller.extendActiveCell(from: focused, direction: direction, jumpToEdge: false, totalRows: 10, totalColumns: 10) + #expect(controller.selection.rectangles == [expectedRect]) + #expect(controller.selection.anchor == focused) + #expect(controller.selection.activeCell == expectedActive) + } + } + + @Test("repeated seeded extend keeps the anchor fixed and reverses by shrinking") + func extendActiveCellAnchorStaysFixedOnReverse() { + let controller = GridSelectionController() + let focused = GridCoord(row: 2, column: 2) + + controller.extendActiveCell(from: focused, direction: .down, jumpToEdge: false, totalRows: 10, totalColumns: 10) + controller.extendActiveCell(direction: .down, jumpToEdge: false, totalRows: 10, totalColumns: 10) + #expect(controller.selection.rectangles == [GridRect(rows: 2...4, columns: 2...2)]) + + controller.extendActiveCell(direction: .up, jumpToEdge: false, totalRows: 10, totalColumns: 10) + #expect(controller.selection.rectangles == [GridRect(rows: 2...3, columns: 2...2)]) + #expect(controller.selection.anchor == focused) + #expect(controller.selection.activeCell == GridCoord(row: 3, column: 2)) + } + + @Test("a seeded extend with jumpToEdge runs from the focused cell to the grid edge") + func extendActiveCellSeedsToEdge() { + let controller = GridSelectionController() + let focused = GridCoord(row: 2, column: 2) + + controller.extendActiveCell(from: focused, direction: .right, jumpToEdge: true, totalRows: 10, totalColumns: 10) + + #expect(controller.selection.rectangles == [GridRect(rows: 2...2, columns: 2...9)]) + #expect(controller.selection.anchor == focused) + #expect(controller.selection.activeCell == GridCoord(row: 2, column: 9)) + } + + @Test("the seed is ignored when a selection already exists") + func extendActiveCellIgnoresSeedWhenSelectionExists() { + let controller = GridSelectionController() + let origin = GridCoord(row: 2, column: 2) + _ = controller.beginDrag(at: origin, modifiers: []) + controller.continueDrag(to: origin) + controller.endDrag(dragged: true, originalCoord: origin) + + controller.extendActiveCell(from: GridCoord(row: 9, column: 9), direction: .down, jumpToEdge: false, totalRows: 10, totalColumns: 10) + + #expect(controller.selection.rectangles == [GridRect(rows: 2...3, columns: 2...2)]) + #expect(controller.selection.anchor == origin) + } + @Test("clear empties the selection") func clearEmpties() { let controller = GridSelectionController()