From 3437002261fd2742a38124100c8db337a2f40266 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 01:31:16 +0700 Subject: [PATCH] refactor(datagrid): cell-based sort indicators with native divider behavior --- CHANGELOG.md | 3 +- TablePro/Views/Results/DataGridView.swift | 16 +- .../Views/Results/SortableHeaderCell.swift | 124 +++++++++++++++ .../Views/Results/SortableHeaderView.swift | 148 +++++++++++------- .../Results/SuppressedSortIndicatorCell.swift | 24 --- .../SortableHeaderResizeZoneTests.swift | 54 +++++++ 6 files changed, 282 insertions(+), 87 deletions(-) create mode 100644 TablePro/Views/Results/SortableHeaderCell.swift delete mode 100644 TablePro/Views/Results/SuppressedSortIndicatorCell.swift create mode 100644 TableProTests/Views/Results/SortableHeaderResizeZoneTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 7df62ea2c..ab1164bb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Data grid column identifiers are now the column name (with positional fallback for duplicate names), so saved widths follow the column across schema changes that shift its position. Identifier resolution moved from static `DataGridView` helpers to a `ColumnIdentitySchema` value type owned by the coordinator. - `ColumnLayoutStorage` singleton replaced by a `ColumnLayoutPersisting` protocol with an injectable `FileColumnLayoutPersister` default. The coordinator depends on the protocol, not the concrete class, so tests can substitute a fake. - Column layout save/restore on table-switch (`saveColumnLayoutForTable` / `restoreColumnLayoutForTable`) folded into the data grid coordinator's lifecycle (load on column build, persist on resize/move/dismantle). The standalone `MainContentCoordinator+ColumnLayout` extension is gone; only the visibility orchestration remains. Removes the redundant `hasUserResizedColumns` flag and the external save trigger from the binding setter. -- Data grid header sort indicators are `NSImageView` overlays drawn on a custom `NSTableHeaderView`, replacing Unicode arrows that were embedded in the column title string. The primary sorted column also gets the system header tint via `highlightedTableColumn`. VoiceOver announces the column name and sort direction separately. +- Data grid header sort indicators are drawn inside a custom `NSTableHeaderCell` instead of overlay `NSImageView` subviews, replacing Unicode arrows that were embedded in the column title string. Removing the overlay subviews lets `NSTableHeaderView`'s native cursor management run unimpeded, so the column resize cursor on hover works without any custom cursor handling. The primary sorted column gets the system header tint via `highlightedTableColumn`, and secondary sort columns show a small priority number to the left of the arrow. +- Data grid header divider taps trigger a column resize instead of sorting the adjacent column. `SortableHeaderView` checks if the click landed within 4 pt of a column edge and forwards the event to `NSTableHeaderView`'s native resize handling. - Data grid column layout persistence routes through a coordinator callback fired from outside SwiftUI's update cycle, removing the `Task`-based `@Binding` mutation inside `updateNSView` and the `isWritingColumnLayout` re-entry guard. - Data grid cell reuse resets foreign-key arrow and dropdown chevron button context (target, action, row, column) when the button hides, preventing a stale handler from firing the wrong row if the column toggles between FK-eligible and not. - `applyColumnOrder` scans only the unsettled tail of the column array per move, halving the constant cost on reorders with many columns. diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 99a13c9dd..779644b25 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -86,10 +86,10 @@ struct DataGridView: NSViewRepresentable { for (index, columnName) in initialRows.columns.enumerated() { guard let identifier = identitySchema.identifier(for: index) else { continue } let column = NSTableColumn(identifier: identifier) - let suppressedCell = SuppressedSortIndicatorCell(textCell: columnName) - suppressedCell.font = column.headerCell.font - suppressedCell.alignment = column.headerCell.alignment - column.headerCell = suppressedCell + let sortableCell = SortableHeaderCell(textCell: columnName) + sortableCell.font = column.headerCell.font + sortableCell.alignment = column.headerCell.alignment + column.headerCell = sortableCell if index < initialRows.columnTypes.count { let typeName = initialRows.columnTypes[index].rawType ?? initialRows.columnTypes[index].displayName column.headerToolTip = "\(columnName) (\(typeName))" @@ -332,10 +332,10 @@ struct DataGridView: NSViewRepresentable { for (index, columnName) in tableRows.columns.enumerated() { guard let identifier = schema.identifier(for: index) else { continue } let column = NSTableColumn(identifier: identifier) - let suppressedCell = SuppressedSortIndicatorCell(textCell: columnName) - suppressedCell.font = column.headerCell.font - suppressedCell.alignment = column.headerCell.alignment - column.headerCell = suppressedCell + let sortableCell = SortableHeaderCell(textCell: columnName) + sortableCell.font = column.headerCell.font + sortableCell.alignment = column.headerCell.alignment + column.headerCell = sortableCell if index < tableRows.columnTypes.count { let typeName = tableRows.columnTypes[index].rawType ?? tableRows.columnTypes[index].displayName diff --git a/TablePro/Views/Results/SortableHeaderCell.swift b/TablePro/Views/Results/SortableHeaderCell.swift new file mode 100644 index 000000000..d6460b26a --- /dev/null +++ b/TablePro/Views/Results/SortableHeaderCell.swift @@ -0,0 +1,124 @@ +// +// SortableHeaderCell.swift +// TablePro +// + +import AppKit + +@MainActor +final class SortableHeaderCell: NSTableHeaderCell { + var sortDirection: SortDirection? + var sortPriority: Int? + + private static let indicatorPadding: CGFloat = 4 + private static let indicatorSpacing: CGFloat = 2 + private static let priorityFontSize: CGFloat = 9 + + override init(textCell string: String) { + super.init(textCell: string) + } + + required init(coder: NSCoder) { + super.init(coder: coder) + } + + override func drawInterior(withFrame cellFrame: NSRect, in controlView: NSView) { + guard let direction = sortDirection else { + super.drawInterior(withFrame: cellFrame, in: controlView) + return + } + + let indicatorImage = Self.indicatorImage(for: direction) + let indicatorSize = indicatorImage?.size ?? NSSize(width: 9, height: 6) + let priorityText = priorityNumberString() + let priorityWidth = priorityText.map { Self.measureWidth(of: $0) } ?? 0 + let reservedWidth = indicatorSize.width + + Self.indicatorPadding * 2 + + (priorityText == nil ? 0 : priorityWidth + Self.indicatorSpacing) + + let titleFrame = NSRect( + x: cellFrame.minX, + y: cellFrame.minY, + width: max(0, cellFrame.width - reservedWidth), + height: cellFrame.height + ) + super.drawInterior(withFrame: titleFrame, in: controlView) + + let indicatorOriginX = cellFrame.maxX - Self.indicatorPadding - indicatorSize.width + let indicatorOriginY = cellFrame.midY - indicatorSize.height / 2 + let indicatorRect = NSRect( + x: indicatorOriginX, + y: indicatorOriginY, + width: indicatorSize.width, + height: indicatorSize.height + ) + Self.drawTintedIndicator(image: indicatorImage, in: indicatorRect) + + if let priorityText { + let textOriginX = indicatorOriginX - Self.indicatorSpacing - priorityWidth + let textRect = NSRect( + x: textOriginX, + y: cellFrame.minY, + width: priorityWidth, + height: cellFrame.height + ) + Self.drawPriorityText(priorityText, in: textRect) + } + } + + override func drawSortIndicator( + withFrame cellFrame: NSRect, + in controlView: NSView, + ascending: Bool, + priority: Int + ) {} + + private func priorityNumberString() -> String? { + guard let sortPriority, sortPriority >= 2 else { return nil } + return String(sortPriority) + } + + private static func indicatorImage(for direction: SortDirection) -> NSImage? { + switch direction { + case .ascending: + return NSImage(named: NSImage.Name("NSAscendingSortIndicator")) + case .descending: + return NSImage(named: NSImage.Name("NSDescendingSortIndicator")) + } + } + + private static func drawTintedIndicator(image: NSImage?, in rect: NSRect) { + guard let image else { return } + image.draw( + in: rect, + from: .zero, + operation: .sourceOver, + fraction: 1.0, + respectFlipped: true, + hints: nil + ) + } + + private static func drawPriorityText(_ text: String, in rect: NSRect) { + let attributes = priorityAttributes() + let textSize = (text as NSString).size(withAttributes: attributes) + let drawRect = NSRect( + x: rect.minX, + y: rect.midY - textSize.height / 2, + width: rect.width, + height: textSize.height + ) + (text as NSString).draw(in: drawRect, withAttributes: attributes) + } + + private static func measureWidth(of text: String) -> CGFloat { + (text as NSString).size(withAttributes: priorityAttributes()).width + } + + private static func priorityAttributes() -> [NSAttributedString.Key: Any] { + [ + .font: NSFont.systemFont(ofSize: priorityFontSize, weight: .medium), + .foregroundColor: NSColor.secondaryLabelColor + ] + } +} diff --git a/TablePro/Views/Results/SortableHeaderView.swift b/TablePro/Views/Results/SortableHeaderView.swift index 0cdbf27c3..2d51d712c 100644 --- a/TablePro/Views/Results/SortableHeaderView.swift +++ b/TablePro/Views/Results/SortableHeaderView.swift @@ -85,79 +85,114 @@ enum HeaderSortCycle { final class SortableHeaderView: NSTableHeaderView { weak var coordinator: TableViewCoordinator? - private var indicatorViews: [String: NSImageView] = [:] - private static let ascendingImage = NSImage(named: NSImage.Name("NSAscendingSortIndicator")) - private static let descendingImage = NSImage(named: NSImage.Name("NSDescendingSortIndicator")) + private static let clickDragThreshold: CGFloat = 4 + private static let resizeZoneWidth: CGFloat = 4 - func updateSortIndicators(state: SortState, schema: ColumnIdentitySchema) { - let activeKeys: Set = Set(state.columns.compactMap { - schema.identifier(for: $0.columnIndex)?.rawValue - }) + private var pendingClickStartLocation: NSPoint? + private var dragOccurredDuringClick = false + private var mouseMovedTrackingArea: NSTrackingArea? - for (key, view) in indicatorViews where !activeKeys.contains(key) { - view.removeFromSuperview() - indicatorViews.removeValue(forKey: key) - } + override init(frame frameRect: NSRect) { + super.init(frame: frameRect) + } - for sortCol in state.columns { - guard let identifier = schema.identifier(for: sortCol.columnIndex) else { continue } - let view = indicatorViews[identifier.rawValue] ?? makeIndicatorView() - view.image = sortCol.direction == .ascending ? Self.ascendingImage : Self.descendingImage - view.setAccessibilityLabel( - sortCol.direction == .ascending - ? String(localized: "Sort ascending") - : String(localized: "Sort descending") + required init?(coder: NSCoder) { + super.init(coder: coder) + } + + override func resetCursorRects() { + super.resetCursorRects() + guard let tableView = tableView else { return } + let zoneWidth = Self.resizeZoneWidth + for (index, column) in tableView.tableColumns.enumerated() { + guard column.resizingMask.contains(.userResizingMask) else { continue } + let columnRect = headerRect(ofColumn: index) + let cursorRect = NSRect( + x: columnRect.maxX - zoneWidth, + y: columnRect.minY, + width: zoneWidth * 2, + height: columnRect.height ) - if view.superview == nil { - addSubview(view) - } - indicatorViews[identifier.rawValue] = view + addCursorRect(cursorRect, cursor: .resizeLeftRight) } + } - repositionIndicators() + override func viewDidMoveToWindow() { + super.viewDidMoveToWindow() + window?.acceptsMouseMovedEvents = true + window?.invalidateCursorRects(for: self) } override func layout() { super.layout() - repositionIndicators() + window?.invalidateCursorRects(for: self) } - private func repositionIndicators() { + override func updateTrackingAreas() { + super.updateTrackingAreas() + if let existing = mouseMovedTrackingArea { + removeTrackingArea(existing) + } + let area = NSTrackingArea( + rect: bounds, + options: [.activeInKeyWindow, .mouseMoved, .inVisibleRect], + owner: self, + userInfo: nil + ) + addTrackingArea(area) + mouseMovedTrackingArea = area + } + + override func mouseMoved(with event: NSEvent) { + guard let tableView = tableView else { + super.mouseMoved(with: event) + return + } + let point = convert(event.locationInWindow, from: nil) + if isInResizeZone(point, in: tableView) { + NSCursor.resizeLeftRight.set() + } else { + NSCursor.arrow.set() + } + } + + func updateSortIndicators(state: SortState, schema: ColumnIdentitySchema) { guard let tableView = tableView else { return } - let padding: CGFloat = 4 - - for (key, view) in indicatorViews { - let identifier = NSUserInterfaceItemIdentifier(key) - let columnIndex = tableView.column(withIdentifier: identifier) - guard columnIndex >= 0 else { - view.isHidden = true - continue + + var priorityByIdentifier: [NSUserInterfaceItemIdentifier: (direction: SortDirection, priority: Int)] = [:] + for (index, sortCol) in state.columns.enumerated() { + guard let identifier = schema.identifier(for: sortCol.columnIndex) else { continue } + priorityByIdentifier[identifier] = (sortCol.direction, index + 1) + } + + for (columnIndex, column) in tableView.tableColumns.enumerated() { + guard let cell = column.headerCell as? SortableHeaderCell else { continue } + let entry = priorityByIdentifier[column.identifier] + let newDirection = entry?.direction + let newPriority = entry?.priority + if cell.sortDirection != newDirection || cell.sortPriority != newPriority { + cell.sortDirection = newDirection + cell.sortPriority = newPriority + setNeedsDisplay(headerRect(ofColumn: columnIndex)) } - view.isHidden = false - let columnRect = headerRect(ofColumn: columnIndex) - let imageSize = view.image?.size ?? NSSize(width: 9, height: 6) - view.frame = NSRect( - x: columnRect.maxX - imageSize.width - padding, - y: columnRect.midY - imageSize.height / 2, - width: imageSize.width, - height: imageSize.height - ) } } - private func makeIndicatorView() -> NSImageView { - let view = NSImageView() - view.imageScaling = .scaleNone - view.imageAlignment = .alignCenter - view.contentTintColor = .secondaryLabelColor - view.translatesAutoresizingMaskIntoConstraints = true - return view + static func isInResizeZone( + point: NSPoint, + columnEdges: [CGFloat], + zoneWidth: CGFloat = SortableHeaderView.resizeZoneWidth + ) -> Bool { + columnEdges.contains { abs(point.x - $0) <= zoneWidth } } - private static let clickDragThreshold: CGFloat = 4 - - private var pendingClickStartLocation: NSPoint? - private var dragOccurredDuringClick = false + private func isInResizeZone(_ point: NSPoint, in tableView: NSTableView) -> Bool { + let edges = tableView.tableColumns.enumerated().compactMap { index, column -> CGFloat? in + guard column.resizingMask.contains(.userResizingMask) else { return nil } + return headerRect(ofColumn: index).maxX + } + return Self.isInResizeZone(point: point, columnEdges: edges) + } override func mouseDragged(with event: NSEvent) { if let start = pendingClickStartLocation { @@ -178,6 +213,11 @@ final class SortableHeaderView: NSTableHeaderView { } let pointInHeader = convert(event.locationInWindow, from: nil) + if isInResizeZone(pointInHeader, in: tableView) { + super.mouseDown(with: event) + return + } + let columnIndex = column(at: pointInHeader) guard columnIndex >= 0, columnIndex < tableView.numberOfColumns else { super.mouseDown(with: event) diff --git a/TablePro/Views/Results/SuppressedSortIndicatorCell.swift b/TablePro/Views/Results/SuppressedSortIndicatorCell.swift deleted file mode 100644 index 95883d8a4..000000000 --- a/TablePro/Views/Results/SuppressedSortIndicatorCell.swift +++ /dev/null @@ -1,24 +0,0 @@ -// -// SuppressedSortIndicatorCell.swift -// TablePro -// - -import AppKit - -@MainActor -final class SuppressedSortIndicatorCell: NSTableHeaderCell { - override init(textCell string: String) { - super.init(textCell: string) - } - - required init(coder: NSCoder) { - super.init(coder: coder) - } - - override func drawSortIndicator( - withFrame cellFrame: NSRect, - in controlView: NSView, - ascending: Bool, - priority: Int - ) {} -} diff --git a/TableProTests/Views/Results/SortableHeaderResizeZoneTests.swift b/TableProTests/Views/Results/SortableHeaderResizeZoneTests.swift new file mode 100644 index 000000000..e4452c2b1 --- /dev/null +++ b/TableProTests/Views/Results/SortableHeaderResizeZoneTests.swift @@ -0,0 +1,54 @@ +// +// SortableHeaderResizeZoneTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +@MainActor +@Suite("SortableHeaderView resize zone detection") +struct SortableHeaderResizeZoneTests { + @Test("Point on column edge is inside resize zone") + func pointOnEdgeIsInside() { + let edges: [CGFloat] = [60, 220, 380] + #expect(SortableHeaderView.isInResizeZone(point: NSPoint(x: 60, y: 8), columnEdges: edges)) + #expect(SortableHeaderView.isInResizeZone(point: NSPoint(x: 220, y: 8), columnEdges: edges)) + } + + @Test("Point within zoneWidth of an edge is inside resize zone") + func pointNearEdgeIsInside() { + let edges: [CGFloat] = [100] + #expect(SortableHeaderView.isInResizeZone(point: NSPoint(x: 96, y: 8), columnEdges: edges)) + #expect(SortableHeaderView.isInResizeZone(point: NSPoint(x: 104, y: 8), columnEdges: edges)) + } + + @Test("Point outside zoneWidth of every edge is not in resize zone") + func pointFarFromEdgeIsOutside() { + let edges: [CGFloat] = [100, 200] + #expect(!SortableHeaderView.isInResizeZone(point: NSPoint(x: 50, y: 8), columnEdges: edges)) + #expect(!SortableHeaderView.isInResizeZone(point: NSPoint(x: 150, y: 8), columnEdges: edges)) + #expect(!SortableHeaderView.isInResizeZone(point: NSPoint(x: 250, y: 8), columnEdges: edges)) + } + + @Test("Empty edge list never matches") + func emptyEdgesNeverMatches() { + #expect(!SortableHeaderView.isInResizeZone(point: NSPoint(x: 0, y: 0), columnEdges: [])) + #expect(!SortableHeaderView.isInResizeZone(point: NSPoint(x: 100, y: 0), columnEdges: [])) + } + + @Test("Custom zone width widens the match band") + func customZoneWidthWidensBand() { + let edges: [CGFloat] = [100] + #expect(!SortableHeaderView.isInResizeZone( + point: NSPoint(x: 92, y: 0), + columnEdges: edges + )) + #expect(SortableHeaderView.isInResizeZone( + point: NSPoint(x: 92, y: 0), + columnEdges: edges, + zoneWidth: 8 + )) + } +}