diff --git a/CHANGELOG.md b/CHANGELOG.md index 38e27769ec..fa49407028 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - NULL written into every selected row when a field they disagree on was cleared in the row inspector. - Row inspector showing the discarded values after Discard Changes. - One undo step per character when typing in a row inspector field. +- 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 f32e7f0438..04d1fbb10a 100644 --- a/TablePro/Core/Coordinators/RowEditingCoordinator.swift +++ b/TablePro/Core/Coordinators/RowEditingCoordinator.swift @@ -250,6 +250,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 714b173c71..371e6fe8dc 100644 --- a/TablePro/Models/UI/MultiRowEditState.swift +++ b/TablePro/Models/UI/MultiRowEditState.swift @@ -86,6 +86,10 @@ final class MultiRowEditState { /// send, so it asks for each row's own configured value instead. var onFieldReverted: ((Int, [RowID: 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. @@ -283,6 +287,20 @@ final class MultiRowEditState { return values } + /// 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) @@ -359,6 +377,7 @@ final class MultiRowEditState { fields = [] onFieldChanged = nil onFieldReverted = 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 26bb7f41d1..2125c5eb34 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift @@ -242,6 +242,16 @@ extension MainContentView { trailingPaneState.inspector.editState.onFieldReverted = { columnIndex, valuesByRow in capturedCoordinator.revertInspectorFieldEdit(columnIndex: columnIndex, valuesByRow: valuesByRow) } + trailingPaneState.inspector.editState.onDetachedFieldChanged = { columnIndex, newValue, rowIDs in + /// A value window commits on every keystroke exactly as the field it detached from + /// does, so its typing folds into one undo step the same way. + capturedCoordinator.stageInspectorFieldEdit( + columnIndex: columnIndex, + value: newValue, + rowIDs: rowIDs, + continuity: .typing + ) + } } /// The per-column display formats the grid is applying, in column order. @@ -290,6 +300,7 @@ extension MainContentView { private func clearSidebarEditHandlers() { trailingPaneState.inspector.editState.onFieldChanged = nil trailingPaneState.inspector.editState.onFieldReverted = nil + trailingPaneState.inspector.editState.onDetachedFieldChanged = nil } /// Populate the inspector from the grid that owns a schema selection, and send every @@ -312,6 +323,7 @@ extension MainContentView { let capturedCoordinator = coordinator trailingPaneState.inspector.editState.onFieldReverted = nil + trailingPaneState.inspector.editState.onDetachedFieldChanged = nil trailingPaneState.inspector.editState.onFieldChanged = { fieldIndex, newValue, _ in capturedCoordinator.inspectorRowSource?.commitInspectorField( displayRow: displayRow, 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..b2d32d5c2c --- /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 = { columnIndex, value, _ in fieldEdits.append((columnIndex, value)) } + 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) + } +}