From cbebdb108939e73f79c75f0c510d16c316daf441 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 28 Apr 2026 03:17:37 +0700 Subject: [PATCH 1/8] refactor(datagrid): use native focus ring instead of CALayer border --- .../Views/Results/CellOverlayEditor.swift | 2 +- .../Views/Results/DataGridCellFactory.swift | 10 ++-------- TablePro/Views/Results/DataGridCellView.swift | 20 +++++++++++++++---- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/TablePro/Views/Results/CellOverlayEditor.swift b/TablePro/Views/Results/CellOverlayEditor.swift index 0fbe3e61c..6bae466f9 100644 --- a/TablePro/Views/Results/CellOverlayEditor.swift +++ b/TablePro/Views/Results/CellOverlayEditor.swift @@ -109,7 +109,7 @@ final class CellOverlayEditor: NSObject, NSTextViewDelegate { newPanel.contentView = scrollView newPanel.contentView?.wantsLayer = true newPanel.contentView?.layer?.borderWidth = 2 - newPanel.contentView?.layer?.borderColor = NSColor.selectedControlColor.safeCGColor + newPanel.contentView?.layer?.borderColor = NSColor.keyboardFocusIndicatorColor.safeCGColor newPanel.contentView?.layer?.cornerRadius = 2 newPanel.contentView?.layer?.masksToBounds = true diff --git a/TablePro/Views/Results/DataGridCellFactory.swift b/TablePro/Views/Results/DataGridCellFactory.swift index f34e6deb6..912f8ce44 100644 --- a/TablePro/Views/Results/DataGridCellFactory.swift +++ b/TablePro/Views/Results/DataGridCellFactory.swift @@ -259,14 +259,8 @@ final class DataGridCellFactory { gridCellView.changeBackgroundColor = nil } - if isLargeDataset { - gridCellView.layer?.borderWidth = 0 - } else if isFocused { - gridCellView.layer?.borderWidth = 2 - gridCellView.layer?.borderColor = ThemeEngine.shared.colors.dataGrid.focusBorderCG - } else { - gridCellView.layer?.borderWidth = 0 - } + gridCellView.layer?.borderWidth = 0 + gridCellView.isFocusedCell = isFocused CATransaction.commit() diff --git a/TablePro/Views/Results/DataGridCellView.swift b/TablePro/Views/Results/DataGridCellView.swift index 8334c8235..fe1e6c338 100644 --- a/TablePro/Views/Results/DataGridCellView.swift +++ b/TablePro/Views/Results/DataGridCellView.swift @@ -5,15 +5,18 @@ import AppKit -/// Custom cell view that uses a background subview for change-state coloring. -/// AppKit's `NSTableRowView` sets `backgroundStyle` to `.emphasized` when the -/// row is selected — we hide the background view so the native selection highlight -/// shows through. final class DataGridCellView: NSTableCellView { var fkArrowButton: FKArrowButton? var chevronButton: CellChevronButton? var textFieldTrailing: NSLayoutConstraint? + var isFocusedCell: Bool = false { + didSet { + guard oldValue != isFocusedCell else { return } + noteFocusRingMaskChanged() + } + } + private lazy var backgroundView: NSView = { let view = NSView() view.wantsLayer = true @@ -45,4 +48,13 @@ final class DataGridCellView: NSTableCellView { backgroundView.isHidden = (backgroundStyle == .emphasized) || (changeBackgroundColor == nil) } } + + override var focusRingMaskBounds: NSRect { + isFocusedCell ? bounds : .zero + } + + override func drawFocusRingMask() { + guard isFocusedCell else { return } + bounds.fill() + } } From a8867e7dc42e8ab61cc0f82b66f449173c8a35ff Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 28 Apr 2026 03:18:33 +0700 Subject: [PATCH 2/8] fix(a11y): always set cell labels and row/column index ranges --- .../Views/Results/DataGridCellFactory.swift | 40 ++++--------------- 1 file changed, 8 insertions(+), 32 deletions(-) diff --git a/TablePro/Views/Results/DataGridCellFactory.swift b/TablePro/Views/Results/DataGridCellFactory.swift index 912f8ce44..81337884f 100644 --- a/TablePro/Views/Results/DataGridCellFactory.swift +++ b/TablePro/Views/Results/DataGridCellFactory.swift @@ -23,40 +23,17 @@ final class CellChevronButton: NSButton { var cellColumnIndex: Int = -1 } -/// Factory for creating data grid cell views @MainActor final class DataGridCellFactory { private let cellIdentifier = NSUserInterfaceItemIdentifier("DataCell") private let rowNumberCellIdentifier = NSUserInterfaceItemIdentifier("RowNumberCell") - /// Large dataset threshold - above this, disable expensive visual features private let largeDatasetThreshold = 5_000 - // MARK: - Cached Settings - - /// Cached NULL display string (updated via settings notification) private var nullDisplayString: String = AppSettingsManager.shared.dataGrid.nullDisplay private var settingsObserver: NSObjectProtocol? - // MARK: - Cached VoiceOver State - - private static var cachedVoiceOverEnabled: Bool = NSWorkspace.shared.isVoiceOverEnabled - // Observer lives for app lifetime — no removal needed since DataGridCellFactory is a static singleton cache - private static let voiceOverObserver: NSObjectProtocol? = { - NotificationCenter.default.addObserver( - forName: NSWorkspace.accessibilityDisplayOptionsDidChangeNotification, - object: nil, - queue: .main - ) { _ in - Task { @MainActor in - DataGridCellFactory.cachedVoiceOverEnabled = NSWorkspace.shared.isVoiceOverEnabled - } - } - }() - init() { - _ = Self.voiceOverObserver - settingsObserver = NotificationCenter.default.addObserver( forName: .dataGridSettingsDidChange, object: nil, @@ -119,9 +96,8 @@ final class DataGridCellFactory { cell.stringValue = "\(row + 1)" cell.textColor = visualState.isDeleted ? ThemeEngine.shared.colors.dataGrid.deletedText : .secondaryLabelColor - if Self.cachedVoiceOverEnabled { - cellView.setAccessibilityLabel(String(format: String(localized: "Row %d"), row + 1)) - } + cellView.setAccessibilityLabel(String(format: String(localized: "Row %d"), row + 1)) + cellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1)) return cellView } @@ -264,12 +240,12 @@ final class DataGridCellFactory { CATransaction.commit() - if Self.cachedVoiceOverEnabled { - let accessibilityValue = rawValue ?? String(localized: "NULL") - cell.setAccessibilityLabel( - String(format: String(localized: "Row %d, column %d: %@"), row + 1, columnIndex + 1, accessibilityValue) - ) - } + let accessibilityValue = rawValue ?? String(localized: "NULL") + cell.setAccessibilityLabel( + String(format: String(localized: "Row %d, column %d: %@"), row + 1, columnIndex + 1, accessibilityValue) + ) + gridCellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1)) + gridCellView.setAccessibilityColumnIndexRange(NSRange(location: columnIndex, length: 1)) return gridCellView } From fcbd8acab4101274daad6f85970e421df5a00e6b Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 28 Apr 2026 03:19:12 +0700 Subject: [PATCH 3/8] refactor(datagrid): use themed font size for inline date picker --- TablePro/Views/Results/DatePickerCellEditor.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/TablePro/Views/Results/DatePickerCellEditor.swift b/TablePro/Views/Results/DatePickerCellEditor.swift index 6e6d0a62e..b8e2177a9 100644 --- a/TablePro/Views/Results/DatePickerCellEditor.swift +++ b/TablePro/Views/Results/DatePickerCellEditor.swift @@ -71,7 +71,8 @@ final class DatePickerCellEditor: NSDatePicker { private func setupUI() { datePickerStyle = .textFieldAndStepper datePickerElements = [.yearMonthDay, .hourMinuteSecond] - font = .monospacedSystemFont(ofSize: 13, weight: .regular) + let pointSize = ThemeEngine.shared.dataGridFonts.regular.pointSize + font = .monospacedSystemFont(ofSize: pointSize, weight: .regular) isBezeled = false isBordered = false drawsBackground = false From 2ce6ced1720fc90f6eb6415902bfcbab95f91d4c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 28 Apr 2026 03:22:22 +0700 Subject: [PATCH 4/8] feat(datagrid): render boolean columns as native NSButton checkboxes --- TablePro/Views/Results/BooleanCellView.swift | 41 +++++++ .../Views/Results/BooleanValueMapper.swift | 34 ++++++ .../Views/Results/DataGridCellFactory.swift | 66 +++++++++++ TablePro/Views/Results/DataGridCellView.swift | 2 +- .../Extensions/DataGridView+Click.swift | 20 ++++ .../Extensions/DataGridView+Columns.swift | 23 +++- .../Views/Results/TableRowViewWithMenu.swift | 39 +++++-- .../Results/BooleanValueMapperTests.swift | 105 ++++++++++++++++++ 8 files changed, 314 insertions(+), 16 deletions(-) create mode 100644 TablePro/Views/Results/BooleanCellView.swift create mode 100644 TablePro/Views/Results/BooleanValueMapper.swift create mode 100644 TableProTests/Views/Results/BooleanValueMapperTests.swift diff --git a/TablePro/Views/Results/BooleanCellView.swift b/TablePro/Views/Results/BooleanCellView.swift new file mode 100644 index 000000000..37ff1d891 --- /dev/null +++ b/TablePro/Views/Results/BooleanCellView.swift @@ -0,0 +1,41 @@ +// +// BooleanCellView.swift +// TablePro +// + +import AppKit + +@MainActor +final class BooleanCellView: DataGridCellView { + var cellRow: Int = -1 + var cellColumnIndex: Int = -1 + + private(set) lazy var checkbox: NSButton = { + let button = NSButton() + button.setButtonType(.switch) + button.allowsMixedState = true + button.title = "" + button.imagePosition = .imageOnly + button.translatesAutoresizingMaskIntoConstraints = false + button.focusRingType = .none + return button + }() + + override init(frame frameRect: NSRect) { + super.init(frame: frameRect) + installCheckbox() + } + + required init?(coder: NSCoder) { + super.init(coder: coder) + installCheckbox() + } + + private func installCheckbox() { + addSubview(checkbox) + NSLayoutConstraint.activate([ + checkbox.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 4), + checkbox.centerYAnchor.constraint(equalTo: centerYAnchor), + ]) + } +} diff --git a/TablePro/Views/Results/BooleanValueMapper.swift b/TablePro/Views/Results/BooleanValueMapper.swift new file mode 100644 index 000000000..98ccd5e08 --- /dev/null +++ b/TablePro/Views/Results/BooleanValueMapper.swift @@ -0,0 +1,34 @@ +// +// BooleanValueMapper.swift +// TablePro +// + +import AppKit + +enum BooleanValueMapper { + static func state(for rawValue: String?) -> NSControl.StateValue { + guard let rawValue else { return .mixed } + let trimmed = rawValue.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return .mixed } + + switch trimmed.lowercased() { + case "1", "true", "t", "yes", "y", "on": + return .on + case "0", "false", "f", "no", "n", "off": + return .off + default: + return .mixed + } + } + + static func rawValue(for state: NSControl.StateValue, usesTrueFalse: Bool) -> String? { + switch state { + case .on: + return usesTrueFalse ? "true" : "1" + case .off: + return usesTrueFalse ? "false" : "0" + default: + return nil + } + } +} diff --git a/TablePro/Views/Results/DataGridCellFactory.swift b/TablePro/Views/Results/DataGridCellFactory.swift index 81337884f..899040003 100644 --- a/TablePro/Views/Results/DataGridCellFactory.swift +++ b/TablePro/Views/Results/DataGridCellFactory.swift @@ -27,6 +27,7 @@ final class CellChevronButton: NSButton { final class DataGridCellFactory { private let cellIdentifier = NSUserInterfaceItemIdentifier("DataCell") private let rowNumberCellIdentifier = NSUserInterfaceItemIdentifier("RowNumberCell") + private let booleanCellIdentifier = NSUserInterfaceItemIdentifier("BooleanCell") private let largeDatasetThreshold = 5_000 @@ -250,6 +251,71 @@ final class DataGridCellFactory { return gridCellView } + // MARK: - Boolean Cell + + func makeBooleanCell( + tableView: NSTableView, + row: Int, + columnIndex: Int, + rawValue: String?, + visualState: RowVisualState, + isEditable: Bool, + isFocused: Bool, + target: AnyObject?, + action: Selector? + ) -> NSView { + let cellView: BooleanCellView + if let reused = tableView.makeView(withIdentifier: booleanCellIdentifier, owner: nil) as? BooleanCellView { + cellView = reused + } else { + cellView = BooleanCellView() + cellView.identifier = booleanCellIdentifier + cellView.wantsLayer = true + } + + cellView.cellRow = row + cellView.cellColumnIndex = columnIndex + + let checkbox = cellView.checkbox + checkbox.target = target + checkbox.action = action + checkbox.isEnabled = isEditable && !visualState.isDeleted + checkbox.state = BooleanValueMapper.state(for: rawValue) + + let isModified = visualState.modifiedColumns.contains(columnIndex) + CATransaction.begin() + CATransaction.setDisableActions(true) + if visualState.isDeleted { + cellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.deleted + } else if visualState.isInserted { + cellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.inserted + } else if isModified { + cellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.modified + } else { + cellView.changeBackgroundColor = nil + } + cellView.layer?.borderWidth = 0 + cellView.isFocusedCell = isFocused + CATransaction.commit() + + let accessibilityValue: String + switch checkbox.state { + case .on: + accessibilityValue = String(localized: "true") + case .off: + accessibilityValue = String(localized: "false") + default: + accessibilityValue = String(localized: "NULL") + } + cellView.setAccessibilityLabel( + String(format: String(localized: "Row %d, column %d: %@"), row + 1, columnIndex + 1, accessibilityValue) + ) + cellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1)) + cellView.setAccessibilityColumnIndexRange(NSRange(location: columnIndex, length: 1)) + + return cellView + } + // MARK: - Button Creation private func createFKArrowButton() -> FKArrowButton { diff --git a/TablePro/Views/Results/DataGridCellView.swift b/TablePro/Views/Results/DataGridCellView.swift index fe1e6c338..a07ae4686 100644 --- a/TablePro/Views/Results/DataGridCellView.swift +++ b/TablePro/Views/Results/DataGridCellView.swift @@ -5,7 +5,7 @@ import AppKit -final class DataGridCellView: NSTableCellView { +class DataGridCellView: NSTableCellView { var fkArrowButton: FKArrowButton? var chevronButton: CellChevronButton? var textFieldTrailing: NSLayoutConstraint? diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index df15970ec..dd01a07a6 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -127,6 +127,26 @@ extension TableViewCoordinator { } } + // MARK: - Boolean Cell Toggle + + @objc func handleBooleanCellToggle(_ sender: NSButton) { + guard isEditable else { return } + var current: NSView? = sender.superview + while let view = current, !(view is BooleanCellView) { + current = view.superview + } + guard let cellView = current as? BooleanCellView else { return } + + let row = cellView.cellRow + let columnIndex = cellView.cellColumnIndex + guard row >= 0, columnIndex >= 0, columnIndex < rowProvider.columns.count else { return } + guard !changeManager.isRowDeleted(row) else { return } + + let usesTrueFalse = databaseType.map { PluginManager.shared.usesTrueFalseBooleans(for: $0) } ?? false + let newValue = BooleanValueMapper.rawValue(for: sender.state, usesTrueFalse: usesTrueFalse) + commitCellEdit(row: row, columnIndex: columnIndex, newValue: newValue) + } + // MARK: - FK Navigation @objc func handleFKArrowClick(_ sender: NSButton) { diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index f776041f1..0728bc895 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -46,10 +46,27 @@ extension TableViewCoordinator { let isEnumOrSet = enumOrSetColumns.contains(columnIndex) let isFKColumn = fkColumns.contains(columnIndex) + let columnType: ColumnType? = columnIndex < rowProvider.columnTypes.count + ? rowProvider.columnTypes[columnIndex] + : nil + + if let columnType, columnType.isBooleanType, !isDropdown, !isTypePicker { + return cellFactory.makeBooleanCell( + tableView: tableView, + row: row, + columnIndex: columnIndex, + rawValue: rawValue, + visualState: state, + isEditable: isEditable, + isFocused: isFocused, + target: self, + action: #selector(handleBooleanCellToggle(_:)) + ) + } + let hasSpecialEditor: Bool = { - guard columnIndex < rowProvider.columnTypes.count else { return false } - let ct = rowProvider.columnTypes[columnIndex] - return ct.isBooleanType || ct.isDateType || ct.isJsonType || ct.isBlobType + guard let columnType else { return false } + return columnType.isDateType || columnType.isJsonType || columnType.isBlobType }() return cellFactory.makeDataCell( diff --git a/TablePro/Views/Results/TableRowViewWithMenu.swift b/TablePro/Views/Results/TableRowViewWithMenu.swift index c703a8c9c..e2ef0b848 100644 --- a/TablePro/Views/Results/TableRowViewWithMenu.swift +++ b/TablePro/Views/Results/TableRowViewWithMenu.swift @@ -130,7 +130,6 @@ final class TableRowViewWithMenu: NSTableRowView { menu.addItem(NSMenuItem.separator()) } - // Set Value (editable + column clicked) if coordinator.isEditable && dataColumnIndex >= 0 { let setValueMenu = NSMenu() @@ -140,17 +139,33 @@ final class TableRowViewWithMenu: NSTableRowView { emptyItem.target = self setValueMenu.addItem(emptyItem) - let nullItem = NSMenuItem( - title: String(localized: "NULL"), action: #selector(setNullValue(_:)), keyEquivalent: "") - nullItem.representedObject = dataColumnIndex - nullItem.target = self - setValueMenu.addItem(nullItem) - - let defaultItem = NSMenuItem( - title: String(localized: "Default"), action: #selector(setDefaultValue(_:)), keyEquivalent: "") - defaultItem.representedObject = dataColumnIndex - defaultItem.target = self - setValueMenu.addItem(defaultItem) + let columnName = dataColumnIndex < coordinator.rowProvider.columns.count + ? coordinator.rowProvider.columns[dataColumnIndex] + : nil + + let isNullable = columnName.flatMap { coordinator.rowProvider.columnNullable[$0] } ?? true + if isNullable { + let nullItem = NSMenuItem( + title: String(localized: "NULL"), action: #selector(setNullValue(_:)), keyEquivalent: "") + nullItem.representedObject = dataColumnIndex + nullItem.target = self + setValueMenu.addItem(nullItem) + } + + let hasDefault: Bool = { + guard let columnName else { return false } + if let value = coordinator.rowProvider.columnDefaults[columnName], value != nil { + return true + } + return false + }() + if hasDefault { + let defaultItem = NSMenuItem( + title: String(localized: "Default"), action: #selector(setDefaultValue(_:)), keyEquivalent: "") + defaultItem.representedObject = dataColumnIndex + defaultItem.target = self + setValueMenu.addItem(defaultItem) + } let setValueItem = NSMenuItem(title: String(localized: "Set Value"), action: nil, keyEquivalent: "") setValueItem.submenu = setValueMenu diff --git a/TableProTests/Views/Results/BooleanValueMapperTests.swift b/TableProTests/Views/Results/BooleanValueMapperTests.swift new file mode 100644 index 000000000..7e7bcfa24 --- /dev/null +++ b/TableProTests/Views/Results/BooleanValueMapperTests.swift @@ -0,0 +1,105 @@ +// +// BooleanValueMapperTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +@Suite("BooleanValueMapper.state(for:)") +struct BooleanValueMapperStateTests { + @Test("nil maps to mixed") + func nilMapsToMixed() { + #expect(BooleanValueMapper.state(for: nil) == .mixed) + } + + @Test("Empty and whitespace map to mixed") + func emptyMapsToMixed() { + #expect(BooleanValueMapper.state(for: "") == .mixed) + #expect(BooleanValueMapper.state(for: " ") == .mixed) + } + + @Test("\"1\" maps to on") + func oneIsOn() { + #expect(BooleanValueMapper.state(for: "1") == .on) + } + + @Test("\"0\" maps to off") + func zeroIsOff() { + #expect(BooleanValueMapper.state(for: "0") == .off) + } + + @Test("Lowercase true/false") + func lowercaseLiterals() { + #expect(BooleanValueMapper.state(for: "true") == .on) + #expect(BooleanValueMapper.state(for: "false") == .off) + } + + @Test("Uppercase TRUE/FALSE") + func uppercaseLiterals() { + #expect(BooleanValueMapper.state(for: "TRUE") == .on) + #expect(BooleanValueMapper.state(for: "FALSE") == .off) + #expect(BooleanValueMapper.state(for: "True") == .on) + } + + @Test("Postgres single-letter t/f") + func singleLetterLiterals() { + #expect(BooleanValueMapper.state(for: "t") == .on) + #expect(BooleanValueMapper.state(for: "f") == .off) + #expect(BooleanValueMapper.state(for: "T") == .on) + #expect(BooleanValueMapper.state(for: "F") == .off) + } + + @Test("Garbage maps to mixed") + func garbageMapsToMixed() { + #expect(BooleanValueMapper.state(for: "maybe") == .mixed) + #expect(BooleanValueMapper.state(for: "2") == .mixed) + #expect(BooleanValueMapper.state(for: "null") == .mixed) + } +} + +@Suite("BooleanValueMapper.rawValue(for:usesTrueFalse:)") +struct BooleanValueMapperRawValueTests { + @Test("On round-trips with usesTrueFalse=false") + func onIntegerStyle() { + #expect(BooleanValueMapper.rawValue(for: .on, usesTrueFalse: false) == "1") + } + + @Test("Off round-trips with usesTrueFalse=false") + func offIntegerStyle() { + #expect(BooleanValueMapper.rawValue(for: .off, usesTrueFalse: false) == "0") + } + + @Test("On round-trips with usesTrueFalse=true") + func onLiteralStyle() { + #expect(BooleanValueMapper.rawValue(for: .on, usesTrueFalse: true) == "true") + } + + @Test("Off round-trips with usesTrueFalse=true") + func offLiteralStyle() { + #expect(BooleanValueMapper.rawValue(for: .off, usesTrueFalse: true) == "false") + } + + @Test("Mixed maps to nil regardless of style") + func mixedIsNull() { + #expect(BooleanValueMapper.rawValue(for: .mixed, usesTrueFalse: false) == nil) + #expect(BooleanValueMapper.rawValue(for: .mixed, usesTrueFalse: true) == nil) + } + + @Test("Round-trip integer style") + func roundTripInteger() { + let onState = BooleanValueMapper.state(for: BooleanValueMapper.rawValue(for: .on, usesTrueFalse: false)) + let offState = BooleanValueMapper.state(for: BooleanValueMapper.rawValue(for: .off, usesTrueFalse: false)) + #expect(onState == .on) + #expect(offState == .off) + } + + @Test("Round-trip literal style") + func roundTripLiteral() { + let onState = BooleanValueMapper.state(for: BooleanValueMapper.rawValue(for: .on, usesTrueFalse: true)) + let offState = BooleanValueMapper.state(for: BooleanValueMapper.rawValue(for: .off, usesTrueFalse: true)) + #expect(onState == .on) + #expect(offState == .off) + } +} From ec3cc8c3960d9d01e785a3dd2b4c1970b3302000 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 28 Apr 2026 03:24:39 +0700 Subject: [PATCH 5/8] feat(datagrid): single-click on focused cell starts inline edit --- CHANGELOG.md | 5 +++ .../Extensions/DataGridView+Editing.swift | 32 +++++++++++++++++++ .../Views/Results/KeyHandlingTableView.swift | 17 +++++++--- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 637662915..2f8cc86ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Connection URL parsing: SSH `user:password@host` split, `safeModeLevel` from TablePlus URLs, case-insensitive query params - Connection URL export: SSH password, Redis database index, MongoDB auth params (`authSource`, `authMechanism`, `replicaSet`), and multi-host - 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 +- Click a focused cell to start editing without a second click +- Boolean columns render as native checkboxes; NULL shows as mixed state. Right-click "Set Value > NULL" sets the cell back to NULL on nullable columns. +- Data grid focus ring follows the system accent color and contrast settings +- Data grid cells expose accessibility row and column index ranges to VoiceOver on all dataset sizes ### Changed @@ -30,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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. - 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. - 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. +- Date picker popover font follows the data grid font setting ### Fixed diff --git a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift index 0e21d41f4..64bec4e4c 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift @@ -7,6 +7,38 @@ import AppKit import SwiftUI extension TableViewCoordinator { + func canStartInlineEdit(row: Int, columnIndex: Int) -> Bool { + guard isEditable else { return false } + guard row >= 0, columnIndex >= 0, columnIndex < rowProvider.columns.count else { return false } + guard !changeManager.isRowDeleted(row) else { return false } + + let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? [] + if immutable.contains(rowProvider.columns[columnIndex]) { + return false + } + + let columnName = rowProvider.columns[columnIndex] + if rowProvider.columnForeignKeys[columnName] != nil { return false } + + if columnIndex < rowProvider.columnTypes.count { + let ct = rowProvider.columnTypes[columnIndex] + if ct.isBooleanType || ct.isDateType || ct.isJsonType + || ct.isBlobType || ct.isEnumType || ct.isSetType { + return false + } + } + + if dropdownColumns?.contains(columnIndex) == true { return false } + if typePickerColumns?.contains(columnIndex) == true { return false } + + if let value = rowProvider.value(atRow: row, column: columnIndex) { + if value.containsLineBreak { return false } + if value.looksLikeJson { return false } + } + + return true + } + func tableView(_ tableView: NSTableView, shouldEdit tableColumn: NSTableColumn?, row: Int) -> Bool { guard isEditable, let tableColumn = tableColumn else { return false } diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index 73f66b600..b61d17679 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -59,7 +59,6 @@ final class KeyHandlingTableView: NSTableView { // MARK: - TablePlus-Style Cell Focus override func mouseDown(with event: NSEvent) { - // Become first responder to capture keyboard events (especially Delete key) window?.makeFirstResponder(self) let point = convert(event.locationInWindow, from: nil) @@ -71,22 +70,24 @@ final class KeyHandlingTableView: NSTableView { return } - // Reset anchor/pivot when clicking without Shift if clickedRow >= 0 && !event.modifierFlags.contains(.shift) { selectionAnchor = clickedRow selectionPivot = clickedRow } + let alreadyFocusedHere = clickedRow >= 0 + && clickedColumn >= 0 + && clickedRow == focusedRow + && clickedColumn == focusedColumn + super.mouseDown(with: event) - // Only handle editing for valid clicks on data cells (not row number column) guard clickedRow >= 0, clickedColumn >= 0, clickedColumn < numberOfColumns else { return } - // Skip row number column let column = tableColumns[clickedColumn] if column.identifier.rawValue == "__rowNumber__" { focusedRow = -1 @@ -94,9 +95,15 @@ final class KeyHandlingTableView: NSTableView { return } - // Update focus (edit mode is triggered by double-click, not single click) focusedRow = clickedRow focusedColumn = clickedColumn + + if alreadyFocusedHere && event.clickCount == 1 { + let dataColumnIndex = DataGridView.dataColumnIndex(for: clickedColumn) + if coordinator?.canStartInlineEdit(row: clickedRow, columnIndex: dataColumnIndex) == true { + editColumn(clickedColumn, row: clickedRow, with: nil, select: true) + } + } } // MARK: - Standard Edit Menu Actions From ab3293a0a13fc24dca96c25ca50ffcdba483ae9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Tue, 28 Apr 2026 09:15:45 +0700 Subject: [PATCH 6/8] refactor(datagrid): revert boolean checkbox to text+dropdown, deduplicate edit eligibility --- CHANGELOG.md | 1 - TablePro/Views/Results/BooleanCellView.swift | 41 ------- .../Views/Results/BooleanValueMapper.swift | 34 ------ .../Views/Results/DataGridCellFactory.swift | 67 ----------- .../Extensions/DataGridView+Click.swift | 20 ---- .../Extensions/DataGridView+Columns.swift | 23 +--- .../Extensions/DataGridView+Editing.swift | 96 ++++++---------- .../Views/Results/TableRowViewWithMenu.swift | 8 +- .../Results/BooleanValueMapperTests.swift | 105 ------------------ 9 files changed, 41 insertions(+), 354 deletions(-) delete mode 100644 TablePro/Views/Results/BooleanCellView.swift delete mode 100644 TablePro/Views/Results/BooleanValueMapper.swift delete mode 100644 TableProTests/Views/Results/BooleanValueMapperTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f8cc86ca..5b5acfbec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Connection URL export: SSH password, Redis database index, MongoDB auth params (`authSource`, `authMechanism`, `replicaSet`), and multi-host - 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 - Click a focused cell to start editing without a second click -- Boolean columns render as native checkboxes; NULL shows as mixed state. Right-click "Set Value > NULL" sets the cell back to NULL on nullable columns. - Data grid focus ring follows the system accent color and contrast settings - Data grid cells expose accessibility row and column index ranges to VoiceOver on all dataset sizes diff --git a/TablePro/Views/Results/BooleanCellView.swift b/TablePro/Views/Results/BooleanCellView.swift deleted file mode 100644 index 37ff1d891..000000000 --- a/TablePro/Views/Results/BooleanCellView.swift +++ /dev/null @@ -1,41 +0,0 @@ -// -// BooleanCellView.swift -// TablePro -// - -import AppKit - -@MainActor -final class BooleanCellView: DataGridCellView { - var cellRow: Int = -1 - var cellColumnIndex: Int = -1 - - private(set) lazy var checkbox: NSButton = { - let button = NSButton() - button.setButtonType(.switch) - button.allowsMixedState = true - button.title = "" - button.imagePosition = .imageOnly - button.translatesAutoresizingMaskIntoConstraints = false - button.focusRingType = .none - return button - }() - - override init(frame frameRect: NSRect) { - super.init(frame: frameRect) - installCheckbox() - } - - required init?(coder: NSCoder) { - super.init(coder: coder) - installCheckbox() - } - - private func installCheckbox() { - addSubview(checkbox) - NSLayoutConstraint.activate([ - checkbox.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 4), - checkbox.centerYAnchor.constraint(equalTo: centerYAnchor), - ]) - } -} diff --git a/TablePro/Views/Results/BooleanValueMapper.swift b/TablePro/Views/Results/BooleanValueMapper.swift deleted file mode 100644 index 98ccd5e08..000000000 --- a/TablePro/Views/Results/BooleanValueMapper.swift +++ /dev/null @@ -1,34 +0,0 @@ -// -// BooleanValueMapper.swift -// TablePro -// - -import AppKit - -enum BooleanValueMapper { - static func state(for rawValue: String?) -> NSControl.StateValue { - guard let rawValue else { return .mixed } - let trimmed = rawValue.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty else { return .mixed } - - switch trimmed.lowercased() { - case "1", "true", "t", "yes", "y", "on": - return .on - case "0", "false", "f", "no", "n", "off": - return .off - default: - return .mixed - } - } - - static func rawValue(for state: NSControl.StateValue, usesTrueFalse: Bool) -> String? { - switch state { - case .on: - return usesTrueFalse ? "true" : "1" - case .off: - return usesTrueFalse ? "false" : "0" - default: - return nil - } - } -} diff --git a/TablePro/Views/Results/DataGridCellFactory.swift b/TablePro/Views/Results/DataGridCellFactory.swift index 899040003..2c78ff376 100644 --- a/TablePro/Views/Results/DataGridCellFactory.swift +++ b/TablePro/Views/Results/DataGridCellFactory.swift @@ -27,8 +27,6 @@ final class CellChevronButton: NSButton { final class DataGridCellFactory { private let cellIdentifier = NSUserInterfaceItemIdentifier("DataCell") private let rowNumberCellIdentifier = NSUserInterfaceItemIdentifier("RowNumberCell") - private let booleanCellIdentifier = NSUserInterfaceItemIdentifier("BooleanCell") - private let largeDatasetThreshold = 5_000 private var nullDisplayString: String = AppSettingsManager.shared.dataGrid.nullDisplay @@ -251,71 +249,6 @@ final class DataGridCellFactory { return gridCellView } - // MARK: - Boolean Cell - - func makeBooleanCell( - tableView: NSTableView, - row: Int, - columnIndex: Int, - rawValue: String?, - visualState: RowVisualState, - isEditable: Bool, - isFocused: Bool, - target: AnyObject?, - action: Selector? - ) -> NSView { - let cellView: BooleanCellView - if let reused = tableView.makeView(withIdentifier: booleanCellIdentifier, owner: nil) as? BooleanCellView { - cellView = reused - } else { - cellView = BooleanCellView() - cellView.identifier = booleanCellIdentifier - cellView.wantsLayer = true - } - - cellView.cellRow = row - cellView.cellColumnIndex = columnIndex - - let checkbox = cellView.checkbox - checkbox.target = target - checkbox.action = action - checkbox.isEnabled = isEditable && !visualState.isDeleted - checkbox.state = BooleanValueMapper.state(for: rawValue) - - let isModified = visualState.modifiedColumns.contains(columnIndex) - CATransaction.begin() - CATransaction.setDisableActions(true) - if visualState.isDeleted { - cellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.deleted - } else if visualState.isInserted { - cellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.inserted - } else if isModified { - cellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.modified - } else { - cellView.changeBackgroundColor = nil - } - cellView.layer?.borderWidth = 0 - cellView.isFocusedCell = isFocused - CATransaction.commit() - - let accessibilityValue: String - switch checkbox.state { - case .on: - accessibilityValue = String(localized: "true") - case .off: - accessibilityValue = String(localized: "false") - default: - accessibilityValue = String(localized: "NULL") - } - cellView.setAccessibilityLabel( - String(format: String(localized: "Row %d, column %d: %@"), row + 1, columnIndex + 1, accessibilityValue) - ) - cellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1)) - cellView.setAccessibilityColumnIndexRange(NSRange(location: columnIndex, length: 1)) - - return cellView - } - // MARK: - Button Creation private func createFKArrowButton() -> FKArrowButton { diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index dd01a07a6..df15970ec 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -127,26 +127,6 @@ extension TableViewCoordinator { } } - // MARK: - Boolean Cell Toggle - - @objc func handleBooleanCellToggle(_ sender: NSButton) { - guard isEditable else { return } - var current: NSView? = sender.superview - while let view = current, !(view is BooleanCellView) { - current = view.superview - } - guard let cellView = current as? BooleanCellView else { return } - - let row = cellView.cellRow - let columnIndex = cellView.cellColumnIndex - guard row >= 0, columnIndex >= 0, columnIndex < rowProvider.columns.count else { return } - guard !changeManager.isRowDeleted(row) else { return } - - let usesTrueFalse = databaseType.map { PluginManager.shared.usesTrueFalseBooleans(for: $0) } ?? false - let newValue = BooleanValueMapper.rawValue(for: sender.state, usesTrueFalse: usesTrueFalse) - commitCellEdit(row: row, columnIndex: columnIndex, newValue: newValue) - } - // MARK: - FK Navigation @objc func handleFKArrowClick(_ sender: NSButton) { diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index 0728bc895..f776041f1 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -46,27 +46,10 @@ extension TableViewCoordinator { let isEnumOrSet = enumOrSetColumns.contains(columnIndex) let isFKColumn = fkColumns.contains(columnIndex) - let columnType: ColumnType? = columnIndex < rowProvider.columnTypes.count - ? rowProvider.columnTypes[columnIndex] - : nil - - if let columnType, columnType.isBooleanType, !isDropdown, !isTypePicker { - return cellFactory.makeBooleanCell( - tableView: tableView, - row: row, - columnIndex: columnIndex, - rawValue: rawValue, - visualState: state, - isEditable: isEditable, - isFocused: isFocused, - target: self, - action: #selector(handleBooleanCellToggle(_:)) - ) - } - let hasSpecialEditor: Bool = { - guard let columnType else { return false } - return columnType.isDateType || columnType.isJsonType || columnType.isBlobType + guard columnIndex < rowProvider.columnTypes.count else { return false } + let ct = rowProvider.columnTypes[columnIndex] + return ct.isBooleanType || ct.isDateType || ct.isJsonType || ct.isBlobType }() return cellFactory.makeDataCell( diff --git a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift index 64bec4e4c..e2631a84f 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift @@ -7,89 +7,67 @@ import AppKit import SwiftUI extension TableViewCoordinator { - func canStartInlineEdit(row: Int, columnIndex: Int) -> Bool { - guard isEditable else { return false } - guard row >= 0, columnIndex >= 0, columnIndex < rowProvider.columns.count else { return false } - guard !changeManager.isRowDeleted(row) else { return false } + enum InlineEditEligibility { + case eligible + case needsOverlayEditor(value: String) + case blocked + } + + func inlineEditEligibility(row: Int, columnIndex: Int) -> InlineEditEligibility { + guard isEditable else { return .blocked } + guard row >= 0, columnIndex >= 0, columnIndex < rowProvider.columns.count else { return .blocked } + guard !changeManager.isRowDeleted(row) else { return .blocked } let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? [] if immutable.contains(rowProvider.columns[columnIndex]) { - return false + return .blocked } let columnName = rowProvider.columns[columnIndex] - if rowProvider.columnForeignKeys[columnName] != nil { return false } + if rowProvider.columnForeignKeys[columnName] != nil { return .blocked } if columnIndex < rowProvider.columnTypes.count { let ct = rowProvider.columnTypes[columnIndex] if ct.isBooleanType || ct.isDateType || ct.isJsonType || ct.isBlobType || ct.isEnumType || ct.isSetType { - return false + return .blocked } } - if dropdownColumns?.contains(columnIndex) == true { return false } - if typePickerColumns?.contains(columnIndex) == true { return false } + if dropdownColumns?.contains(columnIndex) == true { return .blocked } + if typePickerColumns?.contains(columnIndex) == true { return .blocked } if let value = rowProvider.value(atRow: row, column: columnIndex) { - if value.containsLineBreak { return false } - if value.looksLikeJson { return false } + if value.containsLineBreak { return .needsOverlayEditor(value: value) } + if value.looksLikeJson { return .blocked } } - return true + return .eligible } - func tableView(_ tableView: NSTableView, shouldEdit tableColumn: NSTableColumn?, row: Int) -> Bool { - guard isEditable, - let tableColumn = tableColumn else { return false } - - let columnId = tableColumn.identifier.rawValue - guard columnId != "__rowNumber__", - !changeManager.isRowDeleted(row) else { return false } - - let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? [] - if !immutable.isEmpty, - let columnIndex = DataGridView.dataColumnIndex(from: tableColumn.identifier), - columnIndex < rowProvider.columns.count, - immutable.contains(rowProvider.columns[columnIndex]) { - return false + func canStartInlineEdit(row: Int, columnIndex: Int) -> Bool { + if case .eligible = inlineEditEligibility(row: row, columnIndex: columnIndex) { + return true } + return false + } - // Popover-editor columns (date/FK/JSON) are only editable via - // double-click (handleDoubleClick). Block inline editing for them. - if let columnIndex = DataGridView.dataColumnIndex(from: tableColumn.identifier) { - if columnIndex < rowProvider.columns.count { - let columnName = rowProvider.columns[columnIndex] - if rowProvider.columnForeignKeys[columnName] != nil { return false } - } - if columnIndex < rowProvider.columnTypes.count { - let ct = rowProvider.columnTypes[columnIndex] - if ct.isDateType || ct.isJsonType || ct.isEnumType || ct.isSetType || ct.isBlobType || ct.isBooleanType { return false } - } - if let dropdownCols = dropdownColumns, dropdownCols.contains(columnIndex) { - return false - } - if let typePickerCols = typePickerColumns, typePickerCols.contains(columnIndex) { - return false - } - - // Text columns containing JSON use JSON editor popover - if let value = rowProvider.value(atRow: row, column: columnIndex), - value.looksLikeJson { - return false - } + func tableView(_ tableView: NSTableView, shouldEdit tableColumn: NSTableColumn?, row: Int) -> Bool { + guard let tableColumn else { return false } + guard tableColumn.identifier.rawValue != "__rowNumber__" else { return false } + guard let columnIndex = DataGridView.dataColumnIndex(from: tableColumn.identifier) else { return false } - // Multiline values use overlay editor — block inline field editor - if let value = rowProvider.value(atRow: row, column: columnIndex), - value.containsLineBreak { - let tableColumnIdx = tableView.column(withIdentifier: tableColumn.identifier) - guard tableColumnIdx >= 0 else { return false } - showOverlayEditor(tableView: tableView, row: row, column: tableColumnIdx, columnIndex: columnIndex, value: value) - return false - } + switch inlineEditEligibility(row: row, columnIndex: columnIndex) { + case .eligible: + return true + case .needsOverlayEditor(let value): + let tableColumnIdx = tableView.column(withIdentifier: tableColumn.identifier) + guard tableColumnIdx >= 0 else { return false } + showOverlayEditor(tableView: tableView, row: row, column: tableColumnIdx, columnIndex: columnIndex, value: value) + return false + case .blocked: + return false } - - return true } // MARK: - Overlay Editor (Multiline) diff --git a/TablePro/Views/Results/TableRowViewWithMenu.swift b/TablePro/Views/Results/TableRowViewWithMenu.swift index e2ef0b848..645d25e2d 100644 --- a/TablePro/Views/Results/TableRowViewWithMenu.swift +++ b/TablePro/Views/Results/TableRowViewWithMenu.swift @@ -152,13 +152,7 @@ final class TableRowViewWithMenu: NSTableRowView { setValueMenu.addItem(nullItem) } - let hasDefault: Bool = { - guard let columnName else { return false } - if let value = coordinator.rowProvider.columnDefaults[columnName], value != nil { - return true - } - return false - }() + let hasDefault = columnName.flatMap({ coordinator.rowProvider.columnDefaults[$0] ?? nil }) != nil if hasDefault { let defaultItem = NSMenuItem( title: String(localized: "Default"), action: #selector(setDefaultValue(_:)), keyEquivalent: "") diff --git a/TableProTests/Views/Results/BooleanValueMapperTests.swift b/TableProTests/Views/Results/BooleanValueMapperTests.swift deleted file mode 100644 index 7e7bcfa24..000000000 --- a/TableProTests/Views/Results/BooleanValueMapperTests.swift +++ /dev/null @@ -1,105 +0,0 @@ -// -// BooleanValueMapperTests.swift -// TableProTests -// - -import AppKit -@testable import TablePro -import Testing - -@Suite("BooleanValueMapper.state(for:)") -struct BooleanValueMapperStateTests { - @Test("nil maps to mixed") - func nilMapsToMixed() { - #expect(BooleanValueMapper.state(for: nil) == .mixed) - } - - @Test("Empty and whitespace map to mixed") - func emptyMapsToMixed() { - #expect(BooleanValueMapper.state(for: "") == .mixed) - #expect(BooleanValueMapper.state(for: " ") == .mixed) - } - - @Test("\"1\" maps to on") - func oneIsOn() { - #expect(BooleanValueMapper.state(for: "1") == .on) - } - - @Test("\"0\" maps to off") - func zeroIsOff() { - #expect(BooleanValueMapper.state(for: "0") == .off) - } - - @Test("Lowercase true/false") - func lowercaseLiterals() { - #expect(BooleanValueMapper.state(for: "true") == .on) - #expect(BooleanValueMapper.state(for: "false") == .off) - } - - @Test("Uppercase TRUE/FALSE") - func uppercaseLiterals() { - #expect(BooleanValueMapper.state(for: "TRUE") == .on) - #expect(BooleanValueMapper.state(for: "FALSE") == .off) - #expect(BooleanValueMapper.state(for: "True") == .on) - } - - @Test("Postgres single-letter t/f") - func singleLetterLiterals() { - #expect(BooleanValueMapper.state(for: "t") == .on) - #expect(BooleanValueMapper.state(for: "f") == .off) - #expect(BooleanValueMapper.state(for: "T") == .on) - #expect(BooleanValueMapper.state(for: "F") == .off) - } - - @Test("Garbage maps to mixed") - func garbageMapsToMixed() { - #expect(BooleanValueMapper.state(for: "maybe") == .mixed) - #expect(BooleanValueMapper.state(for: "2") == .mixed) - #expect(BooleanValueMapper.state(for: "null") == .mixed) - } -} - -@Suite("BooleanValueMapper.rawValue(for:usesTrueFalse:)") -struct BooleanValueMapperRawValueTests { - @Test("On round-trips with usesTrueFalse=false") - func onIntegerStyle() { - #expect(BooleanValueMapper.rawValue(for: .on, usesTrueFalse: false) == "1") - } - - @Test("Off round-trips with usesTrueFalse=false") - func offIntegerStyle() { - #expect(BooleanValueMapper.rawValue(for: .off, usesTrueFalse: false) == "0") - } - - @Test("On round-trips with usesTrueFalse=true") - func onLiteralStyle() { - #expect(BooleanValueMapper.rawValue(for: .on, usesTrueFalse: true) == "true") - } - - @Test("Off round-trips with usesTrueFalse=true") - func offLiteralStyle() { - #expect(BooleanValueMapper.rawValue(for: .off, usesTrueFalse: true) == "false") - } - - @Test("Mixed maps to nil regardless of style") - func mixedIsNull() { - #expect(BooleanValueMapper.rawValue(for: .mixed, usesTrueFalse: false) == nil) - #expect(BooleanValueMapper.rawValue(for: .mixed, usesTrueFalse: true) == nil) - } - - @Test("Round-trip integer style") - func roundTripInteger() { - let onState = BooleanValueMapper.state(for: BooleanValueMapper.rawValue(for: .on, usesTrueFalse: false)) - let offState = BooleanValueMapper.state(for: BooleanValueMapper.rawValue(for: .off, usesTrueFalse: false)) - #expect(onState == .on) - #expect(offState == .off) - } - - @Test("Round-trip literal style") - func roundTripLiteral() { - let onState = BooleanValueMapper.state(for: BooleanValueMapper.rawValue(for: .on, usesTrueFalse: true)) - let offState = BooleanValueMapper.state(for: BooleanValueMapper.rawValue(for: .off, usesTrueFalse: true)) - #expect(onState == .on) - #expect(offState == .off) - } -} From 2d37a7722b5619825dcde2ef53bdbc99e6994c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Tue, 28 Apr 2026 09:21:25 +0700 Subject: [PATCH 7/8] refactor(datagrid): restore final on DataGridCellView, guard drag-from-focused-cell --- TablePro/Views/Results/DataGridCellView.swift | 2 +- TablePro/Views/Results/KeyHandlingTableView.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/TablePro/Views/Results/DataGridCellView.swift b/TablePro/Views/Results/DataGridCellView.swift index a07ae4686..fe1e6c338 100644 --- a/TablePro/Views/Results/DataGridCellView.swift +++ b/TablePro/Views/Results/DataGridCellView.swift @@ -5,7 +5,7 @@ import AppKit -class DataGridCellView: NSTableCellView { +final class DataGridCellView: NSTableCellView { var fkArrowButton: FKArrowButton? var chevronButton: CellChevronButton? var textFieldTrailing: NSLayoutConstraint? diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index b61d17679..78d2e7703 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -98,7 +98,7 @@ final class KeyHandlingTableView: NSTableView { focusedRow = clickedRow focusedColumn = clickedColumn - if alreadyFocusedHere && event.clickCount == 1 { + if alreadyFocusedHere && event.clickCount == 1 && selectedRowIndexes.count == 1 { let dataColumnIndex = DataGridView.dataColumnIndex(for: clickedColumn) if coordinator?.canStartInlineEdit(row: clickedRow, columnIndex: dataColumnIndex) == true { editColumn(clickedColumn, row: clickedRow, with: nil, select: true) From c9760a53e2b197232d50f6fd2190d6da6bef5bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Tue, 28 Apr 2026 09:50:11 +0700 Subject: [PATCH 8/8] fix(datagrid): use adaptive border color for focused cell instead of system focus ring --- .../Views/Results/DataGridCellFactory.swift | 1 - TablePro/Views/Results/DataGridCellView.swift | 19 +++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/TablePro/Views/Results/DataGridCellFactory.swift b/TablePro/Views/Results/DataGridCellFactory.swift index 2c78ff376..8427d60a1 100644 --- a/TablePro/Views/Results/DataGridCellFactory.swift +++ b/TablePro/Views/Results/DataGridCellFactory.swift @@ -234,7 +234,6 @@ final class DataGridCellFactory { gridCellView.changeBackgroundColor = nil } - gridCellView.layer?.borderWidth = 0 gridCellView.isFocusedCell = isFocused CATransaction.commit() diff --git a/TablePro/Views/Results/DataGridCellView.swift b/TablePro/Views/Results/DataGridCellView.swift index fe1e6c338..4d4486cf6 100644 --- a/TablePro/Views/Results/DataGridCellView.swift +++ b/TablePro/Views/Results/DataGridCellView.swift @@ -13,7 +13,7 @@ final class DataGridCellView: NSTableCellView { var isFocusedCell: Bool = false { didSet { guard oldValue != isFocusedCell else { return } - noteFocusRingMaskChanged() + updateFocusBorder() } } @@ -46,15 +46,18 @@ final class DataGridCellView: NSTableCellView { override var backgroundStyle: NSView.BackgroundStyle { didSet { backgroundView.isHidden = (backgroundStyle == .emphasized) || (changeBackgroundColor == nil) + if isFocusedCell { updateFocusBorder() } } } - override var focusRingMaskBounds: NSRect { - isFocusedCell ? bounds : .zero - } - - override func drawFocusRingMask() { - guard isFocusedCell else { return } - bounds.fill() + private func updateFocusBorder() { + if isFocusedCell { + layer?.borderWidth = 2 + layer?.borderColor = backgroundStyle == .emphasized + ? NSColor.white.withAlphaComponent(0.8).cgColor + : NSColor.keyboardFocusIndicatorColor.cgColor + } else { + layer?.borderWidth = 0 + } } }