From e6a35351ea65342951b00e5899986487f55dc403 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 15 Sep 2026 09:13:52 +0700 Subject: [PATCH] fix(datagrid): keep a detached row inspector value window writing the rows it was opened for --- CHANGELOG.md | 2 + .../Coordinators/RowEditingCoordinator.swift | 1 + TablePro/Models/UI/MultiRowEditState.swift | 19 +++++ .../MainContentView+EventHandlers.swift | 3 + .../Extensions/MainContentView+Helpers.swift | 10 +++ .../Views/Main/MainContentCoordinator.swift | 5 ++ TablePro/Views/Main/MainContentView.swift | 7 ++ .../Views/RowInspector/RowInspectorView.swift | 9 ++- ...MultiRowEditStateDetachedCommitTests.swift | 77 +++++++++++++++++++ 9 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 TableProTests/Models/UI/MultiRowEditStateDetachedCommitTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d4b9654f6..bfee38882e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Stutter when scrolling a very long line in the SQL editor. - Row inspector edits missing from the grid cell and gone from the inspector on reselect. (#2851) +- Text typed into a detached row inspector value window silently dropped once another row was selected. +- Row inspector's JSON view showing the row as it was while a detached value window was still writing it. - Empty Procedures and Functions lists on every SQL Server connection. - SQL Server rows that could not be saved on a table with a filtered index or an index on a computed column. - SQL Server CLR and extended procedures and functions missing from the Procedures and Functions lists. diff --git a/TablePro/Core/Coordinators/RowEditingCoordinator.swift b/TablePro/Core/Coordinators/RowEditingCoordinator.swift index 690e5196e0..c1b4ae6f43 100644 --- a/TablePro/Core/Coordinators/RowEditingCoordinator.swift +++ b/TablePro/Core/Coordinators/RowEditingCoordinator.swift @@ -205,6 +205,7 @@ final class RowEditingCoordinator { parent.mutateActiveTableRows(for: tabId) { rows in rows.editMany(edits) } parent.tabManager.mutate(at: tabIndex) { $0.hasUserInteraction = true } repaintInspectorEdit(rowIDs: editedRowIDs, columnIndex: columnIndex, in: tableRows) + parent.inspectorRowContentRevision &+= 1 } /// `editMany` reports the rows it changed by their position in storage, and the grid reads a diff --git a/TablePro/Models/UI/MultiRowEditState.swift b/TablePro/Models/UI/MultiRowEditState.swift index 06fadc2fa7..56d521a109 100644 --- a/TablePro/Models/UI/MultiRowEditState.swift +++ b/TablePro/Models/UI/MultiRowEditState.swift @@ -75,6 +75,10 @@ final class MultiRowEditState { var onFieldChanged: ((Int, PluginCellValue) -> Void)? + /// A value window still open over a selection that has moved on. It names the rows it was + /// opened for, because the fields it was opened from are gone. + var onDetachedFieldChanged: ((Int, PluginCellValue, [RowID]) -> Void)? + private(set) var selectedRowIndices: Set = [] /// The rows an edit is staged against, captured when the selection was configured. @@ -252,6 +256,20 @@ final class MultiRowEditState { } } + /// A commit from a detached value window, which outlives the selection it was opened from. + /// + /// While that selection is still the one on screen this is an ordinary field edit. Once it has + /// moved the fields no longer describe those rows, so the value goes straight to the rows the + /// window was opened for rather than into whatever is selected now. + func updateDetachedField(columnIndex: Int, rowIDs: [RowID], value: String?) { + guard !rowIDs.isEmpty else { return } + if self.rowIDs == rowIDs, fields.indices.contains(columnIndex) { + updateField(at: columnIndex, value: value) + return + } + onDetachedFieldChanged?(columnIndex, PluginCellValue.fromOptional(value), rowIDs) + } + private static func resolvePendingValue(_ value: String?, original: String?, isJson: Bool) -> String? { if isJson, let value, !value.isEmpty { let normalized = JsonReindenter.normalize(value) @@ -327,6 +345,7 @@ final class MultiRowEditState { func releaseData() { fields = [] onFieldChanged = nil + onDetachedFieldChanged = nil selectedRowIndices = [] rowIDs = [] allRows = [] diff --git a/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift b/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift index 85ff59812c..c2c9e1d8ed 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift @@ -234,6 +234,9 @@ extension MainContentView { rowIDs: capturedEditState.rowIDs ) } + trailingPaneState.inspector.editState.onDetachedFieldChanged = { columnIndex, newValue, rowIDs in + capturedCoordinator.stageInspectorFieldEdit(columnIndex: columnIndex, value: newValue, rowIDs: rowIDs) + } } /// The per-column display formats the grid is applying, in column order. diff --git a/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift b/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift index fa81b07f0a..620825a768 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift @@ -64,6 +64,16 @@ extension MainContentView { } } + func scheduleInspectorContextRefresh() { + guard trailingPaneState.inspector.viewMode == .json else { return } + inspectorContextRefreshTask?.cancel() + inspectorContextRefreshTask = Task { @MainActor in + try? await Task.sleep(for: .milliseconds(50)) + guard !Task.isCancelled else { return } + updateInspectorContext() + } + } + func updateInspectorContext() { trailingPaneState.inspector.context = RowInspectorContext( subject: inspectorSubject, diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index bbbb439374..db6c308fae 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -226,6 +226,11 @@ final class MainContentCoordinator { /// AppKit object reached through observation-ignored hops, so it cannot invalidate a view. var gridDisplayRevision: Int = 0 + /// Bumped when an inspector edit rewrites the selected row's values, so the inspector's JSON + /// rendering re-reads the row. Apart from `gridDisplayRevision`, which drives a full rebuild of + /// the field list and takes first responder out of whatever is being typed into. + var inspectorRowContentRevision: Int = 0 + /// dispatch insertRows/removeRows directly to the NSTableView via DataGridViewDelegate. @ObservationIgnored weak var dataTabDelegate: DataTabGridDelegate? diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index 1ad56eb0dd..338b764c2b 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -54,6 +54,7 @@ struct MainContentView: View { @State var commandActions: MainContentCommandActions? @State var queryResultsSummaryCache: (tabId: UUID, version: Int, summary: String?)? @State var inspectorUpdateTask: Task? + @State var inspectorContextRefreshTask: Task? /// Stable identifier for this window in WindowLifecycleMonitor @State var windowId = UUID() @State var hasInitialized = false @@ -316,6 +317,12 @@ struct MainContentView: View { .onChange(of: trailingPaneState.inspector.viewMode) { updateInspectorContext() } + /// A value window detached from a field goes on writing while the JSON rendering is the + /// one on screen, and it moves nothing the trigger above watches. Debounced, because it + /// commits per keystroke and rebuilding the JSON tree cancels the reader's fetches. + .onChange(of: coordinator.inspectorRowContentRevision) { + scheduleInspectorContextRefresh() + } .onAppear { let start = Date() Self.lifecycleLogger.info( diff --git a/TablePro/Views/RowInspector/RowInspectorView.swift b/TablePro/Views/RowInspector/RowInspectorView.swift index 23acd936cc..f0b24f0d64 100644 --- a/TablePro/Views/RowInspector/RowInspectorView.swift +++ b/TablePro/Views/RowInspector/RowInspectorView.swift @@ -103,11 +103,14 @@ internal struct RowInspectorView: View { /// no competitor ships an in-panel takeover and neither does this any more. private func popOut(field: FieldEditState, text: String, kind: FieldEditorKind) { let isEditable = context.isEditable && !context.isRowDeleted && !field.isServerOwned - let fieldID = field.id + /// Captured here rather than looked up on each keystroke: the field's id is reissued on + /// every selection change, so a window left open over a new selection used to fail its own + /// lookup and drop everything typed into it without a word. + let columnIndex = field.columnIndex + let rowIDs = state.editState.rowIDs let commit: ((String) -> Void)? = isEditable ? { [editState = state.editState] newValue in - guard let current = editState.fields.first(where: { $0.id == fieldID }) else { return } - editState.updateField(at: current.columnIndex, value: newValue) + editState.updateDetachedField(columnIndex: columnIndex, rowIDs: rowIDs, value: newValue) } : nil diff --git a/TableProTests/Models/UI/MultiRowEditStateDetachedCommitTests.swift b/TableProTests/Models/UI/MultiRowEditStateDetachedCommitTests.swift new file mode 100644 index 0000000000..aea45d7ea9 --- /dev/null +++ b/TableProTests/Models/UI/MultiRowEditStateDetachedCommitTests.swift @@ -0,0 +1,77 @@ +// +// MultiRowEditStateDetachedCommitTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +@testable import TablePro + +@Suite("A detached value window writes the rows it was opened for") +@MainActor +struct MultiRowEditStateDetachedCommitTests { + private func makeState(rowIDs: [RowID], values: [[String?]]) -> MultiRowEditState { + let state = MultiRowEditState() + state.configure( + selectedRowIndices: Set(rowIDs.indices), + rowIDs: rowIDs, + allRows: values, + columns: ["id", "name"], + columnTypes: [.text(rawType: nil), .text(rawType: nil)] + ) + return state + } + + @Test("a commit while the same rows are selected is an ordinary field edit") + func commitsThroughTheFieldWhileTheSelectionHolds() { + let state = makeState(rowIDs: [.existing(1)], values: [["2", "Bob"]]) + var fieldEdits: [(Int, PluginCellValue)] = [] + var detached: [(Int, PluginCellValue, [RowID])] = [] + state.onFieldChanged = { fieldEdits.append(($0, $1)) } + state.onDetachedFieldChanged = { detached.append(($0, $1, $2)) } + + state.updateDetachedField(columnIndex: 1, rowIDs: [.existing(1)], value: "Zed") + + #expect(fieldEdits.count == 1) + #expect(detached.isEmpty) + #expect(state.fields[1].pendingValue == "Zed") + } + + /// The field's id is reissued on every selection change, so the lookup this replaced failed and + /// dropped the text with no warning. + @Test("a commit after the selection moved writes the rows the window was opened for") + func commitsToTheOpenedRowsAfterTheSelectionMoved() { + let state = makeState(rowIDs: [.existing(1)], values: [["2", "Bob"]]) + var detached: [(Int, PluginCellValue, [RowID])] = [] + state.onFieldChanged = { _, _ in Issue.record("the field path must not take a moved selection") } + state.onDetachedFieldChanged = { detached.append(($0, $1, $2)) } + + state.configure( + selectedRowIndices: [0], + rowIDs: [.existing(2)], + allRows: [["3", "Carol"]], + columns: ["id", "name"], + columnTypes: [.text(rawType: nil), .text(rawType: nil)] + ) + state.updateDetachedField(columnIndex: 1, rowIDs: [.existing(1)], value: "Zed") + + #expect(detached.count == 1) + #expect(detached.first?.2 == [.existing(1)]) + #expect(detached.first?.1 == .text("Zed")) + #expect(state.fields[1].pendingValue == nil) + } + + @Test("a commit naming no rows does nothing") + func commitWithoutRowsDoesNothing() { + let state = makeState(rowIDs: [.existing(1)], values: [["2", "Bob"]]) + var detached = 0 + state.onDetachedFieldChanged = { _, _, _ in detached += 1 } + + state.updateDetachedField(columnIndex: 1, rowIDs: [], value: "Zed") + + #expect(detached == 0) + #expect(state.fields[1].pendingValue == nil) + } +}