fix(datagrid): keep the reader's place on refresh and start new views at the first row - #2825
Open
datlechin wants to merge 1 commit into
Open
fix(datagrid): keep the reader's place on refresh and start new views at the first row#2825datlechin wants to merge 1 commit into
datlechin wants to merge 1 commit into
Conversation
… at the first row
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
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.
Problem
A user reported that after a refresh the new data is there but the grid does not scroll to it, so an insert looks like it failed.
Root cause
Nothing owned where a table tab's data grid lands when a result replaces its rows.
NSTableView.reloadData()keeps the clip view's origin and drops the selection, so every reload nobody made a decision for showed the old result's pixel offset over new rows. Refresh anchored nothing, so rows inserted above the reader shifted everything under them.pendingScrollToTopAfterReplaceand Back/Forward throughpendingRowAnchors. Both were per-tab flags consumed only when the result landed on the selected, mounted grid, so a failed, cancelled, empty or background load left them set and they fired on a later, unrelated reload.applyFullReplace()left the inline cell editor open over a display position that now held another record.Fix
Each table-tab reload cause declares a
GridReloadIntent, carried by the execution that produces the rows, so it dies with a failed or superseded run:GridViewportResolver(pure) snapshots the grid's first visible row by primary key before the rows are replaced and resolves it against the new rows. A key is used only when it comes from a saved, unedited row and matches exactly one new row; otherwise the position holds. A grid scrolled to the very top stays at the top, so rows added above come into view.TabSessionwith the buffer epoch it was resolved for, only when a table tab's grid is on screen, and the grid applies it at the end of its update pass, after the last reload for that content.pendingScrollToTopAfterReplaceandpendingRowAnchorsare gone; the Back/Forward anchor lives onQueryTab.restoredRowAnchorand any install of rows spends it.contentInsets, where AppKit puts the header height (28pt, 42pt with column comments; measured).applyFullReplace()dismisses the inline editor. The discard gates commit an active edit first, and Refresh and column visibility restore the row buffer before clearing edits, so a discarded key never becomes an anchor.Refresh keeps place rather than jumping to the first row, which is what Sequel Ace and DBeaver do; users of TablePlus and Postico have filed complaints about losing their place, and DataGrip closed "Reload doesn't display newly-added record" as works-as-intended.
Deliberately not fixed here
Selecting and revealing the rows a Save inserted. Three designs were built and reviewed, and each round found a new way to select the wrong row or none:
5comes back as5.00, a date gains00:00:00).DataWriteStepResultcarries no key the server assigned, so the client cannot know which row it created. Doing this properly needs the write path to return the inserted key (RETURNINGon PostgreSQL and SQLite,LAST_INSERT_ID()on MySQL,OUTPUTon SQL Server) and a reveal keyed on it. That is its own change across the drivers.Verification
check-writing-style.shandcheck-docs-against-source.pypass.GridViewportResolverTests,GridReloadViewportTestsandDataGridViewportPlacementTests, with 241 passing.MainContentCoordinatorRefreshTests/singleRequestRefreshHasNoTrailingfailed only in that parallel run; its 400 ms sleep races a 250 ms coalescing task under main-actor load. It passes when run on its own (13/13).DataGridViewportPlacementTestschecks the real clip-view inset from anNSTableHeaderView, not a value set by hand.SaveCompletionTests: left out of the local runs. Two of its tests reach the real Save Changes confirmation, which runs app-modal when no window is key and hangs the test host (sampled stack:saveChanges→OperationConfirming.confirm→runStatementConfirmation). That is onmainand unrelated to this change.DataGridReloadViewportUITests(Refresh keeps the place, a header sort lands on the first row) was added but not run locally. macOS held an "Enable UI Automation" password prompt, and every run ended with "Timed out while enabling automation mode". CI runs it.https://claude.ai/code/session_01EPyw1skNf2cexdYTa82zLw