Skip to content

fix(datagrid): restore each row's own value when a row inspector field they disagree on is cleared - #2867

Merged
datlechin merged 1 commit into
mainfrom
fix/inspector-multivalue-clear
Sep 15, 2026
Merged

datlechin merged 1 commit into
mainfrom
fix/inspector-multivalue-clear

Conversation

@datlechin

Copy link
Copy Markdown
Member

Found while reviewing #2858, which introduced the first defect below.

Clearing a field the selected rows disagree on wrote NULL into all of them

Select two rows whose name differs, type something into the inspector's name field, then clear it. Every selected row is staged and painted NULL, while the field itself reports no pending edit and reads "Multiple values" again.

MultiRowEditState.configure sets originalValue to nil for two different situations: a stored NULL, and a selection whose rows do not agree, which it records in hasMultipleValues instead. updateField sent pending ?? original as one value, so clearing a disagreeing field sent .null.

Before #2858 this cancelled itself: the inspector passed one shared oldValue taken from the same nil, so PendingChanges.recordCellChange saw oldValue == newValue and rolled the change back. #2858 made oldValue per row and read it from the live buffer, which is correct and which is what made the NULL survive.

A field with no value of its own now asks for each row's own value back instead, through onFieldReverted and RowEditingCoordinator.revertInspectorFieldEdit. The values come from what the inspector was configured with, not from the change manager's originals: a row that already carried a pending grid edit reverts to that edit, not past it. Single-valued fields keep the old path, so a stored NULL, a deliberate Set NULL and the schema inspector are untouched.

Discard Changes left the inspector showing the discarded values

restoreRowBufferToOriginals rewrites TableRows and repaints the grid, but moves nothing InspectorTrigger watches, so the pane went on showing what was just thrown away. It now bumps the same revision handleUndoResult does.

Its repaint was also positional: editMany reports the rows it changed by storage position and the grid reads a delta's rows as display positions, so under a sort or a value filter a discard invalidated and redrew the wrong cells. Resolved through DisplayRowMapping now, the way the inspector's own staging already does.

Verification

  • verify.sh build PASS.
  • verify.sh test PASS: 126 cases over InspectorFieldEditStagingTests, MultiRowEditStateTests, MultiRowEditStateJsonTests, FieldValueStateTests, RowEditingCoordinatorValueFilterTests, DataChangeManagerTests.
  • swiftlint --strict clean on the changed files.
  • New cases drive a real MultiRowEditState wired the way the inspector wires it: clearing a disagreeing field restores both rows and stages nothing, clearing an agreeing field still sends the stored value, and an explicit Set NULL over disagreeing rows still writes NULL to both.

@datlechin
datlechin merged commit 5b39212 into main Sep 15, 2026
8 checks passed
@datlechin
datlechin deleted the fix/inspector-multivalue-clear branch September 15, 2026 01: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