Repository navigation
fix(datagrid): restore each row's own value when a row inspector field they disagree on is cleared - #2867
Merged
Conversation
…d they disagree on is cleared
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
namediffers, type something into the inspector'snamefield, 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.configuresetsoriginalValueto nil for two different situations: a stored NULL, and a selection whose rows do not agree, which it records inhasMultipleValuesinstead.updateFieldsentpending ?? originalas one value, so clearing a disagreeing field sent.null.Before #2858 this cancelled itself: the inspector passed one shared
oldValuetaken from the same nil, soPendingChanges.recordCellChangesawoldValue == newValueand rolled the change back. #2858 madeoldValueper 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
onFieldRevertedandRowEditingCoordinator.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
restoreRowBufferToOriginalsrewritesTableRowsand repaints the grid, but moves nothingInspectorTriggerwatches, so the pane went on showing what was just thrown away. It now bumps the same revisionhandleUndoResultdoes.Its repaint was also positional:
editManyreports 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 throughDisplayRowMappingnow, the way the inspector's own staging already does.Verification
verify.sh buildPASS.verify.sh testPASS: 126 cases overInspectorFieldEditStagingTests,MultiRowEditStateTests,MultiRowEditStateJsonTests,FieldValueStateTests,RowEditingCoordinatorValueFilterTests,DataChangeManagerTests.swiftlint --strictclean on the changed files.MultiRowEditStatewired 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.