diff --git a/CHANGELOG.md b/CHANGELOG.md index 8002a0ac4..c3d479b0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A column comment in the data grid header is no longer crossed by the header's bottom line, and the empty strip under it is gone. The line now sits on the header's bottom edge where it belongs. (#2017) +- A sorted column no longer draws an extra line through its comment or an extra divider down its left edge. Its header now looks like every other column's. (#2017) +- Selecting a column now highlights the full height of its header instead of a band across the middle. (#2017) - The New Connection window now comes to the front on the first try instead of opening behind the Welcome window. This applies to Import from URL, creating a connection from a project folder, picking a database type, File > New Connection, and duplicating a connection. - Importing a connection URL while a New Connection window was already open no longer throws the pasted URL away. Each import now opens its own window instead of re-using the one already on screen. - Clicking a foreign key arrow in a query tab's results no longer replaces that tab and loses the query and its results. The referenced table opens in its own tab, and clicking the same reference again returns to that tab instead of opening a duplicate. A tab with unsaved cell edits is kept the same way. diff --git a/CLAUDE.md b/CLAUDE.md index 23957042d..90e1b386e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -175,6 +175,8 @@ These have caused real bugs when violated: **A SwiftUI-hosted split view needs an explicit divider cursor**: `NSSplitView` shows the resize cursor over its dividers through AppKit's cursor-rects system, which does not fire once the split view is mounted inside an `NSHostingController` (every tab-content split is, several SwiftUI layers deep under `MainSplitViewController.detailHosting`). The divider still drags because drag hit-testing is independent of cursor rects, but the pointer never changes. Every SwiftUI-hosted split-view controller must subclass `ResizeCursorSplitViewController`, which adds a key-window tracking area to its own split view and sets `NSCursor.columnResize`/`rowResize` (falling back to `resizeLeftRight`/`resizeUpDown` before macOS 15) in `mouseMoved`, the same hand-rolled approach `SortableHeaderView` uses for column resize. It attaches the tracking area to the framework's split view in `viewDidLoad` rather than replacing the split view, so `NSSplitViewController`'s own layout and divider orientation stay intact; replacing the split view through a `loadView` override that skips `super` leaves the controller half-initialized and its panes stack instead of laying out side by side. Do not swap the controller back to a plain `NSSplitViewController` expecting the stock cursor to work; the window's own sidebar and inspector dividers only get the cursor for free because `MainSplitViewController` is the window's `contentViewController` directly, with no SwiftUI host in between. This shipped as Users & Roles, Structure, Server Dashboard, and query editor dividers that dragged but never showed the resize cursor (#1905). +**The data grid header owns all of its own chrome, so nothing may ask AppKit to paint any of it**: `NSTableHeaderCell` and `NSTableHeaderView` both paint a fixed 28pt band that they centre vertically in whatever frame they are given, a 16pt column divider on `midY` and a 1pt rule at `midY + 13`. The data grid grows its header to 42pt for a column comment, so that band lands mid-cell: the rule crosses the comment's descenders and sits 8pt above the real bottom edge. `SortableHeaderChrome` is therefore the single owner of header geometry and colours, `SortableHeaderCell.draw(withFrame:in:)` never calls `super`, and `SortableHeaderView.draw(_:)` fills the background and rules the bottom edge itself. The trap is that the header view paints a second copy of that same band for `NSTableView.highlightedTableColumn`, driven by *state* rather than by a drawing call, so no cell override can reach it: setting it gives the sorted column a stray divider and a rule no other column has. TablePro already draws the sorted-column affordance itself (bold title, chevron, priority number, with `drawSortIndicator` overridden to nothing), so `highlightedTableColumn` is a redundant second channel and must stay unset. All sorted-column presentation goes through `SortableHeaderView.applySortState(_:schema:)`, which publishes the order natively through `tableView.sortDescriptors` (for accessibility; it paints nothing) and updates the cells. `SortableHeaderRenderingTests` rasterises the header and guards this. This shipped as a rule through the comment line and a stray divider on the sorted column (#2017). + ### Main Coordinator Pattern `MainContentCoordinator` is the central coordinator, split across 7+ extension files in `Views/Main/Extensions/` (e.g., `+Alerts`, `+Filtering`, `+Pagination`, `+RowOperations`). When adding coordinator functionality, add a new extension file rather than growing the main file. diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 9910ddf92..e5241cb43 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -195,7 +195,7 @@ struct DataGridView: NSViewRepresentable { coordinator.lastUpdateSnapshot = snapshot } - syncSortDescriptors(tableView: tableView, coordinator: coordinator, columns: latestRows.columns) + syncSortState(tableView: tableView, coordinator: coordinator) syncSelection(tableView: tableView, coordinator: coordinator) } @@ -350,39 +350,10 @@ struct DataGridView: NSViewRepresentable { ) } - private func syncSortDescriptors(tableView: NSTableView, coordinator: TableViewCoordinator, columns: [String]) { + private func syncSortState(tableView: NSTableView, coordinator: TableViewCoordinator) { coordinator.currentSortState = sortState - - let schema = coordinator.identitySchema - let primaryIdentifier: NSUserInterfaceItemIdentifier? - let primary: NSSortDescriptor? - if let firstSort = sortState.columns.first, - let identifier = schema.identifier(for: firstSort.columnIndex), - let name = schema.columnName(for: firstSort.columnIndex) { - primaryIdentifier = identifier - primary = NSSortDescriptor(key: name, ascending: firstSort.direction == .ascending) - } else { - primaryIdentifier = nil - primary = nil - } - - let desired = primary.map { [$0] } ?? [] - let current = tableView.sortDescriptors.first - let needsUpdate = (current?.key != primary?.key) || (current?.ascending != primary?.ascending) - if needsUpdate { - tableView.sortDescriptors = desired - } - - if let primaryIdentifier { - let columnIndex = tableView.column(withIdentifier: primaryIdentifier) - tableView.highlightedTableColumn = columnIndex >= 0 ? tableView.tableColumns[columnIndex] : nil - } else { - tableView.highlightedTableColumn = nil - } - - if let header = tableView.headerView as? SortableHeaderView { - header.updateSortIndicators(state: sortState, schema: schema) - } + guard let header = tableView.headerView as? SortableHeaderView else { return } + header.applySortState(sortState, schema: coordinator.identitySchema) } // MARK: - Column Layout Helpers diff --git a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift index 783448985..18f34960b 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift @@ -242,7 +242,7 @@ extension TableViewCoordinator { var state = SortState() state.columns = [SortColumn(columnIndex: columnIndex, direction: .ascending)] currentSortState = state - updateSortIndicatorsFromCurrentState() + applyCurrentSortStateToHeader() delegate?.dataGridSortStateChanged(state) } @@ -251,7 +251,7 @@ extension TableViewCoordinator { var state = SortState() state.columns = [SortColumn(columnIndex: columnIndex, direction: .descending)] currentSortState = state - updateSortIndicatorsFromCurrentState() + applyCurrentSortStateToHeader() delegate?.dataGridSortStateChanged(state) } @@ -261,13 +261,13 @@ extension TableViewCoordinator { @objc func clearSortAction() { currentSortState = SortState() - updateSortIndicatorsFromCurrentState() + applyCurrentSortStateToHeader() delegate?.dataGridSortStateChanged(SortState()) } - private func updateSortIndicatorsFromCurrentState() { + private func applyCurrentSortStateToHeader() { guard let header = tableView?.headerView as? SortableHeaderView else { return } - header.updateSortIndicators(state: currentSortState, schema: identitySchema) + header.applySortState(currentSortState, schema: identitySchema) } @objc func copyColumnName(_ sender: NSMenuItem) { diff --git a/TablePro/Views/Results/SortableHeaderCell.swift b/TablePro/Views/Results/SortableHeaderCell.swift index 2a2de237b..2eacb4cbe 100644 --- a/TablePro/Views/Results/SortableHeaderCell.swift +++ b/TablePro/Views/Results/SortableHeaderCell.swift @@ -47,6 +47,11 @@ final class SortableHeaderCell: NSTableHeaderCell { wraps = false } + override func draw(withFrame cellFrame: NSRect, in controlView: NSView) { + SortableHeaderChrome.drawColumnDivider(in: cellFrame) + drawInterior(withFrame: cellFrame, in: controlView) + } + override func drawInterior(withFrame cellFrame: NSRect, in controlView: NSView) { if isColumnSelected { NSColor.selectedContentBackgroundColor.setFill() diff --git a/TablePro/Views/Results/SortableHeaderChrome.swift b/TablePro/Views/Results/SortableHeaderChrome.swift new file mode 100644 index 000000000..8eb7aba26 --- /dev/null +++ b/TablePro/Views/Results/SortableHeaderChrome.swift @@ -0,0 +1,38 @@ +// +// SortableHeaderChrome.swift +// TablePro +// + +import AppKit + +@MainActor +enum SortableHeaderChrome { + static let separatorThickness: CGFloat = 1 + static let columnDividerHeight: CGFloat = 16 + + static func fillBackground(_ rect: NSRect) { + NSColor.windowBackgroundColor.setFill() + rect.fill() + } + + static func drawBottomSeparator(in bounds: NSRect) { + NSColor.separatorColor.setFill() + NSRect( + x: bounds.minX, + y: bounds.maxY - separatorThickness, + width: bounds.width, + height: separatorThickness + ).fill() + } + + static func drawColumnDivider(in cellFrame: NSRect) { + let dividerHeight = min(columnDividerHeight, cellFrame.height) + NSColor.separatorColor.setFill() + NSRect( + x: cellFrame.maxX - separatorThickness, + y: cellFrame.midY - dividerHeight / 2, + width: separatorThickness, + height: dividerHeight + ).fill() + } +} diff --git a/TablePro/Views/Results/SortableHeaderView.swift b/TablePro/Views/Results/SortableHeaderView.swift index 06cc1ef9b..e55a523bf 100644 --- a/TablePro/Views/Results/SortableHeaderView.swift +++ b/TablePro/Views/Results/SortableHeaderView.swift @@ -119,6 +119,12 @@ final class SortableHeaderView: NSTableHeaderView { needsDisplay = true } + override func draw(_ dirtyRect: NSRect) { + SortableHeaderChrome.fillBackground(dirtyRect) + super.draw(dirtyRect) + SortableHeaderChrome.drawBottomSeparator(in: bounds) + } + override func updateTrackingAreas() { super.updateTrackingAreas() if let existing = mouseMovedTrackingArea { @@ -216,9 +222,31 @@ final class SortableHeaderView: NSTableHeaderView { } } - func updateSortIndicators(state: SortState, schema: ColumnIdentitySchema) { - guard let tableView = tableView else { return } + func applySortState(_ state: SortState, schema: ColumnIdentitySchema) { + guard let tableView else { return } + applySortDescriptors(state: state, schema: schema, in: tableView) + applySortIndicators(state: state, schema: schema, in: tableView) + } + private func applySortDescriptors( + state: SortState, + schema: ColumnIdentitySchema, + in tableView: NSTableView + ) { + var primary: NSSortDescriptor? + if let leading = state.columns.first, let name = schema.columnName(for: leading.columnIndex) { + primary = NSSortDescriptor(key: name, ascending: leading.direction == .ascending) + } + let current = tableView.sortDescriptors.first + guard current?.key != primary?.key || current?.ascending != primary?.ascending else { return } + tableView.sortDescriptors = primary.map { [$0] } ?? [] + } + + private func applySortIndicators( + state: SortState, + schema: ColumnIdentitySchema, + in tableView: NSTableView + ) { var priorityByIdentifier: [NSUserInterfaceItemIdentifier: (direction: SortDirection, priority: Int)] = [:] for (index, sortCol) in state.columns.enumerated() { guard let identifier = schema.identifier(for: sortCol.columnIndex) else { continue } @@ -315,7 +343,7 @@ final class SortableHeaderView: NSTableHeaderView { ) coordinator.currentSortState = transition.newState - updateSortIndicators(state: transition.newState, schema: coordinator.identitySchema) + applySortState(transition.newState, schema: coordinator.identitySchema) coordinator.delegate?.dataGridSortStateChanged(transition.newState) } } diff --git a/TableProTests/Views/Results/SortableHeaderRenderingTests.swift b/TableProTests/Views/Results/SortableHeaderRenderingTests.swift new file mode 100644 index 000000000..57406933c --- /dev/null +++ b/TableProTests/Views/Results/SortableHeaderRenderingTests.swift @@ -0,0 +1,213 @@ +// +// SortableHeaderRenderingTests.swift +// TableProTests +// + +import AppKit +import Testing + +@testable import TablePro + +@Suite("SortableHeaderView chrome rendering") +@MainActor +struct SortableHeaderRenderingTests { + private struct Grid { + let window: NSWindow + let tableView: NSTableView + let headerView: SortableHeaderView + let headerCells: [SortableHeaderCell] + let schema: ColumnIdentitySchema + + var headerCell: SortableHeaderCell { headerCells[0] } + } + + private static let contentWidth: CGFloat = 400 + private static let columnAreaWidth: CGFloat = 360 + + private func makeGrid(comment: String?, columns: [String] = ["code"]) -> Grid { + let scrollView = NSScrollView(frame: NSRect(x: 0, y: 0, width: Self.contentWidth, height: 200)) + let tableView = NSTableView(frame: scrollView.bounds) + tableView.style = .plain + + let schema = ColumnIdentitySchema(columns: columns) + var cells: [SortableHeaderCell] = [] + var comments: [NSUserInterfaceItemIdentifier: String] = [:] + for (index, name) in columns.enumerated() { + let column = NSTableColumn(identifier: ColumnIdentitySchema.slotIdentifier(index)) + column.width = Self.columnAreaWidth / CGFloat(columns.count) + let headerCell = SortableHeaderCell(textCell: name) + headerCell.font = column.headerCell.font + column.headerCell = headerCell + cells.append(headerCell) + if let comment { + comments[column.identifier] = comment + } + tableView.addTableColumn(column) + } + + let headerView = SortableHeaderView(frame: tableView.headerView?.frame ?? .zero) + tableView.headerView = headerView + scrollView.documentView = tableView + + let window = makeWindow(content: scrollView) + + if comment != nil { + headerView.updateComments(comments) + headerView.showsComments = true + } + + scrollView.tile() + window.layoutIfNeeded() + return Grid( + window: window, + tableView: tableView, + headerView: headerView, + headerCells: cells, + schema: schema + ) + } + + private func makeWindow(content: NSView) -> NSWindow { + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: Self.contentWidth, height: 200), + styleMask: [.titled], + backing: .buffered, + defer: false + ) + window.appearance = NSAppearance(named: .darkAqua) + window.contentView = content + window.layoutIfNeeded() + return window + } + + private func sortState(column: Int, direction: SortDirection = .ascending) -> SortState { + var state = SortState() + state.columns = [SortColumn(columnIndex: column, direction: direction)] + return state + } + + private func brightnessColumn(of view: NSView, atX sampleX: CGFloat) -> [CGFloat] { + guard let rep = view.bitmapImageRepForCachingDisplay(in: view.bounds) else { return [] } + view.cacheDisplay(in: view.bounds, to: rep) + let scale = CGFloat(rep.pixelsHigh) / view.bounds.height + let x = min(rep.pixelsWide - 1, Int(sampleX * scale)) + return (0 ..< rep.pixelsHigh).map { rep.colorAt(x: x, y: $0)?.brightnessComponent ?? 0 } + } + + private func ruleOffsets(in view: NSView, atX sampleX: CGFloat = 300) -> [CGFloat] { + let brightness = brightnessColumn(of: view, atX: sampleX) + guard !brightness.isEmpty else { return [] } + let scale = CGFloat(brightness.count) / view.bounds.height + let span = max(1, Int((3 * scale).rounded())) + return brightness.indices.compactMap { index in + guard abs(brightness[index] - brightness[max(0, index - span)]) > 0.02 else { return nil } + return CGFloat(index) / scale + } + } + + private func isAlongBottomEdge(_ offset: CGFloat, of view: NSView) -> Bool { + offset >= view.bounds.height - SortableHeaderChrome.separatorThickness + } + + @Test("The comment header draws its only horizontal rule along the bottom edge") + func commentHeaderRuleSitsAlongBottomEdge() { + let grid = makeGrid(comment: "ISO 4217 exponent for the minor unit") + let offsets = ruleOffsets(in: grid.headerView) + + #expect(!offsets.isEmpty) + #expect(offsets.allSatisfy { isAlongBottomEdge($0, of: grid.headerView) }) + } + + @Test("The comment header draws no horizontal rule across the comment line") + func commentHeaderDrawsNoRuleAcrossTheCommentLine() { + let grid = makeGrid(comment: "ISO 4217 exponent for the minor unit") + let naturalHeight = grid.headerView.commentHeaderHeight - SortableHeaderCell.commentLineHeight + let centredBandBottom = (grid.headerView.commentHeaderHeight + naturalHeight) / 2 + - SortableHeaderChrome.separatorThickness + + let offsets = ruleOffsets(in: grid.headerView) + + #expect(!offsets.contains { abs($0 - centredBandBottom) < 1 }) + } + + @Test("The bottom rule spans past the trailing edge of the last column") + func bottomRuleSpansPastTheLastColumn() { + let grid = makeGrid(comment: "ISO 4217 exponent for the minor unit") + let offsets = ruleOffsets(in: grid.headerView, atX: 390) + + #expect(!offsets.isEmpty) + #expect(offsets.allSatisfy { isAlongBottomEdge($0, of: grid.headerView) }) + } + + @Test("The natural height header keeps its rule along the bottom edge") + func naturalHeaderRuleSitsAlongBottomEdge() { + let grid = makeGrid(comment: nil) + let offsets = ruleOffsets(in: grid.headerView) + + #expect(!offsets.isEmpty) + #expect(offsets.allSatisfy { isAlongBottomEdge($0, of: grid.headerView) }) + } + + @Test("Selecting a column highlights the full height of the comment header") + func selectionHighlightSpansTheCommentHeader() { + let grid = makeGrid(comment: "ISO 4217 exponent for the minor unit") + let unselected = brightnessColumn(of: grid.headerView, atX: 300) + + grid.headerCell.isColumnSelected = true + grid.headerView.needsDisplay = true + let selected = brightnessColumn(of: grid.headerView, atX: 300) + + #expect(!unselected.isEmpty) + #expect(unselected.count == selected.count) + let repainted = zip(unselected, selected).filter { abs($0 - $1) > 0.02 }.count + #expect(repainted >= unselected.count - 4) + } + + @Test("A sorted column keeps its only horizontal rule along the bottom edge") + func sortedCommentHeaderRuleSitsAlongBottomEdge() { + let grid = makeGrid(comment: "ISO 4217 exponent for the minor unit") + grid.headerView.applySortState(sortState(column: 0), schema: grid.schema) + grid.headerView.needsDisplay = true + + let offsets = ruleOffsets(in: grid.headerView) + + #expect(grid.headerCell.sortDirection == .ascending) + #expect(!offsets.isEmpty) + #expect(offsets.allSatisfy { isAlongBottomEdge($0, of: grid.headerView) }) + } + + @Test("Sorting a column leaves every column divider where it was") + func sortingDoesNotMoveColumnDividers() { + let grid = makeGrid(comment: "ISO 4217 exponent for the minor unit", columns: ["code", "amount"]) + let leadingEdge = grid.headerView.headerRect(ofColumn: 1).minX + let unsorted = brightnessColumn(of: grid.headerView, atX: leadingEdge) + + grid.headerView.applySortState(sortState(column: 1), schema: grid.schema) + grid.headerView.needsDisplay = true + let sorted = brightnessColumn(of: grid.headerView, atX: leadingEdge) + + #expect(!unsorted.isEmpty) + #expect(unsorted == sorted) + } + + @Test("Sorting never hands the column to AppKit's own header highlight") + func sortingDoesNotUseTheAppKitColumnHighlight() { + let grid = makeGrid(comment: "ISO 4217 exponent for the minor unit", columns: ["code", "amount"]) + grid.headerView.applySortState(sortState(column: 1), schema: grid.schema) + + #expect(grid.tableView.highlightedTableColumn == nil) + } + + @Test("Applying a sort state publishes it through the table's sort descriptors") + func sortStatePublishesSortDescriptors() { + let grid = makeGrid(comment: nil, columns: ["code", "amount"]) + + grid.headerView.applySortState(sortState(column: 1, direction: .descending), schema: grid.schema) + #expect(grid.tableView.sortDescriptors.count == 1) + #expect(grid.tableView.sortDescriptors.first?.key == "amount") + #expect(grid.tableView.sortDescriptors.first?.ascending == false) + + grid.headerView.applySortState(SortState(), schema: grid.schema) + #expect(grid.tableView.sortDescriptors.isEmpty) + } +}