From 0f1e212552551c4b0251e59fee2589d9801bcd3f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 20 May 2026 00:48:36 +0700 Subject: [PATCH 1/7] feat(datagrid): read-only cell viewer with unified interaction dispatcher (#1336) --- CHANGELOG.md | 1 + TablePro/Resources/Localizable.xcstrings | 4 + .../Results/CellInteractionResolver.swift | 76 ++++++ TablePro/Views/Results/CellOverlayBase.swift | 197 +++++++++++++++ .../Views/Results/CellOverlayEditor.swift | 239 +++--------------- .../Views/Results/CellOverlayViewer.swift | 68 +++++ .../Views/Results/DataGridCoordinator.swift | 6 + TablePro/Views/Results/DataGridView.swift | 1 + .../Extensions/DataGridView+Click.swift | 97 ++++--- .../Extensions/DataGridView+Editing.swift | 10 + .../Extensions/DataGridView+Popovers.swift | 66 ++++- .../Views/Results/HexEditorContentView.swift | 88 ++++--- .../Views/Results/JSONViewerContentView.swift | 39 +++ .../Views/Results/KeyHandlingTableView.swift | 48 ++-- .../CellInteractionResolverTests.swift | 224 ++++++++++++++++ 15 files changed, 847 insertions(+), 317 deletions(-) create mode 100644 TablePro/Views/Results/CellInteractionResolver.swift create mode 100644 TablePro/Views/Results/CellOverlayBase.swift create mode 100644 TablePro/Views/Results/CellOverlayViewer.swift create mode 100644 TablePro/Views/Results/JSONViewerContentView.swift create mode 100644 TableProTests/Views/Results/CellInteractionResolverTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index e895228f7..3ce67a19e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Right-click a column header to copy all its values from the loaded rows (#1325) - Copy as submenu on the row context menu now offers CSV, CSV with Headers, Markdown table, and IN Clause for SQL `WHERE id IN (...)` lookups (#1325) +- Double-click or press Return on a query result cell to open a selectable text viewer in the cell. JSON columns open the JSON viewer in a popover, BLOB columns open the hex viewer. The value is selectable and copyable (#1336) ### Changed diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index 890e0328a..bc1c9fd34 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -49481,6 +49481,7 @@ } }, "Truncated — read only" : { + "extractionState" : "stale", "localizations" : { "tr" : { "stringUnit" : { @@ -49501,6 +49502,9 @@ } } } + }, + "Truncated, read only" : { + }, "Trust" : { "localizations" : { diff --git a/TablePro/Views/Results/CellInteractionResolver.swift b/TablePro/Views/Results/CellInteractionResolver.swift new file mode 100644 index 000000000..c6992d2c3 --- /dev/null +++ b/TablePro/Views/Results/CellInteractionResolver.swift @@ -0,0 +1,76 @@ +// +// CellInteractionResolver.swift +// TablePro +// + +import Foundation + +internal struct CellContext: Equatable { + let row: Int + let columnIndex: Int + let columnName: String + let columnType: ColumnType? + let value: String? + let isTableEditable: Bool + let isRowDeleted: Bool + let isImmutableColumn: Bool + let foreignKeyInfo: ForeignKeyInfo? + let isDropdownColumn: Bool + let isTypePickerColumn: Bool + let hasEnumValues: Bool +} + +internal enum CellInteractionMode: Equatable { + case viewInline(value: String) + case viewJson + case viewBlob + + case editInline(value: String) + case editOverlay(value: String) + case editJson + case editBlob + case editForeignKey(ForeignKeyInfo) + case editDropdown + case editEnum + case editSet + case editTypePicker + + case blocked +} + +internal struct CellInteractionResolver { + func resolve(_ context: CellContext) -> CellInteractionMode { + guard !context.isRowDeleted else { return .blocked } + + let isReadOnly = !context.isTableEditable || context.isImmutableColumn + + if isReadOnly { + if let columnType = context.columnType { + if columnType.isBlobType { return .viewBlob } + if columnType.isJsonType { return .viewJson } + } + return .viewInline(value: context.value ?? "NULL") + } + + if context.isDropdownColumn { return .editDropdown } + if context.isTypePickerColumn { return .editTypePicker } + + if let columnType = context.columnType, context.hasEnumValues { + return columnType.isSetType ? .editSet : .editEnum + } + + if let foreignKeyInfo = context.foreignKeyInfo { + return .editForeignKey(foreignKeyInfo) + } + + if let columnType = context.columnType { + if columnType.isBlobType { return .editBlob } + if columnType.isJsonType { return .editJson } + } + + let value = context.value ?? "" + if value.containsLineBreak { return .editOverlay(value: value) } + if value.looksLikeJson { return .editJson } + return .editInline(value: value) + } +} diff --git a/TablePro/Views/Results/CellOverlayBase.swift b/TablePro/Views/Results/CellOverlayBase.swift new file mode 100644 index 000000000..eb940e47a --- /dev/null +++ b/TablePro/Views/Results/CellOverlayBase.swift @@ -0,0 +1,197 @@ +// +// CellOverlayBase.swift +// TablePro +// + +import AppKit + +enum CellOverlayDismissReason { + case userAction + case scroll + case columnResize + case appResign + case windowResignKey + case outsideClick +} + +@MainActor +class CellOverlayBase: NSObject { + private var container: CellOverlayContainerView? + private weak var hostTableView: NSTableView? + private var scrollObserver: NSObjectProtocol? + private var columnResizeObserver: NSObjectProtocol? + private var appResignObserver: NSObjectProtocol? + private var windowResignKeyObserver: NSObjectProtocol? + private var outsideClickMonitor: Any? + + private(set) var row: Int = -1 + private(set) var column: Int = -1 + private(set) var columnIndex: Int = -1 + + var isActive: Bool { container != nil } + var containerView: NSView? { container } + var tableView: NSTableView? { hostTableView } + + func raiseToFront() { + guard let container, let hostTableView, container.superview === hostTableView else { return } + guard hostTableView.subviews.last !== container else { return } + hostTableView.addSubview(container) + } + + func install( + in tableView: NSTableView, + row: Int, + column: Int, + columnIndex: Int, + container: CellOverlayContainerView + ) { + self.hostTableView = tableView + self.row = row + self.column = column + self.columnIndex = columnIndex + tableView.addSubview(container) + self.container = container + installDismissObservers() + } + + func handleDismiss(reason: CellOverlayDismissReason) { + removeOverlay() + } + + func removeOverlay() { + guard let activeContainer = container else { return } + removeDismissObservers() + activeContainer.removeFromSuperview() + container = nil + if let hostTableView { + hostTableView.window?.makeFirstResponder(hostTableView) + } + } + + static func overlayFrame(for cellFrame: NSRect, value: String) -> NSRect { + let lineHeight = ThemeEngine.shared.dataGridFonts.regular.boundingRectForFont.height + 4 + var newlineCount = 0 + for scalar in value.unicodeScalars where scalar == "\n" { + newlineCount += 1 + } + let lineCount = CGFloat(newlineCount + 1) + let contentHeight = max(lineCount * lineHeight + 8, cellFrame.height) + let height = min(max(contentHeight, cellFrame.height), 120) + return NSRect(x: cellFrame.origin.x, y: cellFrame.origin.y, width: cellFrame.width, height: height) + } + + static func makeContainer(frame: NSRect) -> CellOverlayContainerView { + let container = CellOverlayContainerView(frame: frame) + container.wantsLayer = true + container.layer?.borderWidth = 2 + container.layer?.borderColor = NSColor.keyboardFocusIndicatorColor.cgColor + container.layer?.cornerRadius = 2 + container.layer?.masksToBounds = true + container.layer?.backgroundColor = NSColor.textBackgroundColor.cgColor + return container + } + + static func makeScrollView(in container: NSView) -> NSScrollView { + let scrollView = NSScrollView(frame: container.bounds) + scrollView.autoresizingMask = [.width, .height] + scrollView.hasVerticalScroller = true + scrollView.hasHorizontalScroller = false + scrollView.autohidesScrollers = true + scrollView.borderType = .noBorder + scrollView.drawsBackground = true + scrollView.backgroundColor = .textBackgroundColor + return scrollView + } + + private func installDismissObservers() { + guard let hostTableView else { return } + + if let clipView = hostTableView.enclosingScrollView?.contentView { + scrollObserver = NotificationCenter.default.addObserver( + forName: NSView.boundsDidChangeNotification, + object: clipView, + queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { + self?.handleDismiss(reason: .scroll) + } + } + } + + columnResizeObserver = NotificationCenter.default.addObserver( + forName: NSTableView.columnDidResizeNotification, + object: hostTableView, + queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { + self?.handleDismiss(reason: .columnResize) + } + } + + appResignObserver = NotificationCenter.default.addObserver( + forName: NSApplication.didResignActiveNotification, + object: nil, + queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { + self?.handleDismiss(reason: .appResign) + } + } + + if let overlayWindow = hostTableView.window { + windowResignKeyObserver = NotificationCenter.default.addObserver( + forName: NSWindow.didResignKeyNotification, + object: overlayWindow, + queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { + self?.handleDismiss(reason: .windowResignKey) + } + } + } + + outsideClickMonitor = NSEvent.addLocalMonitorForEvents(matching: [.leftMouseDown, .rightMouseDown]) { [weak self] event in + MainActor.assumeIsolated { + self?.handleOutsideClick(event: event) + } + return event + } + } + + private func removeDismissObservers() { + if let observer = scrollObserver { + NotificationCenter.default.removeObserver(observer) + scrollObserver = nil + } + if let observer = columnResizeObserver { + NotificationCenter.default.removeObserver(observer) + columnResizeObserver = nil + } + if let observer = appResignObserver { + NotificationCenter.default.removeObserver(observer) + appResignObserver = nil + } + if let observer = windowResignKeyObserver { + NotificationCenter.default.removeObserver(observer) + windowResignKeyObserver = nil + } + if let monitor = outsideClickMonitor { + NSEvent.removeMonitor(monitor) + outsideClickMonitor = nil + } + } + + private func handleOutsideClick(event: NSEvent) { + guard let containerView = container, + let containerWindow = containerView.window, + event.window === containerWindow else { return } + let frameInWindow = containerView.convert(containerView.bounds, to: nil) + if !frameInWindow.contains(event.locationInWindow) { + handleDismiss(reason: .outsideClick) + } + } +} + +final class CellOverlayContainerView: NSView { + override var isFlipped: Bool { true } +} diff --git a/TablePro/Views/Results/CellOverlayEditor.swift b/TablePro/Views/Results/CellOverlayEditor.swift index 7c5db03fb..9fcd3fd2b 100644 --- a/TablePro/Views/Results/CellOverlayEditor.swift +++ b/TablePro/Views/Results/CellOverlayEditor.swift @@ -6,36 +6,13 @@ import AppKit @MainActor -final class CellOverlayEditor: NSObject, NSTextViewDelegate { - private var container: OverlayContainerView? - private var textView: OverlayTextView? - private weak var tableView: NSTableView? - private var scrollObserver: NSObjectProtocol? - private var columnResizeObserver: NSObjectProtocol? - private var appResignObserver: NSObjectProtocol? - private var windowResignKeyObserver: NSObjectProtocol? - private var outsideClickMonitor: Any? - - private(set) var row: Int = -1 - private(set) var column: Int = -1 - private(set) var columnIndex: Int = -1 +final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate { + private var editorTextView: OverlayTextView? private var initialValue: String = "" var onCommit: ((_ row: Int, _ columnIndex: Int, _ newValue: String) -> Void)? var onTabNavigation: ((_ row: Int, _ column: Int, _ forward: Bool) -> Void)? - var isActive: Bool { container != nil } - - var containerView: NSView? { container } - - func raiseToFront() { - guard let container, let tableView, container.superview === tableView else { return } - guard tableView.subviews.last !== container else { return } - tableView.addSubview(container) - } - - // MARK: - Show / Dismiss - func show( in tableView: NSTableView, row: Int, @@ -45,193 +22,63 @@ final class CellOverlayEditor: NSObject, NSTextViewDelegate { ) { dismiss(commit: true) - self.tableView = tableView - self.row = row - self.column = column - self.columnIndex = columnIndex - self.initialValue = value - let cellFrame = tableView.frameOfCell(atColumn: column, row: row) guard !cellFrame.isEmpty else { return } guard let window = tableView.window else { return } - let lineHeight = ThemeEngine.shared.dataGridFonts.regular.boundingRectForFont.height + 4 - var newlineCount = 0 - for scalar in value.unicodeScalars where scalar == "\n" { - newlineCount += 1 - } - let lineCount = CGFloat(newlineCount + 1) - let contentHeight = max(lineCount * lineHeight + 8, cellFrame.height) - let overlayHeight = min(max(contentHeight, cellFrame.height), 120) - - let editorFrame = NSRect( - x: cellFrame.origin.x, - y: cellFrame.origin.y, - width: cellFrame.width, - height: overlayHeight - ) - - let containerView = OverlayContainerView(frame: editorFrame) - containerView.wantsLayer = true - containerView.layer?.borderWidth = 2 - containerView.layer?.borderColor = NSColor.keyboardFocusIndicatorColor.cgColor - containerView.layer?.cornerRadius = 2 - containerView.layer?.masksToBounds = true - containerView.layer?.backgroundColor = NSColor.textBackgroundColor.cgColor - - let scrollView = NSScrollView(frame: containerView.bounds) - scrollView.autoresizingMask = [.width, .height] - scrollView.hasVerticalScroller = true - scrollView.hasHorizontalScroller = false - scrollView.autohidesScrollers = true - scrollView.borderType = .noBorder - scrollView.drawsBackground = true - scrollView.backgroundColor = .textBackgroundColor - - let editorTextView = OverlayTextView(frame: scrollView.bounds) - editorTextView.overlayEditor = self - editorTextView.isRichText = false - editorTextView.allowsUndo = true - editorTextView.font = ThemeEngine.shared.dataGridFonts.regular - editorTextView.textColor = .labelColor - editorTextView.backgroundColor = .textBackgroundColor - editorTextView.isVerticallyResizable = true - editorTextView.isHorizontallyResizable = false - editorTextView.textContainer?.widthTracksTextView = true - editorTextView.textContainer?.containerSize = NSSize( + let frame = Self.overlayFrame(for: cellFrame, value: value) + let containerView = Self.makeContainer(frame: frame) + let scrollView = Self.makeScrollView(in: containerView) + + let textView = OverlayTextView(frame: scrollView.bounds) + textView.overlayEditor = self + textView.isEditable = true + textView.isRichText = false + textView.allowsUndo = true + textView.font = ThemeEngine.shared.dataGridFonts.regular + textView.textColor = .labelColor + textView.backgroundColor = .textBackgroundColor + textView.isVerticallyResizable = true + textView.isHorizontallyResizable = false + textView.textContainer?.widthTracksTextView = true + textView.textContainer?.containerSize = NSSize( width: scrollView.bounds.width, height: CGFloat.greatestFiniteMagnitude ) - editorTextView.delegate = self - editorTextView.string = value - editorTextView.selectAll(nil) + textView.delegate = self + textView.string = value + textView.selectAll(nil) - scrollView.documentView = editorTextView + scrollView.documentView = textView containerView.addSubview(scrollView) - tableView.addSubview(containerView) - container = containerView - textView = editorTextView + initialValue = value + editorTextView = textView - window.makeFirstResponder(editorTextView) + install(in: tableView, row: row, column: column, columnIndex: columnIndex, container: containerView) + window.makeFirstResponder(textView) + } - installDismissObservers() + override func handleDismiss(reason: CellOverlayDismissReason) { + dismiss(commit: reason != .columnResize) } func dismiss(commit: Bool) { - guard let activeContainer = container, let activeTextView = textView else { return } - + guard let activeTextView = editorTextView else { return } let newValue = activeTextView.string let originalValue = initialValue + let dismissRow = row + let dismissColumnIndex = columnIndex - removeDismissObservers() - - activeContainer.removeFromSuperview() - container = nil - textView = nil + editorTextView = nil initialValue = "" - - if let tableView { - tableView.window?.makeFirstResponder(tableView) - } + removeOverlay() if commit, newValue != originalValue { - onCommit?(row, columnIndex, newValue) - } - } - - // MARK: - Observers - - private func installDismissObservers() { - guard let tableView else { return } - - if let clipView = tableView.enclosingScrollView?.contentView { - scrollObserver = NotificationCenter.default.addObserver( - forName: NSView.boundsDidChangeNotification, - object: clipView, - queue: .main - ) { [weak self] _ in - MainActor.assumeIsolated { - self?.dismiss(commit: true) - } - } - } - - columnResizeObserver = NotificationCenter.default.addObserver( - forName: NSTableView.columnDidResizeNotification, - object: tableView, - queue: .main - ) { [weak self] _ in - MainActor.assumeIsolated { - self?.dismiss(commit: false) - } - } - - appResignObserver = NotificationCenter.default.addObserver( - forName: NSApplication.didResignActiveNotification, - object: nil, - queue: .main - ) { [weak self] _ in - MainActor.assumeIsolated { - self?.dismiss(commit: true) - } - } - - if let editorWindow = tableView.window { - windowResignKeyObserver = NotificationCenter.default.addObserver( - forName: NSWindow.didResignKeyNotification, - object: editorWindow, - queue: .main - ) { [weak self] _ in - MainActor.assumeIsolated { - self?.dismiss(commit: true) - } - } - } - - outsideClickMonitor = NSEvent.addLocalMonitorForEvents(matching: [.leftMouseDown, .rightMouseDown]) { [weak self] event in - MainActor.assumeIsolated { - self?.handleOutsideClick(event: event) - } - return event - } - } - - private func removeDismissObservers() { - if let observer = scrollObserver { - NotificationCenter.default.removeObserver(observer) - scrollObserver = nil - } - if let observer = columnResizeObserver { - NotificationCenter.default.removeObserver(observer) - columnResizeObserver = nil - } - if let observer = appResignObserver { - NotificationCenter.default.removeObserver(observer) - appResignObserver = nil - } - if let observer = windowResignKeyObserver { - NotificationCenter.default.removeObserver(observer) - windowResignKeyObserver = nil - } - if let monitor = outsideClickMonitor { - NSEvent.removeMonitor(monitor) - outsideClickMonitor = nil + onCommit?(dismissRow, dismissColumnIndex, newValue) } } - private func handleOutsideClick(event: NSEvent) { - guard let containerView = container, - let containerWindow = containerView.window, - event.window === containerWindow else { return } - let frameInWindow = containerView.convert(containerView.bounds, to: nil) - if !frameInWindow.contains(event.locationInWindow) { - dismiss(commit: true) - } - } - - // MARK: - NSTextViewDelegate - func textView(_ textView: NSTextView, doCommandBy commandSelector: Selector) -> Bool { if commandSelector == #selector(NSResponder.insertNewline(_:)) { if NSApp.currentEvent?.modifierFlags.contains(.option) == true { @@ -248,16 +95,16 @@ final class CellOverlayEditor: NSObject, NSTextViewDelegate { } if commandSelector == #selector(NSResponder.insertTab(_:)) { - let r = row, c = column + let dismissRow = row, dismissColumn = column dismiss(commit: true) - onTabNavigation?(r, c, true) + onTabNavigation?(dismissRow, dismissColumn, true) return true } if commandSelector == #selector(NSResponder.insertBacktab(_:)) { - let r = row, c = column + let dismissRow = row, dismissColumn = column dismiss(commit: true) - onTabNavigation?(r, c, false) + onTabNavigation?(dismissRow, dismissColumn, false) return true } @@ -265,14 +112,6 @@ final class CellOverlayEditor: NSObject, NSTextViewDelegate { } } -// MARK: - Container View - -private final class OverlayContainerView: NSView { - override var isFlipped: Bool { true } -} - -// MARK: - Overlay Text View - private final class OverlayTextView: NSTextView { private let storedUndoManager = UndoManager() diff --git a/TablePro/Views/Results/CellOverlayViewer.swift b/TablePro/Views/Results/CellOverlayViewer.swift new file mode 100644 index 000000000..05f9c26a0 --- /dev/null +++ b/TablePro/Views/Results/CellOverlayViewer.swift @@ -0,0 +1,68 @@ +// +// CellOverlayViewer.swift +// TablePro +// + +import AppKit + +@MainActor +final class CellOverlayViewer: CellOverlayBase, NSTextViewDelegate { + func show( + in tableView: NSTableView, + row: Int, + column: Int, + columnIndex: Int, + value: String + ) { + removeOverlay() + + let cellFrame = tableView.frameOfCell(atColumn: column, row: row) + guard !cellFrame.isEmpty else { return } + guard let window = tableView.window else { return } + + let frame = Self.overlayFrame(for: cellFrame, value: value) + let containerView = Self.makeContainer(frame: frame) + let scrollView = Self.makeScrollView(in: containerView) + + let textView = NSTextView(frame: scrollView.bounds) + textView.isEditable = false + textView.isSelectable = true + textView.isRichText = false + textView.font = ThemeEngine.shared.dataGridFonts.regular + textView.textColor = .labelColor + textView.backgroundColor = .textBackgroundColor + textView.isVerticallyResizable = true + textView.isHorizontallyResizable = false + textView.textContainer?.widthTracksTextView = true + textView.textContainer?.containerSize = NSSize( + width: scrollView.bounds.width, + height: CGFloat.greatestFiniteMagnitude + ) + textView.delegate = self + textView.string = value + textView.selectAll(nil) + + scrollView.documentView = textView + containerView.addSubview(scrollView) + + install(in: tableView, row: row, column: column, columnIndex: columnIndex, container: containerView) + window.makeFirstResponder(textView) + } + + func dismiss() { + removeOverlay() + } + + func textView(_ textView: NSTextView, doCommandBy commandSelector: Selector) -> Bool { + switch commandSelector { + case #selector(NSResponder.insertNewline(_:)), + #selector(NSResponder.cancelOperation(_:)), + #selector(NSResponder.insertTab(_:)), + #selector(NSResponder.insertBacktab(_:)): + removeOverlay() + return true + default: + return false + } + } +} diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index 3080619b2..5aa49651a 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -95,6 +95,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData let columnPool = DataGridColumnPool() let tableRowsController = TableRowsController() var overlayEditor: CellOverlayEditor? + var overlayViewer: CellOverlayViewer? var settingsCancellable: AnyCancellable? var themeCancellable: AnyCancellable? @@ -196,6 +197,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData prewarmResumeTask = nil detachScrollObservers() overlayEditor?.dismiss(commit: false) + overlayViewer?.dismiss() settingsCancellable?.cancel() settingsCancellable = nil themeCancellable?.cancel() @@ -492,17 +494,20 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData case .rowsInserted(let indices): guard !indices.isEmpty else { return } overlayEditor?.dismiss(commit: false) + overlayViewer?.dismiss() dismissFKPreviewOnColumnChange() appendInsertedIDsToSortedIDs(at: indices) applyInsertedRows(indices) case .rowsRemoved(let indices): guard !indices.isEmpty else { return } overlayEditor?.dismiss(commit: false) + overlayViewer?.dismiss() dismissFKPreviewOnColumnChange() removeMissingIDsFromSortedIDs() applyRemovedRows(indices) case .columnsReplaced, .fullReplace: overlayEditor?.dismiss(commit: false) + overlayViewer?.dismiss() dismissFKPreviewOnColumnChange() sortedIDs = nil applyFullReplace() @@ -581,6 +586,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData func commitActiveCellEdit() { overlayEditor?.dismiss(commit: true) + overlayViewer?.dismiss() guard let tableView, let window = tableView.window else { return } if let firstResponder = window.firstResponder as? NSView, firstResponder.isDescendant(of: tableView) { diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 0665159be..302bc7fd9 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -142,6 +142,7 @@ struct DataGridView: NSViewRepresentable { if tableView.editedRow >= 0 { return } if let editor = coordinator.overlayEditor, editor.isActive { return } + if let viewer = coordinator.overlayViewer, viewer.isActive { return } coordinator.tableRowsProvider = tableRowsProvider coordinator.tableRowsMutator = tableRowsMutator diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index 8323045fd..0521c4dea 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -10,57 +10,70 @@ extension TableViewCoordinator { // MARK: - Click Handlers @objc func handleDoubleClick(_ sender: NSTableView) { - guard isEditable else { return } - let row = sender.clickedRow let column = sender.clickedColumn guard row >= 0, column > 0 else { return } - guard let columnIndex = DataGridView.dataColumnIndex(for: column, in: sender, schema: identitySchema) else { return } - guard !changeManager.isRowDeleted(row) else { return } - - let tableRows = tableRowsProvider() - let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? [] - if !immutable.isEmpty, - columnIndex < tableRows.columns.count, - immutable.contains(tableRows.columns[columnIndex]) { - return - } - - if columnIndex < tableRows.columns.count { - let columnName = tableRows.columns[columnIndex] - if let fkInfo = tableRows.columnForeignKeys[columnName] { - showForeignKeyPopover(tableView: sender, row: row, column: column, columnIndex: columnIndex, fkInfo: fkInfo) - return - } - } + handleCellInteraction(row: row, tableColumn: column, columnIndex: columnIndex, tableView: sender) + } - // Column-type guards run BEFORE content checks. Binary cells contain bytes - // that may incidentally match line-break or JSON heuristics; routing them - // through the text/overlay editor would corrupt the bytes on save. - if columnIndex < tableRows.columnTypes.count { - let ct = tableRows.columnTypes[columnIndex] - if ct.isBlobType { - showBlobEditorPopover(tableView: sender, row: row, column: column, columnIndex: columnIndex) - return - } - if ct.isJsonType { - showJSONEditorPopover(tableView: sender, row: row, column: column, columnIndex: columnIndex) - return - } - } + func handleCellInteraction(row: Int, tableColumn: Int, columnIndex: Int, tableView: NSTableView) { + guard let context = makeCellContext(row: row, columnIndex: columnIndex) else { return } + guard tableView.view(atColumn: tableColumn, row: row, makeIfNecessary: false) != nil else { return } - let value = cellValue(at: row, column: columnIndex) - if let value, value.containsLineBreak { - showOverlayEditor(tableView: sender, row: row, column: column, columnIndex: columnIndex, value: value) - return - } - if let value, value.looksLikeJson { - showJSONEditorPopover(tableView: sender, row: row, column: column, columnIndex: columnIndex) + switch CellInteractionResolver().resolve(context) { + case .blocked: return + case .viewInline(let value): + showOverlayViewer(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex, value: value) + case .viewJson: + showJSONViewerPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) + case .viewBlob: + showBlobViewerPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) + case .editInline: + beginCellEdit(row: row, tableColumnIndex: tableColumn) + case .editOverlay(let value): + showOverlayEditor(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex, value: value) + case .editJson: + showJSONEditorPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) + case .editBlob: + showBlobEditorPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) + case .editForeignKey(let fkInfo): + showForeignKeyPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex, fkInfo: fkInfo) + case .editDropdown: + showDropdownMenu(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) + case .editEnum: + showEnumPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) + case .editSet: + showSetPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) + case .editTypePicker: + showTypePickerPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) } + } + + private func makeCellContext(row: Int, columnIndex: Int) -> CellContext? { + let tableRows = tableRowsProvider() + guard row >= 0, columnIndex >= 0, columnIndex < tableRows.columns.count else { return nil } - beginCellEdit(row: row, tableColumnIndex: column) + let columnName = tableRows.columns[columnIndex] + let columnType = columnIndex < tableRows.columnTypes.count ? tableRows.columnTypes[columnIndex] : nil + let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? [] + let hasEnumValues = (tableRows.columnEnumValues[columnName]?.isEmpty == false) + + return CellContext( + row: row, + columnIndex: columnIndex, + columnName: columnName, + columnType: columnType, + value: cellValue(at: row, column: columnIndex), + isTableEditable: isEditable, + isRowDeleted: changeManager.isRowDeleted(row), + isImmutableColumn: immutable.contains(columnName), + foreignKeyInfo: tableRows.columnForeignKeys[columnName], + isDropdownColumn: dropdownColumns?.contains(columnIndex) == true, + isTypePickerColumn: typePickerColumns?.contains(columnIndex) == true, + hasEnumValues: hasEnumValues + ) } // MARK: - Chevron Click diff --git a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift index 2ea83c167..611cd902a 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift @@ -84,9 +84,19 @@ extension TableViewCoordinator { editor.onTabNavigation = { [weak self] row, column, forward in self?.handleOverlayTabNavigation(row: row, column: column, forward: forward) } + overlayViewer?.dismiss() editor.show(in: tableView, row: row, column: column, columnIndex: columnIndex, value: value) } + func showOverlayViewer(tableView: NSTableView, row: Int, column: Int, columnIndex: Int, value: String) { + if overlayViewer == nil { + overlayViewer = CellOverlayViewer() + } + guard let viewer = overlayViewer else { return } + overlayEditor?.dismiss(commit: false) + viewer.show(in: tableView, row: row, column: column, columnIndex: columnIndex, value: value) + } + func handleOverlayTabNavigation(row: Int, column: Int, forward: Bool) { guard let tableView = tableView else { return } diff --git a/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift b/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift index 453c30803..723997673 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift @@ -178,13 +178,7 @@ extension TableViewCoordinator { } func showBlobEditorPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - let typed = cellTypedValue(at: row, column: columnIndex) - let currentValue: String? - switch typed { - case .null: currentValue = nil - case .text(let s): currentValue = s - case .bytes(let data): currentValue = String(data: data, encoding: .isoLatin1) - } + let currentValue = blobStringValue(at: row, columnIndex: columnIndex) guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } @@ -316,6 +310,64 @@ extension TableViewCoordinator { func commitBinaryEdit(row: Int, columnIndex: Int, data: Data) { commitTypedCellEdit(row: row, columnIndex: columnIndex, newValue: .bytes(data)) } + + func showJSONViewerPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { + let currentValue = cellValue(at: row, column: columnIndex) + let tableRows = tableRowsProvider() + guard columnIndex >= 0, columnIndex < tableRows.columns.count else { return } + let columnName = tableRows.columns[columnIndex] + + guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + + let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) + PopoverPresenter.show( + relativeTo: cellRect, + of: tableView, + contentSize: nil + ) { dismiss in + JSONViewerContentView( + initialValue: currentValue, + columnName: columnName, + onDismiss: dismiss, + onPopOut: { currentText in + dismiss() + JSONViewerWindowController.open( + text: currentText, + columnName: columnName, + isEditable: false, + onCommit: nil + ) + } + ) + } + } + + func showBlobViewerPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { + let currentValue = blobStringValue(at: row, columnIndex: columnIndex) + + guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + + let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) + PopoverPresenter.show( + relativeTo: cellRect, + of: tableView, + contentSize: nil + ) { dismiss in + HexEditorContentView( + initialValue: currentValue, + isEditable: false, + onDismiss: dismiss + ) + } + } + + private func blobStringValue(at row: Int, columnIndex: Int) -> String? { + switch cellTypedValue(at: row, column: columnIndex) { + case .null: return nil + case .text(let text): return text + case .bytes(let data): return String(data: data, encoding: .isoLatin1) + } + } } private final class DropdownMenuContext { diff --git a/TablePro/Views/Results/HexEditorContentView.swift b/TablePro/Views/Results/HexEditorContentView.swift index d45757f63..f8ab5e126 100644 --- a/TablePro/Views/Results/HexEditorContentView.swift +++ b/TablePro/Views/Results/HexEditorContentView.swift @@ -10,6 +10,7 @@ import SwiftUI struct HexEditorContentView: View { let initialValue: String? + let isEditable: Bool let onCommit: (String) -> Void let onCommitBytes: ((Data) -> Void)? let onDismiss: () -> Void @@ -23,11 +24,13 @@ struct HexEditorContentView: View { init( initialValue: String?, - onCommit: @escaping (String) -> Void, + isEditable: Bool = true, + onCommit: @escaping (String) -> Void = { _ in }, onCommitBytes: ((Data) -> Void)? = nil, onDismiss: @escaping () -> Void ) { self.initialValue = initialValue + self.isEditable = isEditable self.onCommit = onCommit self.onCommitBytes = onCommitBytes self.onDismiss = onDismiss @@ -52,51 +55,66 @@ struct HexEditorContentView: View { VStack(spacing: 0) { HexDumpDisplayView(text: hexDumpText) - Divider() + if isEditable { + Divider() - VStack(alignment: .leading, spacing: 4) { - Text("Editable Hex") - .font(.caption) - .foregroundStyle(.secondary) + VStack(alignment: .leading, spacing: 4) { + Text("Editable Hex") + .font(.caption) + .foregroundStyle(.secondary) + + HexInputTextView(text: $editableHex) + .frame(height: 80) + + HStack(spacing: 4) { + Text("\(byteCount) bytes") + .font(.caption) + .foregroundStyle(.tertiary) + + if isTruncated { + Text(String(localized: "Truncated, read only")) + .font(.caption) + .foregroundStyle(.orange) + } else if !isValid, !editableHex.isEmpty { + Text(String(localized: "Invalid hex")) + .font(.caption) + .foregroundStyle(.red) + } + + Spacer() + } + } + .padding(.horizontal, 12) + .padding(.vertical, 8) - HexInputTextView(text: $editableHex) - .frame(height: 80) + Divider() + + HStack { + Spacer() + Button("Cancel") { onDismiss() } + .keyboardShortcut(.cancelAction) + Button("Save") { saveHex() } + .keyboardShortcut(.defaultAction) + .disabled(!isValid || isTruncated) + } + .padding(.horizontal, 12) + .padding(.vertical, 8) + } else { + Divider() HStack(spacing: 4) { Text("\(byteCount) bytes") .font(.caption) .foregroundStyle(.tertiary) - - if isTruncated { - Text(String(localized: "Truncated — read only")) - .font(.caption) - .foregroundStyle(.orange) - } else if !isValid, !editableHex.isEmpty { - Text(String(localized: "Invalid hex")) - .font(.caption) - .foregroundStyle(.red) - } - Spacer() + Button("Close") { onDismiss() } + .keyboardShortcut(.cancelAction) } + .padding(.horizontal, 12) + .padding(.vertical, 8) } - .padding(.horizontal, 12) - .padding(.vertical, 8) - - Divider() - - HStack { - Spacer() - Button("Cancel") { onDismiss() } - .keyboardShortcut(.cancelAction) - Button("Save") { saveHex() } - .keyboardShortcut(.defaultAction) - .disabled(!isValid || isTruncated) - } - .padding(.horizontal, 12) - .padding(.vertical, 8) } - .frame(width: 520, height: 400) + .frame(width: 520, height: isEditable ? 400 : 280) .onChange(of: editableHex) { _, newValue in scheduleValidation(newValue) } diff --git a/TablePro/Views/Results/JSONViewerContentView.swift b/TablePro/Views/Results/JSONViewerContentView.swift new file mode 100644 index 000000000..428f384ca --- /dev/null +++ b/TablePro/Views/Results/JSONViewerContentView.swift @@ -0,0 +1,39 @@ +// +// JSONViewerContentView.swift +// TablePro +// + +import SwiftUI + +struct JSONViewerContentView: View { + let initialValue: String? + let columnName: String? + let onDismiss: () -> Void + var onPopOut: ((String) -> Void)? + + @State private var text: String + + init( + initialValue: String?, + columnName: String? = nil, + onDismiss: @escaping () -> Void, + onPopOut: ((String) -> Void)? = nil + ) { + self.initialValue = initialValue + self.columnName = columnName + self.onDismiss = onDismiss + self.onPopOut = onPopOut + self._text = State(initialValue: initialValue?.prettyPrintedAsJson() ?? initialValue ?? "") + } + + var body: some View { + JSONViewerView( + text: $text, + isEditable: false, + onDismiss: onDismiss, + onPopOut: onPopOut + ) + .frame(width: 560) + .frame(minHeight: 200, maxHeight: 480) + } +} diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index 47fcd4e54..c8a807f5b 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -4,6 +4,7 @@ final class KeyHandlingTableView: NSTableView { weak var coordinator: TableViewCoordinator? private var isRaisingOverlayEditor = false + private var isRaisingOverlayViewer = false override var acceptsFirstResponder: Bool { true @@ -11,16 +12,21 @@ final class KeyHandlingTableView: NSTableView { override func didAddSubview(_ subview: NSView) { super.didAddSubview(subview) - guard !isRaisingOverlayEditor else { return } - guard let editor = coordinator?.overlayEditor, - editor.isActive, - let container = editor.containerView, + raiseOverlayIfNeeded(coordinator?.overlayEditor, subview: subview, flag: &isRaisingOverlayEditor) + raiseOverlayIfNeeded(coordinator?.overlayViewer, subview: subview, flag: &isRaisingOverlayViewer) + } + + private func raiseOverlayIfNeeded(_ overlay: CellOverlayBase?, subview: NSView, flag: inout Bool) { + guard !flag else { return } + guard let overlay, + overlay.isActive, + let container = overlay.containerView, container !== subview, container.superview === self, subviews.last !== container else { return } - isRaisingOverlayEditor = true - editor.raiseToFront() - isRaisingOverlayEditor = false + flag = true + overlay.raiseToFront() + flag = false } var selection = TableSelection() { @@ -161,7 +167,7 @@ final class KeyHandlingTableView: NSTableView { case #selector(paste(_:)): return coordinator?.isEditable == true && coordinator?.delegate != nil case #selector(insertNewline(_:)): - return selectedRow >= 0 && DataGridView.isDataTableColumn(focusedColumn) && coordinator?.isEditable == true + return selectedRow >= 0 && DataGridView.isDataTableColumn(focusedColumn) case #selector(cancelOperation(_:)): return false default: @@ -233,36 +239,12 @@ final class KeyHandlingTableView: NSTableView { let row = selectedRow guard row >= 0, DataGridView.isDataTableColumn(focusedColumn), - coordinator?.isEditable == true, let schema = coordinator?.identitySchema, let columnIndex = DataGridView.dataColumnIndex(for: focusedColumn, in: self, schema: schema), let coordinator else { return } - - // Dropdown / type-picker columns: Return opens the popup, matching the - // chevron and double-click paths. Without this branch, Return on a focused - // dropdown cell does nothing because beginCellEdit is blocked by editEligibility. - if coordinator.dropdownColumns?.contains(columnIndex) == true || - coordinator.typePickerColumns?.contains(columnIndex) == true { - coordinator.handleChevronAction(row: row, columnIndex: columnIndex) - return - } - - let tableRows = coordinator.tableRowsProvider() - if columnIndex < tableRows.columnTypes.count, - tableRows.columnTypes[columnIndex].isBlobType { - coordinator.showBlobEditorPopover(tableView: self, row: row, column: focusedColumn, columnIndex: columnIndex) - return - } - - if let value = coordinator.cellValue(at: row, column: columnIndex), - value.containsLineBreak { - coordinator.showOverlayEditor(tableView: self, row: row, column: focusedColumn, columnIndex: columnIndex, value: value) - return - } - - coordinator.beginCellEdit(row: row, tableColumnIndex: focusedColumn) + coordinator.handleCellInteraction(row: row, tableColumn: focusedColumn, columnIndex: columnIndex, tableView: self) } @objc override func cancelOperation(_ sender: Any?) { diff --git a/TableProTests/Views/Results/CellInteractionResolverTests.swift b/TableProTests/Views/Results/CellInteractionResolverTests.swift new file mode 100644 index 000000000..2b0adbcba --- /dev/null +++ b/TableProTests/Views/Results/CellInteractionResolverTests.swift @@ -0,0 +1,224 @@ +// +// CellInteractionResolverTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("CellInteractionResolver - read-only path") +struct CellInteractionResolverReadOnlyTests { + private let resolver = CellInteractionResolver() + + @Test("deleted row returns blocked regardless of editability") + func deletedRowReturnsBlocked() { + let context = ContextFactory.make(value: "hello", isTableEditable: false, isRowDeleted: true) + #expect(resolver.resolve(context) == .blocked) + } + + @Test("deleted row blocked even in editable table") + func deletedRowBlockedInEditableTable() { + let context = ContextFactory.make(value: "hello", isTableEditable: true, isRowDeleted: true) + #expect(resolver.resolve(context) == .blocked) + } + + @Test("read-only plain text returns viewInline with value") + func readOnlyPlainTextReturnsViewInline() { + let context = ContextFactory.make(value: "hello", isTableEditable: false) + #expect(resolver.resolve(context) == .viewInline(value: "hello")) + } + + @Test("read-only single-line text returns viewInline") + func readOnlySingleLineReturnsViewInline() { + let context = ContextFactory.make(value: "A", isTableEditable: false) + #expect(resolver.resolve(context) == .viewInline(value: "A")) + } + + @Test("read-only nil value returns viewInline with NULL placeholder") + func readOnlyNilValueReturnsViewInlineWithNull() { + let context = ContextFactory.make(value: nil, isTableEditable: false) + #expect(resolver.resolve(context) == .viewInline(value: "NULL")) + } + + @Test("read-only multiline text returns viewInline") + func readOnlyMultilineReturnsViewInline() { + let context = ContextFactory.make(value: "line1\nline2", isTableEditable: false) + #expect(resolver.resolve(context) == .viewInline(value: "line1\nline2")) + } + + @Test("read-only JSON column returns viewJson") + func readOnlyJsonColumnReturnsViewJson() { + let context = ContextFactory.make(value: #"{"k":1}"#, columnType: .json(rawType: "JSON"), isTableEditable: false) + #expect(resolver.resolve(context) == .viewJson) + } + + @Test("read-only BLOB column returns viewBlob") + func readOnlyBlobColumnReturnsViewBlob() { + let context = ContextFactory.make(value: nil, columnType: .blob(rawType: "BLOB"), isTableEditable: false) + #expect(resolver.resolve(context) == .viewBlob) + } + + @Test("immutable column on editable table follows read-only path for plain text") + func immutableColumnFollowsReadOnlyPath() { + let context = ContextFactory.make(value: "id-123", isTableEditable: true, isImmutableColumn: true) + #expect(resolver.resolve(context) == .viewInline(value: "id-123")) + } + + @Test("immutable JSON column on editable table still returns viewJson") + func immutableJsonColumnReturnsViewJson() { + let context = ContextFactory.make(value: "{}", columnType: .json(rawType: "JSON"), isTableEditable: true, isImmutableColumn: true) + #expect(resolver.resolve(context) == .viewJson) + } + + @Test("read-only FK column falls through to viewInline") + func readOnlyForeignKeyReturnsViewInline() { + let fkInfo = ForeignKeyInfo(name: "fk", column: "uid", referencedTable: "u", referencedColumn: "id") + let context = ContextFactory.make( + value: "42", + columnType: .integer(rawType: "INT"), + isTableEditable: false, + foreignKeyInfo: fkInfo + ) + #expect(resolver.resolve(context) == .viewInline(value: "42")) + } + + @Test("read-only JSON-looking plain text without columnType returns viewInline, not viewJson") + func readOnlyJsonLikeTextWithoutTypeReturnsViewInline() { + let context = ContextFactory.make(value: #"{"k":1}"#, columnType: nil, isTableEditable: false) + #expect(resolver.resolve(context) == .viewInline(value: #"{"k":1}"#)) + } +} + +@Suite("CellInteractionResolver - editable path") +struct CellInteractionResolverEditableTests { + private let resolver = CellInteractionResolver() + + @Test("editable plain single-line returns editInline") + func editablePlainSingleLineReturnsEditInline() { + let context = ContextFactory.make(value: "hello", isTableEditable: true) + #expect(resolver.resolve(context) == .editInline(value: "hello")) + } + + @Test("editable plain multiline returns editOverlay") + func editablePlainMultilineReturnsEditOverlay() { + let context = ContextFactory.make(value: "line1\nline2", isTableEditable: true) + #expect(resolver.resolve(context) == .editOverlay(value: "line1\nline2")) + } + + @Test("editable plain text that looks like JSON returns editJson") + func editableJsonLikeTextReturnsEditJson() { + let context = ContextFactory.make(value: #"{"k":1}"#, isTableEditable: true) + #expect(resolver.resolve(context) == .editJson) + } + + @Test("editable JSON column returns editJson") + func editableJsonColumnReturnsEditJson() { + let context = ContextFactory.make(value: "{}", columnType: .json(rawType: "JSON"), isTableEditable: true) + #expect(resolver.resolve(context) == .editJson) + } + + @Test("editable BLOB column returns editBlob") + func editableBlobColumnReturnsEditBlob() { + let context = ContextFactory.make(value: nil, columnType: .blob(rawType: "BLOB"), isTableEditable: true) + #expect(resolver.resolve(context) == .editBlob) + } + + @Test("editable foreign key column returns editForeignKey") + func editableForeignKeyReturnsEditForeignKey() { + let fkInfo = ForeignKeyInfo(name: "fk_users", column: "user_id", referencedTable: "users", referencedColumn: "id") + let context = ContextFactory.make( + value: "1", + columnType: .integer(rawType: "INT"), + isTableEditable: true, + foreignKeyInfo: fkInfo + ) + #expect(resolver.resolve(context) == .editForeignKey(fkInfo)) + } + + @Test("editable FK column with nil columnType still returns editForeignKey") + func editableForeignKeyNilColumnTypeReturnsEditForeignKey() { + let fkInfo = ForeignKeyInfo(name: "fk", column: "uid", referencedTable: "u", referencedColumn: "id") + let context = ContextFactory.make( + value: "1", + columnType: nil, + isTableEditable: true, + foreignKeyInfo: fkInfo + ) + #expect(resolver.resolve(context) == .editForeignKey(fkInfo)) + } + + @Test("editable dropdown column returns editDropdown") + func editableDropdownColumnReturnsEditDropdown() { + let context = ContextFactory.make(value: "true", isTableEditable: true, isDropdownColumn: true) + #expect(resolver.resolve(context) == .editDropdown) + } + + @Test("editable type-picker column returns editTypePicker") + func editableTypePickerColumnReturnsEditTypePicker() { + let context = ContextFactory.make(value: "TEXT", isTableEditable: true, isTypePickerColumn: true) + #expect(resolver.resolve(context) == .editTypePicker) + } + + @Test("editable enum column returns editEnum") + func editableEnumColumnReturnsEditEnum() { + let context = ContextFactory.make( + value: "small", + columnType: .enumType(rawType: "ENUM", values: ["small", "medium", "large"]), + isTableEditable: true, + hasEnumValues: true + ) + #expect(resolver.resolve(context) == .editEnum) + } + + @Test("editable set column returns editSet") + func editableSetColumnReturnsEditSet() { + let context = ContextFactory.make( + value: "a,b", + columnType: .set(rawType: "SET", values: ["a", "b", "c"]), + isTableEditable: true, + hasEnumValues: true + ) + #expect(resolver.resolve(context) == .editSet) + } + + @Test("dropdown column takes priority over column type") + func dropdownColumnPriorityOverType() { + let context = ContextFactory.make( + value: nil, + columnType: .json(rawType: "JSON"), + isTableEditable: true, + isDropdownColumn: true + ) + #expect(resolver.resolve(context) == .editDropdown) + } +} + +private enum ContextFactory { + static func make( + value: String?, + columnType: ColumnType? = nil, + isTableEditable: Bool = false, + isRowDeleted: Bool = false, + isImmutableColumn: Bool = false, + foreignKeyInfo: ForeignKeyInfo? = nil, + isDropdownColumn: Bool = false, + isTypePickerColumn: Bool = false, + hasEnumValues: Bool = false + ) -> CellContext { + CellContext( + row: 0, + columnIndex: 0, + columnName: "col", + columnType: columnType, + value: value, + isTableEditable: isTableEditable, + isRowDeleted: isRowDeleted, + isImmutableColumn: isImmutableColumn, + foreignKeyInfo: foreignKeyInfo, + isDropdownColumn: isDropdownColumn, + isTypePickerColumn: isTypePickerColumn, + hasEnumValues: hasEnumValues + ) + } +} From b668af456f0037d5f2cff610d41a2128d3d166a6 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 20 May 2026 12:33:21 +0700 Subject: [PATCH 2/7] refactor(datagrid): route chevron action through cell interaction resolver (#1336) --- CHANGELOG.md | 1 + .../Results/CellInteractionResolver.swift | 7 ++-- .../Extensions/DataGridView+Click.swift | 34 +------------------ .../CellInteractionResolverTests.swift | 12 +++++++ 4 files changed, 19 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ce67a19e..58dd36c3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Right-click a column header to copy all its values from the loaded rows (#1325) - Copy as submenu on the row context menu now offers CSV, CSV with Headers, Markdown table, and IN Clause for SQL `WHERE id IN (...)` lookups (#1325) - Double-click or press Return on a query result cell to open a selectable text viewer in the cell. JSON columns open the JSON viewer in a popover, BLOB columns open the hex viewer. The value is selectable and copyable (#1336) +- Double-click or press Return on a boolean, enum, or set cell in an editable table now opens the value picker, matching the chevron button (#1336) ### Changed diff --git a/TablePro/Views/Results/CellInteractionResolver.swift b/TablePro/Views/Results/CellInteractionResolver.swift index c6992d2c3..bef6dce4f 100644 --- a/TablePro/Views/Results/CellInteractionResolver.swift +++ b/TablePro/Views/Results/CellInteractionResolver.swift @@ -55,8 +55,11 @@ internal struct CellInteractionResolver { if context.isDropdownColumn { return .editDropdown } if context.isTypePickerColumn { return .editTypePicker } - if let columnType = context.columnType, context.hasEnumValues { - return columnType.isSetType ? .editSet : .editEnum + if let columnType = context.columnType { + if columnType.isBooleanType { return .editDropdown } + if context.hasEnumValues { + return columnType.isSetType ? .editSet : .editEnum + } } if let foreignKeyInfo = context.foreignKeyInfo { diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index 0521c4dea..c99f5c3d3 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -79,45 +79,13 @@ extension TableViewCoordinator { // MARK: - Chevron Click func handleChevronAction(row: Int, columnIndex: Int) { - guard isEditable else { return } - guard row >= 0, columnIndex >= 0 else { return } - guard !changeManager.isRowDeleted(row) else { return } guard let tableView else { return } guard let column = DataGridView.tableColumnIndex( for: columnIndex, in: tableView, schema: identitySchema ) else { return } - - if let dropdownCols = dropdownColumns, dropdownCols.contains(columnIndex) { - showDropdownMenu(tableView: tableView, row: row, column: column, columnIndex: columnIndex) - return - } - if let typePickerCols = typePickerColumns, typePickerCols.contains(columnIndex) { - showTypePickerPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) - return - } - - let tableRows = tableRowsProvider() - guard columnIndex < tableRows.columnTypes.count, - columnIndex < tableRows.columns.count else { return } - - let ct = tableRows.columnTypes[columnIndex] - let columnName = tableRows.columns[columnIndex] - - if ct.isBooleanType { - showDropdownMenu(tableView: tableView, row: row, column: column, columnIndex: columnIndex) - } else if let values = tableRows.columnEnumValues[columnName], !values.isEmpty { - if ct.isSetType { - showSetPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) - } else { - showEnumPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) - } - } else if ct.isJsonType { - showJSONEditorPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) - } else if ct.isBlobType { - showBlobEditorPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) - } + handleCellInteraction(row: row, tableColumn: column, columnIndex: columnIndex, tableView: tableView) } // MARK: - FK Navigation diff --git a/TableProTests/Views/Results/CellInteractionResolverTests.swift b/TableProTests/Views/Results/CellInteractionResolverTests.swift index 2b0adbcba..09075635e 100644 --- a/TableProTests/Views/Results/CellInteractionResolverTests.swift +++ b/TableProTests/Views/Results/CellInteractionResolverTests.swift @@ -154,6 +154,18 @@ struct CellInteractionResolverEditableTests { #expect(resolver.resolve(context) == .editDropdown) } + @Test("editable boolean column returns editDropdown") + func editableBooleanColumnReturnsEditDropdown() { + let context = ContextFactory.make(value: "true", columnType: .boolean(rawType: "BOOL"), isTableEditable: true) + #expect(resolver.resolve(context) == .editDropdown) + } + + @Test("read-only boolean column returns viewInline") + func readOnlyBooleanColumnReturnsViewInline() { + let context = ContextFactory.make(value: "true", columnType: .boolean(rawType: "BOOL"), isTableEditable: false) + #expect(resolver.resolve(context) == .viewInline(value: "true")) + } + @Test("editable type-picker column returns editTypePicker") func editableTypePickerColumnReturnsEditTypePicker() { let context = ContextFactory.make(value: "TEXT", isTableEditable: true, isTypePickerColumn: true) From 3fd31f23c58d94e88613eecde07dbe282dd9ec91 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 20 May 2026 12:37:36 +0700 Subject: [PATCH 3/7] fix(coordinator): restore CodeEditTextView import dropped in #1340 --- TablePro/TableProApp.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/TablePro/TableProApp.swift b/TablePro/TableProApp.swift index 08bba088a..b22c6add7 100644 --- a/TablePro/TableProApp.swift +++ b/TablePro/TableProApp.swift @@ -5,6 +5,7 @@ // Created by Ngo Quoc Dat on 16/12/25. // +import CodeEditTextView import Combine import Observation import os From 0b0c6a163beee55b0f35aa911aa800229d2196af Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 20 May 2026 12:46:03 +0700 Subject: [PATCH 4/7] fix(datagrid): keep double-click in edit mode, pickers only via chevron (#1336) --- CHANGELOG.md | 3 +- .../Results/CellInteractionResolver.swift | 17 ----- .../Extensions/DataGridView+Click.swift | 48 +++++++++---- .../CellInteractionResolverTests.swift | 67 ++++--------------- 4 files changed, 48 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58dd36c3d..dee8192c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Right-click a column header to copy all its values from the loaded rows (#1325) - Copy as submenu on the row context menu now offers CSV, CSV with Headers, Markdown table, and IN Clause for SQL `WHERE id IN (...)` lookups (#1325) -- Double-click or press Return on a query result cell to open a selectable text viewer in the cell. JSON columns open the JSON viewer in a popover, BLOB columns open the hex viewer. The value is selectable and copyable (#1336) -- Double-click or press Return on a boolean, enum, or set cell in an editable table now opens the value picker, matching the chevron button (#1336) +- Double-click or press Return on a read-only query result cell to open a selectable text viewer in the cell. JSON columns open the JSON viewer in a popover, BLOB columns open the hex viewer. The value is selectable and copyable (#1336) ### Changed diff --git a/TablePro/Views/Results/CellInteractionResolver.swift b/TablePro/Views/Results/CellInteractionResolver.swift index bef6dce4f..32657c1aa 100644 --- a/TablePro/Views/Results/CellInteractionResolver.swift +++ b/TablePro/Views/Results/CellInteractionResolver.swift @@ -15,9 +15,6 @@ internal struct CellContext: Equatable { let isRowDeleted: Bool let isImmutableColumn: Bool let foreignKeyInfo: ForeignKeyInfo? - let isDropdownColumn: Bool - let isTypePickerColumn: Bool - let hasEnumValues: Bool } internal enum CellInteractionMode: Equatable { @@ -30,10 +27,6 @@ internal enum CellInteractionMode: Equatable { case editJson case editBlob case editForeignKey(ForeignKeyInfo) - case editDropdown - case editEnum - case editSet - case editTypePicker case blocked } @@ -52,16 +45,6 @@ internal struct CellInteractionResolver { return .viewInline(value: context.value ?? "NULL") } - if context.isDropdownColumn { return .editDropdown } - if context.isTypePickerColumn { return .editTypePicker } - - if let columnType = context.columnType { - if columnType.isBooleanType { return .editDropdown } - if context.hasEnumValues { - return columnType.isSetType ? .editSet : .editEnum - } - } - if let foreignKeyInfo = context.foreignKeyInfo { return .editForeignKey(foreignKeyInfo) } diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index c99f5c3d3..5b39e9233 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -40,14 +40,6 @@ extension TableViewCoordinator { showBlobEditorPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) case .editForeignKey(let fkInfo): showForeignKeyPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex, fkInfo: fkInfo) - case .editDropdown: - showDropdownMenu(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) - case .editEnum: - showEnumPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) - case .editSet: - showSetPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) - case .editTypePicker: - showTypePickerPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) } } @@ -58,7 +50,6 @@ extension TableViewCoordinator { let columnName = tableRows.columns[columnIndex] let columnType = columnIndex < tableRows.columnTypes.count ? tableRows.columnTypes[columnIndex] : nil let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? [] - let hasEnumValues = (tableRows.columnEnumValues[columnName]?.isEmpty == false) return CellContext( row: row, @@ -69,23 +60,52 @@ extension TableViewCoordinator { isTableEditable: isEditable, isRowDeleted: changeManager.isRowDeleted(row), isImmutableColumn: immutable.contains(columnName), - foreignKeyInfo: tableRows.columnForeignKeys[columnName], - isDropdownColumn: dropdownColumns?.contains(columnIndex) == true, - isTypePickerColumn: typePickerColumns?.contains(columnIndex) == true, - hasEnumValues: hasEnumValues + foreignKeyInfo: tableRows.columnForeignKeys[columnName] ) } // MARK: - Chevron Click func handleChevronAction(row: Int, columnIndex: Int) { + guard isEditable else { return } + guard row >= 0, columnIndex >= 0 else { return } + guard !changeManager.isRowDeleted(row) else { return } guard let tableView else { return } guard let column = DataGridView.tableColumnIndex( for: columnIndex, in: tableView, schema: identitySchema ) else { return } - handleCellInteraction(row: row, tableColumn: column, columnIndex: columnIndex, tableView: tableView) + + if let dropdownCols = dropdownColumns, dropdownCols.contains(columnIndex) { + showDropdownMenu(tableView: tableView, row: row, column: column, columnIndex: columnIndex) + return + } + if let typePickerCols = typePickerColumns, typePickerCols.contains(columnIndex) { + showTypePickerPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) + return + } + + let tableRows = tableRowsProvider() + guard columnIndex < tableRows.columnTypes.count, + columnIndex < tableRows.columns.count else { return } + + let columnType = tableRows.columnTypes[columnIndex] + let columnName = tableRows.columns[columnIndex] + + if columnType.isBooleanType { + showDropdownMenu(tableView: tableView, row: row, column: column, columnIndex: columnIndex) + } else if let values = tableRows.columnEnumValues[columnName], !values.isEmpty { + if columnType.isSetType { + showSetPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) + } else { + showEnumPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) + } + } else if columnType.isJsonType { + showJSONEditorPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) + } else if columnType.isBlobType { + showBlobEditorPopover(tableView: tableView, row: row, column: column, columnIndex: columnIndex) + } } // MARK: - FK Navigation diff --git a/TableProTests/Views/Results/CellInteractionResolverTests.swift b/TableProTests/Views/Results/CellInteractionResolverTests.swift index 09075635e..996d81142 100644 --- a/TableProTests/Views/Results/CellInteractionResolverTests.swift +++ b/TableProTests/Views/Results/CellInteractionResolverTests.swift @@ -148,61 +148,26 @@ struct CellInteractionResolverEditableTests { #expect(resolver.resolve(context) == .editForeignKey(fkInfo)) } - @Test("editable dropdown column returns editDropdown") - func editableDropdownColumnReturnsEditDropdown() { - let context = ContextFactory.make(value: "true", isTableEditable: true, isDropdownColumn: true) - #expect(resolver.resolve(context) == .editDropdown) - } - - @Test("editable boolean column returns editDropdown") - func editableBooleanColumnReturnsEditDropdown() { + @Test("editable boolean column returns editInline, not a picker (pickers are chevron-only)") + func editableBooleanColumnReturnsEditInline() { let context = ContextFactory.make(value: "true", columnType: .boolean(rawType: "BOOL"), isTableEditable: true) - #expect(resolver.resolve(context) == .editDropdown) - } - - @Test("read-only boolean column returns viewInline") - func readOnlyBooleanColumnReturnsViewInline() { - let context = ContextFactory.make(value: "true", columnType: .boolean(rawType: "BOOL"), isTableEditable: false) - #expect(resolver.resolve(context) == .viewInline(value: "true")) - } - - @Test("editable type-picker column returns editTypePicker") - func editableTypePickerColumnReturnsEditTypePicker() { - let context = ContextFactory.make(value: "TEXT", isTableEditable: true, isTypePickerColumn: true) - #expect(resolver.resolve(context) == .editTypePicker) + #expect(resolver.resolve(context) == .editInline(value: "true")) } - @Test("editable enum column returns editEnum") - func editableEnumColumnReturnsEditEnum() { + @Test("editable enum column returns editInline, not a picker") + func editableEnumColumnReturnsEditInline() { let context = ContextFactory.make( value: "small", columnType: .enumType(rawType: "ENUM", values: ["small", "medium", "large"]), - isTableEditable: true, - hasEnumValues: true + isTableEditable: true ) - #expect(resolver.resolve(context) == .editEnum) + #expect(resolver.resolve(context) == .editInline(value: "small")) } - @Test("editable set column returns editSet") - func editableSetColumnReturnsEditSet() { - let context = ContextFactory.make( - value: "a,b", - columnType: .set(rawType: "SET", values: ["a", "b", "c"]), - isTableEditable: true, - hasEnumValues: true - ) - #expect(resolver.resolve(context) == .editSet) - } - - @Test("dropdown column takes priority over column type") - func dropdownColumnPriorityOverType() { - let context = ContextFactory.make( - value: nil, - columnType: .json(rawType: "JSON"), - isTableEditable: true, - isDropdownColumn: true - ) - #expect(resolver.resolve(context) == .editDropdown) + @Test("read-only boolean column returns viewInline") + func readOnlyBooleanColumnReturnsViewInline() { + let context = ContextFactory.make(value: "true", columnType: .boolean(rawType: "BOOL"), isTableEditable: false) + #expect(resolver.resolve(context) == .viewInline(value: "true")) } } @@ -213,10 +178,7 @@ private enum ContextFactory { isTableEditable: Bool = false, isRowDeleted: Bool = false, isImmutableColumn: Bool = false, - foreignKeyInfo: ForeignKeyInfo? = nil, - isDropdownColumn: Bool = false, - isTypePickerColumn: Bool = false, - hasEnumValues: Bool = false + foreignKeyInfo: ForeignKeyInfo? = nil ) -> CellContext { CellContext( row: 0, @@ -227,10 +189,7 @@ private enum ContextFactory { isTableEditable: isTableEditable, isRowDeleted: isRowDeleted, isImmutableColumn: isImmutableColumn, - foreignKeyInfo: foreignKeyInfo, - isDropdownColumn: isDropdownColumn, - isTypePickerColumn: isTypePickerColumn, - hasEnumValues: hasEnumValues + foreignKeyInfo: foreignKeyInfo ) } } From cc38ddb073219b65de4cca5d597e2800657de083 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 20 May 2026 12:51:46 +0700 Subject: [PATCH 5/7] fix(datagrid): edit FK cell inline on double-click, remove FK picker popover (#1336) --- .../Results/CellInteractionResolver.swift | 6 - .../Extensions/DataGridView+Click.swift | 5 +- .../Extensions/DataGridView+Popovers.swift | 25 --- .../ForeignKeyPopoverContentView.swift | 191 ------------------ .../CellInteractionResolverTests.swift | 44 +--- 5 files changed, 7 insertions(+), 264 deletions(-) delete mode 100644 TablePro/Views/Results/ForeignKeyPopoverContentView.swift diff --git a/TablePro/Views/Results/CellInteractionResolver.swift b/TablePro/Views/Results/CellInteractionResolver.swift index 32657c1aa..e10843f04 100644 --- a/TablePro/Views/Results/CellInteractionResolver.swift +++ b/TablePro/Views/Results/CellInteractionResolver.swift @@ -14,7 +14,6 @@ internal struct CellContext: Equatable { let isTableEditable: Bool let isRowDeleted: Bool let isImmutableColumn: Bool - let foreignKeyInfo: ForeignKeyInfo? } internal enum CellInteractionMode: Equatable { @@ -26,7 +25,6 @@ internal enum CellInteractionMode: Equatable { case editOverlay(value: String) case editJson case editBlob - case editForeignKey(ForeignKeyInfo) case blocked } @@ -45,10 +43,6 @@ internal struct CellInteractionResolver { return .viewInline(value: context.value ?? "NULL") } - if let foreignKeyInfo = context.foreignKeyInfo { - return .editForeignKey(foreignKeyInfo) - } - if let columnType = context.columnType { if columnType.isBlobType { return .editBlob } if columnType.isJsonType { return .editJson } diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index 5b39e9233..8f6cf1598 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -38,8 +38,6 @@ extension TableViewCoordinator { showJSONEditorPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) case .editBlob: showBlobEditorPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex) - case .editForeignKey(let fkInfo): - showForeignKeyPopover(tableView: tableView, row: row, column: tableColumn, columnIndex: columnIndex, fkInfo: fkInfo) } } @@ -59,8 +57,7 @@ extension TableViewCoordinator { value: cellValue(at: row, column: columnIndex), isTableEditable: isEditable, isRowDeleted: changeManager.isRowDeleted(row), - isImmutableColumn: immutable.contains(columnName), - foreignKeyInfo: tableRows.columnForeignKeys[columnName] + isImmutableColumn: immutable.contains(columnName) ) } diff --git a/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift b/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift index 723997673..7f212ed11 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift @@ -24,31 +24,6 @@ extension TableViewCoordinator { return displayRow.values[columnIndex] } - func showForeignKeyPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int, fkInfo: ForeignKeyInfo) { - let currentValue = cellValue(at: row, column: columnIndex) - - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } - guard let databaseType, let connectionId else { return } - - let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) - PopoverPresenter.show( - relativeTo: cellRect, - of: tableView, - contentSize: NSSize(width: 420, height: 320) - ) { [weak self] dismiss in - ForeignKeyPopoverContentView( - currentValue: currentValue, - fkInfo: fkInfo, - connectionId: connectionId, - databaseType: databaseType, - onCommit: { newValue in - self?.commitPopoverEdit(row: row, columnIndex: columnIndex, newValue: newValue) - }, - onDismiss: dismiss - ) - } - } - func toggleForeignKeyPreview(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { if let popover = activeFKPreviewPopover, popover.isShown { popover.close() diff --git a/TablePro/Views/Results/ForeignKeyPopoverContentView.swift b/TablePro/Views/Results/ForeignKeyPopoverContentView.swift deleted file mode 100644 index d328c1ca1..000000000 --- a/TablePro/Views/Results/ForeignKeyPopoverContentView.swift +++ /dev/null @@ -1,191 +0,0 @@ -// -// ForeignKeyPopoverContentView.swift -// TablePro -// -// SwiftUI popover content for searchable foreign key column editing. -// - -import os -import SwiftUI -import TableProPluginKit - -struct ForeignKeyPopoverContentView: View { - let currentValue: String? - let fkInfo: ForeignKeyInfo - let connectionId: UUID - let databaseType: DatabaseType - let onCommit: (String) -> Void - let onDismiss: () -> Void - - @State private var searchText = "" - @State private var allValues: [FKValue] = [] - @State private var selectedId: String? - @State private var isLoading = true - - private static let logger = Logger(subsystem: "com.TablePro", category: "FKPopover") - private static let maxFetchRows = 1_000 - private static let rowHeight: CGFloat = 24 - private static let searchAreaHeight: CGFloat = 44 - private static let maxHeight: CGFloat = 320 - - private var filteredValues: [FKValue] { - let query = searchText.lowercased() - if query.isEmpty { return allValues } - return allValues.filter { $0.display.lowercased().contains(query) } - } - - private var listHeight: CGFloat { - let contentHeight = CGFloat(filteredValues.count) * Self.rowHeight - return min(contentHeight, Self.maxHeight - Self.searchAreaHeight) - } - - var body: some View { - VStack(spacing: 0) { - NativeSearchField(text: $searchText, placeholder: String(localized: "Search...")) - .padding(.horizontal, 8) - .padding(.vertical, 8) - - Divider() - - if isLoading { - ProgressView() - .frame(maxWidth: .infinity, alignment: .center) - .frame(height: 60) - } else if filteredValues.isEmpty { - Text("No values found") - .foregroundStyle(.secondary) - .font(.callout) - .frame(maxWidth: .infinity, alignment: .leading) - .frame(height: 60) - } else { - List(filteredValues, selection: $selectedId) { value in - Button { - onCommit(value.id) - onDismiss() - } label: { - rowLabel(for: value) - .frame(maxWidth: .infinity, alignment: .leading) - } - .buttonStyle(.plain) - .listRowInsets(EdgeInsets( - top: 2, leading: 6, bottom: 2, trailing: 6 - )) - } - .listStyle(.plain) - .environment(\.defaultMinListRowHeight, Self.rowHeight) - .frame(height: listHeight) - .onKeyPress(.return) { - guard let id = selectedId else { return .ignored } - onCommit(id) - onDismiss() - return .handled - } - } - } - .frame(width: 420) - .fixedSize(horizontal: false, vertical: true) - .task { await fetchForeignKeyValues() } - .onChange(of: searchText) { - selectedId = filteredValues.first?.id - } - } - - // MARK: - Row View - - @ViewBuilder - private func rowLabel(for value: FKValue) -> some View { - if value.id == currentValue { - Text(value.display) - .font(.system(.callout, design: .monospaced)) - .foregroundStyle(.tint) - .lineLimit(1) - .truncationMode(.tail) - } else { - Text(value.display) - .font(.system(.callout, design: .monospaced)) - .foregroundStyle(.primary) - .lineLimit(1) - .truncationMode(.tail) - } - } - - // MARK: - Data Fetching - - private func fetchForeignKeyValues() async { - guard let driver = DatabaseManager.shared.driver(for: connectionId) else { - Self.logger.error("No active driver for FK lookup") - isLoading = false - return - } - - let quotedTable: String - if let schema = fkInfo.referencedSchema { - quotedTable = "\(driver.quoteIdentifier(schema)).\(driver.quoteIdentifier(fkInfo.referencedTable))" - } else { - quotedTable = driver.quoteIdentifier(fkInfo.referencedTable) - } - let quotedColumn = driver.quoteIdentifier(fkInfo.referencedColumn) - - var displayColumn: String? - do { - let columnInfos = try await driver.fetchColumns(table: fkInfo.referencedTable, schema: fkInfo.referencedSchema) - displayColumn = columnInfos.first(where: { col in - col.name != fkInfo.referencedColumn && - !col.isPrimaryKey && - isTextLikeType(col.dataType) - })?.name - } catch { - Self.logger.debug("Could not fetch columns for display: \(error.localizedDescription)") - } - - let query: String - let limitSuffix: String - switch PluginManager.shared.paginationStyle(for: databaseType) { - case .offsetFetch: - limitSuffix = "OFFSET 0 ROWS FETCH NEXT \(Self.maxFetchRows) ROWS ONLY" - case .limit: - limitSuffix = "LIMIT \(Self.maxFetchRows)" - } - if let displayCol = displayColumn { - let quotedDisplay = driver.quoteIdentifier(displayCol) - query = "SELECT \(quotedColumn), \(quotedDisplay) FROM \(quotedTable) ORDER BY \(quotedColumn) \(limitSuffix)" - } else { - query = "SELECT DISTINCT \(quotedColumn) FROM \(quotedTable) ORDER BY \(quotedColumn) \(limitSuffix)" - } - - do { - let result = try await driver.execute(query: query) - var values: [FKValue] = [] - for row in result.rows { - guard !row.isEmpty, let idVal = row[0].asText else { continue } - let displayVal: String - if displayColumn != nil, row.count > 1, let second = row[1].asText { - displayVal = "\(idVal), \(second)" - } else { - displayVal = idVal - } - values.append(FKValue(id: idVal, display: displayVal)) - } - allValues = values - selectedId = currentValue - } catch { - Self.logger.error("FK value fetch failed: \(error.localizedDescription)") - } - - isLoading = false - } - - // MARK: - Helpers - - private func isTextLikeType(_ typeString: String) -> Bool { - let upper = typeString.uppercased() - return upper.contains("CHAR") || upper.contains("TEXT") || upper.contains("NAME") - } -} - -// MARK: - FK Value Model - -private struct FKValue: Identifiable, Hashable { - let id: String - let display: String -} diff --git a/TableProTests/Views/Results/CellInteractionResolverTests.swift b/TableProTests/Views/Results/CellInteractionResolverTests.swift index 996d81142..cec97c018 100644 --- a/TableProTests/Views/Results/CellInteractionResolverTests.swift +++ b/TableProTests/Views/Results/CellInteractionResolverTests.swift @@ -71,18 +71,6 @@ struct CellInteractionResolverReadOnlyTests { #expect(resolver.resolve(context) == .viewJson) } - @Test("read-only FK column falls through to viewInline") - func readOnlyForeignKeyReturnsViewInline() { - let fkInfo = ForeignKeyInfo(name: "fk", column: "uid", referencedTable: "u", referencedColumn: "id") - let context = ContextFactory.make( - value: "42", - columnType: .integer(rawType: "INT"), - isTableEditable: false, - foreignKeyInfo: fkInfo - ) - #expect(resolver.resolve(context) == .viewInline(value: "42")) - } - @Test("read-only JSON-looking plain text without columnType returns viewInline, not viewJson") func readOnlyJsonLikeTextWithoutTypeReturnsViewInline() { let context = ContextFactory.make(value: #"{"k":1}"#, columnType: nil, isTableEditable: false) @@ -124,28 +112,10 @@ struct CellInteractionResolverEditableTests { #expect(resolver.resolve(context) == .editBlob) } - @Test("editable foreign key column returns editForeignKey") - func editableForeignKeyReturnsEditForeignKey() { - let fkInfo = ForeignKeyInfo(name: "fk_users", column: "user_id", referencedTable: "users", referencedColumn: "id") - let context = ContextFactory.make( - value: "1", - columnType: .integer(rawType: "INT"), - isTableEditable: true, - foreignKeyInfo: fkInfo - ) - #expect(resolver.resolve(context) == .editForeignKey(fkInfo)) - } - - @Test("editable FK column with nil columnType still returns editForeignKey") - func editableForeignKeyNilColumnTypeReturnsEditForeignKey() { - let fkInfo = ForeignKeyInfo(name: "fk", column: "uid", referencedTable: "u", referencedColumn: "id") - let context = ContextFactory.make( - value: "1", - columnType: nil, - isTableEditable: true, - foreignKeyInfo: fkInfo - ) - #expect(resolver.resolve(context) == .editForeignKey(fkInfo)) + @Test("editable foreign key column returns editInline (FK popover is not opened by double-click)") + func editableForeignKeyReturnsEditInline() { + let context = ContextFactory.make(value: "1", columnType: .integer(rawType: "INT"), isTableEditable: true) + #expect(resolver.resolve(context) == .editInline(value: "1")) } @Test("editable boolean column returns editInline, not a picker (pickers are chevron-only)") @@ -177,8 +147,7 @@ private enum ContextFactory { columnType: ColumnType? = nil, isTableEditable: Bool = false, isRowDeleted: Bool = false, - isImmutableColumn: Bool = false, - foreignKeyInfo: ForeignKeyInfo? = nil + isImmutableColumn: Bool = false ) -> CellContext { CellContext( row: 0, @@ -188,8 +157,7 @@ private enum ContextFactory { value: value, isTableEditable: isTableEditable, isRowDeleted: isRowDeleted, - isImmutableColumn: isImmutableColumn, - foreignKeyInfo: foreignKeyInfo + isImmutableColumn: isImmutableColumn ) } } From adff512575e5c573a70b5b705e415d1d3f678270 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 20 May 2026 12:54:32 +0700 Subject: [PATCH 6/7] fix(datagrid): allow inline editing of foreign key cells (#1336) --- TablePro/Views/Results/Extensions/DataGridView+Editing.swift | 3 --- 1 file changed, 3 deletions(-) diff --git a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift index 611cd902a..186e9c85e 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift @@ -22,9 +22,6 @@ extension TableViewCoordinator { let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? [] if immutable.contains(tableRows.columns[columnIndex]) { return .blocked } - let columnName = tableRows.columns[columnIndex] - if tableRows.columnForeignKeys[columnName] != nil { return .blocked } - if columnIndex < tableRows.columnTypes.count { let ct = tableRows.columnTypes[columnIndex] if ct.isJsonType || ct.isBlobType { From 76d64bb3384316a83a4695475590bd883cf9c756 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 20 May 2026 12:59:09 +0700 Subject: [PATCH 7/7] refactor(datagrid): trim CellContext to decision-relevant fields (#1336) --- TablePro/Views/Results/CellInteractionResolver.swift | 3 --- TablePro/Views/Results/Extensions/DataGridView+Click.swift | 3 --- TableProTests/Views/Results/CellInteractionResolverTests.swift | 3 --- 3 files changed, 9 deletions(-) diff --git a/TablePro/Views/Results/CellInteractionResolver.swift b/TablePro/Views/Results/CellInteractionResolver.swift index e10843f04..1812195d8 100644 --- a/TablePro/Views/Results/CellInteractionResolver.swift +++ b/TablePro/Views/Results/CellInteractionResolver.swift @@ -6,9 +6,6 @@ import Foundation internal struct CellContext: Equatable { - let row: Int - let columnIndex: Int - let columnName: String let columnType: ColumnType? let value: String? let isTableEditable: Bool diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index 8f6cf1598..d1f560372 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -50,9 +50,6 @@ extension TableViewCoordinator { let immutable = databaseType.map { PluginManager.shared.immutableColumns(for: $0) } ?? [] return CellContext( - row: row, - columnIndex: columnIndex, - columnName: columnName, columnType: columnType, value: cellValue(at: row, column: columnIndex), isTableEditable: isEditable, diff --git a/TableProTests/Views/Results/CellInteractionResolverTests.swift b/TableProTests/Views/Results/CellInteractionResolverTests.swift index cec97c018..042e4e882 100644 --- a/TableProTests/Views/Results/CellInteractionResolverTests.swift +++ b/TableProTests/Views/Results/CellInteractionResolverTests.swift @@ -150,9 +150,6 @@ private enum ContextFactory { isImmutableColumn: Bool = false ) -> CellContext { CellContext( - row: 0, - columnIndex: 0, - columnName: "col", columnType: columnType, value: value, isTableEditable: isTableEditable,