Skip to content

refactor(datagrid): unify cell edit path, class-based RowProvider cache#918

Merged
datlechin merged 1 commit intomainfrom
refactor/datagrid-critical-audit
Apr 27, 2026
Merged

refactor(datagrid): unify cell edit path, class-based RowProvider cache#918
datlechin merged 1 commit intomainfrom
refactor/datagrid-critical-audit

Conversation

@datlechin
Copy link
Copy Markdown
Member

Summary

  • Single cell edit path: replace 5 duplicate recordCellChange + updateValue + reloadData sequences with a single commitCellEdit() method on TableViewCoordinator. ChangeManager is the source of truth for pending changes, RowProvider reflects display state, both updated atomically through one code path.
  • Class-based RowProvider cache: replace @State dictionary cache (tabProviderCache) with RowProviderCache class. Cache writes are now synchronous (no more deferred Task blocks that could create duplicate providers during body evaluation).

Addresses DataGrid audit items #2 (dual state for cell edits) and #4 (RowProvider cache as @State).

Test plan

  • Edit a cell via inline field editor, overlay editor, date picker, boolean dropdown, FK search popover, and "Set Value" context menu (NULL, Empty, Default). All should commit correctly.
  • Undo/redo cell edits across all edit types
  • Switch between tabs rapidly, verify no stale data displayed
  • Edit cells, switch tabs, switch back, verify edits preserved
  • Sort a table with pending edits, verify edits survive the sort

🤖 Generated with Claude Code

…he (#918)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 740b5d5cb9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +29 to +33
let tableColumnIndex = DataGridView.tableColumnIndex(for: columnIndex)
tableView.reloadData(
forRowIndexes: IndexSet(integer: row),
columnIndexes: IndexSet(integer: tableColumnIndex)
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Defer table reload until inline edit teardown completes

commitCellEdit now reloads the table synchronously, but control(_:textShouldEndEditing:) calls into this path while AppKit is still in the end-edit callback. Reloading the edited cell at that point can tear down the active field editor mid-cycle (e.g., Enter/Tab commits), which can lead to intermittent dropped commits or broken focus navigation. This path previously deferred reload to the next main-actor turn, and that deferral is still needed for inline text editing.

Useful? React with 👍 / 👎.

@datlechin datlechin force-pushed the refactor/datagrid-critical-audit branch from 740b5d5 to d8945ea Compare April 27, 2026 17:19
@datlechin datlechin merged commit 76fc4b3 into main Apr 27, 2026
2 checks passed
@datlechin datlechin deleted the refactor/datagrid-critical-audit branch April 27, 2026 17:23
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