Skip to content

fix(datagrid): read the inspector row from the grid that owns the selection - #1998

Merged
datlechin merged 3 commits into
mainfrom
fix/structure-inspector-row-source
Jul 30, 2026
Merged

fix(datagrid): read the inspector row from the grid that owns the selection#1998
datlechin merged 3 commits into
mainfrom
fix/structure-inspector-row-source

Conversation

@datlechin

Copy link
Copy Markdown
Member

Selecting a row in the Structure tab showed the data row in the same position, not the structure row.

Root cause

GridSelectionState is one selection channel written by three grids (TableStructureView.swift:92, CreateTableView.swift:98, and the data grid), and it carries no owner. The inspector resolved those indices against the data tab's rows unconditionally, so a structure selection rendered a data row.

The write path was worse. isSidebarEditable only checked "table tab + non-empty selection", so the inspector rendered an editable data form over a structure selection, onFieldChanged recorded a data cell change keyed by the structure grid's display index, and saveSidebarEdits would UPDATE a data row the user never selected.

Every other consumer of the channel already discriminated on resultsViewMode (MainContentCommandActions.saveChanges, add, delete, copy). The inspector was the one that never asked.

The fix

GridSelectionOwner is now the single answer to "whose display positions are these", resolved from the same (tabType, resultsViewMode) pair the save routing uses. The grid that owns a schema selection publishes its row through InspectorRowSource instead of the inspector guessing:

  • StructureGridDelegate and CreateTableGridDelegate conform, and StructureInspectorRowBuilder builds the payload from the same StructureRowProvider the grid renders, so the panel and the grid cannot disagree about what a row holds.
  • Commits route through the delegate's existing dataGridDidEditCell, so an inspector edit is indistinguishable from an inline cell edit: same StructureChangeManager, one undo step, one pending change, applied by the same Save.
  • Fields use the grid's own editors: YES/NO menus for Nullable, Primary Key, Auto Inc, and index Unique, the searchable type list for Type, text elsewhere. No Set NULL, Set DEFAULT, or type badge on a schema field, since none of those mean anything for a schema property (adding table columns #1991).
  • Text commits on Return or focus loss, not per keystroke, because updateColumn registers one undo action per call. Per-keystroke commits would make Cmd+Z undo one character at a time.
  • A row marked for deletion, and any engine without supportsSchemaEditing, gets read-only fields. Editability mirrors the grid's canEdit exactly, so the inspector is never more permissive than the grid beside it.
  • Switching results mode clears the shared channel on appear, so a stale data index can never be read as a structure row.

Also fixed, same bug class

duplicateRow() and copySelectedRowsWithHeaders() acted on data rows while the Structure tab was open. Both now require the data grid to own the selection. saveSidebarEdits also stopped indexing tableRows.rows[rowIndex] with a display position, which violated the selection-indices invariant in CLAUDE.md.

Tests

  • GridSelectionOwnerTests: the resolve matrix, including query tabs, JSON view, and tabs with no row grid.
  • StructureInspectorRowBuilderTests and StructureGridDelegateInspectorTests: the regression. The payload comes from the structure provider, and an edit made under an active filter or sort lands on the entity behind that display row rather than the one at that array position.
  • Additions to StructureRowProviderTests (modified fields and pending deletes across columns, indexes, and foreign keys) and MultiRowEditStateTests (schema field descriptions, field identity across a reconfigure so an editor keeps focus, commit routing).

No TableProUITests coverage: the flow needs a live connection with a loaded table, so it would not run deterministically in CI.

Notes for the reviewer

  • The first commit is the unreleased CHANGELOG simplification requested earlier and is unrelated to the fix. Drop it if you would rather ship that separately.
  • Localizable.xcstrings is deliberately not in this PR. The working copy held unrelated regeneration churn from a local build. Choose Type is the one new string and the next build will add it.
  • Pre-existing and untouched: unsaved structure changes are still absent from the window-close prompt, since hasUnsavedWork covers data rows, files, and users and roles only. Inspector edits land in the same change manager as grid edits, so this behaves identically either way.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@mintlify

mintlify Bot commented Jul 30, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
TablePro 🟢 Ready View Preview Jul 30, 2026, 12:37 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@datlechin
datlechin merged commit dc7b430 into main Jul 30, 2026
1 of 2 checks passed
@datlechin
datlechin deleted the fix/structure-inspector-row-source branch July 30, 2026 12:47
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