Skip to content

fix(datagrid): keep a detached row inspector value window writing the rows it was opened for - #2870

Merged
datlechin merged 3 commits into
mainfrom
fix/inspector-popout-commit
Sep 15, 2026
Merged

datlechin merged 3 commits into
mainfrom
fix/inspector-popout-commit

Conversation

@datlechin

Copy link
Copy Markdown
Member

Two defects found while investigating #2851, both about the value window a row inspector field detaches into.

Text typed into the window was dropped once another row was selected

RowInspectorView.popOut built its commit closure around the field's UUID and looked the field up on every keystroke. MultiRowEditState.configure reissues those ids whenever the selection changes, and reissues one when an inline grid edit changes that cell's value, so after either the lookup failed and the closure returned. Nothing was recorded, nothing was shown, and nothing said so, while docs/features/json-viewer.mdx promises those edits join the same pending changes as the field.

The window now captures the column and the rows it was opened for. While that selection is still the one on screen the commit is an ordinary field edit; once it has moved the value goes to the rows the window was opened for rather than into whatever is selected now, which #2858 made possible by keying the staging on RowID.

The JSON rendering stayed on the row as it was

The inspector rebuilds RowInspectorContext when its Fields/JSON control changes, which covers editing in Fields and then switching to JSON. It does not cover a value window that is still open and still writing while JSON is the rendering on screen: the edit reaches TableRows and the grid, and moves nothing InspectorTrigger watches.

stageInspectorFieldEdit now bumps inspectorRowContentRevision, and the inspector rebuilds its context from it while JSON is showing. Debounced on its own task slot, because the window commits per keystroke and JSONRowInspectorViewModel.update cancels the reader's fetches on every content change. The field list is deliberately not rebuilt: a rebuilt field takes a new id, which would drop first responder out of the window being typed into.

Verification

  • verify.sh build PASS.
  • verify.sh test PASS: 84 cases over MultiRowEditStateDetachedCommitTests, MultiRowEditStateTests, InspectorFieldEditStagingTests, MultiRowEditStateJsonTests.
  • swiftlint --strict clean on the changed files.
  • New suite: a commit while the same rows are selected stays an ordinary field edit; a commit after the selection moved writes the rows the window was opened for and leaves the new selection's field alone; a commit naming no rows does nothing.
  • No UI test: the value window is a second NSWindow the runner has to target before typing, and the row behind it can only be selected through an offset click on the grid. Noted rather than written, per rule 4.

…commit

# Conflicts:
#	CHANGELOG.md
#	TablePro/Models/UI/MultiRowEditState.swift
#	TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift
…commit

# Conflicts:
#	CHANGELOG.md
#	TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift
@datlechin
datlechin merged commit 3f2bf37 into main Sep 15, 2026
5 of 7 checks passed
@datlechin
datlechin deleted the fix/inspector-popout-commit branch September 15, 2026 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant