Repository navigation
fix(bindx-dataview): reload, select paging and infinite load - #162
Conversation
|
Review round 1: independent Claude agent (Claude Code, Verdict: changes
Checked, no issue: the render-phase |
|
Response to review round 1
|
|
Review round 2: a fresh independent Claude agent (Claude Code, Verdict: changes
Checked, no issue:
|
reload() in DataGrid, HasManyDataGrid and SelectDataView bumped a counter that nothing read, so the toolbar reload button and useDataViewReload() left stale rows on screen. DataGrid and SelectDataView fold the counter into the list's query key and bump the paging's count refresh token. HasManyDataGrid re-runs its relation query, which also carries the total, and keeps the rows on screen while the same query loads again. The loader state is now derived in render from the list result instead of an effect, and reports `refreshing` while the rows on screen answer an earlier query (stale-while-revalidate). Before, a refetch kept the state at `loaded`, so the reload button never spun and the refreshing overlay never showed. Fixes #124 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SelectDataView kept the page index of the previous search, so a search narrowed after scrolling went out at the old offset and showed nothing until the count caught up. It now resets paging to the first page when its filter (search and static filter) changes, during render, so no query is sent at the old offset. The paging also judged `hasNext` by a counted total that can still describe the previous search while the new count loads, so the sentinel of a short list loaded an empty page. usePagingTotalCount now lets a current page shorter than the limit set the total: such a page ends the list whatever the count says. Fixes #145 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The provider kept `pages` in state but never filled it, and rendered the view's `items`, which hold only the current page, so "load more" replaced the rows. `isLoadingMore` was set once the offset moved and never reset, so no page past the second was ever requested: no bindx-ui select could reach options past the first 100. loadMore now stores the current page and moves the view to the next one; two calls before the next render request one page. Children get a data view whose `items` are the stored pages followed by the current one (each row once), so DataViewEachRow, the keyboard handler and the highlight indices see the whole list. Stored pages belong to one query (filter, order, page size) and to an unbroken run of offsets from the first row; anything else shows only the current page. Loading follows the view's loader state, so it ends when the page lands, empty or not. Fixes #146 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eshing Since views report `refreshing` during a stale-while-revalidate refetch, DataViewNonEmpty, which rendered only on `loaded`, unmounted the default grid table (header, filter popovers, rows) on every page, sort, filter change and reload. It now also renders on `refreshing` while rows are on screen. DataViewEmpty still waits for `loaded`, so a view without rows does not flash its empty state while it loads. Refs #124 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The view refetches only its current page, so reload() in an infinite-scroll list kept the earlier pages the provider had stored. The provider now hands its children a reload that moves the view back to the first page and refetches it. Refs #146 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… screen The loader state reported `refreshing` both while stale rows were shown and while a view loaded with nothing to show. So DataViewEmpty, which waits for `loaded`, hid an empty grid's "No results" on every refetch, and letting it render on `refreshing` would have flashed it during a load with no result yet. - `refreshing` now means the rows on screen answer an earlier query; a load with nothing to show is `initial`. - DataViewEmpty, like DataViewNonEmpty, stays on screen while refreshing. - HasManyDataGrid keeps the previous rows for any refetch (page, sort, filter, reload) instead of clearing them, and derives the refetch in render from the request key, so its loader state never lags behind the rows by a render. Refs #124 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A reload moved the view to the first page while it still held the rows of the last offset, so the list showed only its last page until the first one landed. The provider now keeps the list shown at the reload until the first page lands (or fails), and drops its stored pages. Documents that only the provider's descendants get its reload. Refs #146 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c9e2d8a to
3dd3746
Compare
|
Response to review round 2 The branch was rebased onto current
Each new test fails without its fix. Local checks on 3dd3746's tree: typecheck clean, 2542/2542 tests, lint 0 errors. |
|
Review round 3: a fresh independent Claude agent (Claude Code, Verdict: changes
Checked, no issue:
|
…tion HasManyDataGrid keeps the previous rows on screen while a refetch loads, but its request key covered only the query options. When the parent changed (p1 -> p2 -> p1 with the grid kept mounted), the rows of the other parent were shown as `loaded`, and their count fed the current parent's total. - Rows are kept only for the relation (parent, field, selection) they were loaded for; for another one the grid loads as `initial`. The same check gates the page that feeds the total. - The data effect follows the request key, which covers the selection's content, instead of the selection object, which is new on every parent render and refetched the grid each time. - Responses are applied with refreshServerData, as useEntityList does, so a refetch no longer overwrites edits made to rows still on screen. Refs #124 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A second reload() before the first page landed snapshotted the joined list, which by then held only the stale current page, so the infinite list shrank to one page until the load finished. The list shown at the first reload is kept. Refs #146 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`initial` is now any load with no rows to show, including a retry after a failure, and `refreshing` means the rows shown answer an earlier query. Documents the states on DataViewLoaderState and the loader state components, and covers the retry case with a test. Refs #124 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Response to review round 3
Each new test fails without its fix, except the per-render recorder, which passes on eb4eeb9 by design. Local checks: typecheck clean, 2549/2549 tests, lint 0 errors (the same 13 warnings as |
|
Review round 4: a fresh independent Claude agent (Claude Code, Verdict: approve. No blockers and no major findings. Minor findings:
Checked, no issue:
|
|
Response to review round 4 Approved, no major findings. To keep the reviewed commit as the merge candidate, all four minors go to a follow-up, not this PR:
|
Fixes three DataView bugs. They share
SelectDataView.tsxand build on each other (the infinite-load fix relies on the loader state introduced for reload and on the select page reset), so they land as one PR with one commit per bug plus two follow-ups from review.#124 —
reload()is a no-op (480f41c, follow-ups 7b7049a, eb4eeb9)reload()inDataGrid,HasManyDataGridandSelectDataViewbumped a counter nothing read.DataGrid/SelectDataView: the counter is folded into the list'squeryKeyandreload()bumps the paging's count refresh token.HasManyDataGrid: the relation query re-runs (it carries the total) and keeps the rows on screen while the same query loads again.useLoaderState) instead of an effect, and reportsrefreshingwhile the rows on screen answer an earlier query. Before, a stale-while-revalidate refetch stayedloaded, so the reload button never spun and the refreshing overlay never showed. Behaviour change: grids and selects now reportrefreshingon every refetch (page, sort, filter, reload), as the bindx-ui loader and toolbar expect.refreshingmeans only "the rows on screen answer an earlier query" (a load with nothing to show isinitial).DataViewNonEmptyandDataViewEmptystay on screen while refreshing, so the default grid neither unmounts its table nor hides "No results" during a refetch.HasManyDataGridkeeps its previous rows for any refetch, but only for the relation they were loaded for (844bde1). It derives the refetch in render, and applies responses without overwriting local edits.#145 — a narrowed select search shows nothing (853511c)
SelectDataViewresets paging to the first page when its filter (search + staticfilter) changes, during render, so no query goes out at the old offset.usePagingTotalCount: a current page shorter than the limit sets the total, over a counted total that may still describe the previous search. SohasNextis false for a short page and the sentinel no longer loads an empty page.#146 — infinite scroll stops after page 2 (123b5d6, follow-ups 6431851, 3dd3746)
DataViewInfiniteLoadProvider:loadMorestores the current page and callspaging.next(); two calls before the next render request one page.itemsare the stored pages followed by the current one (each row once), soDataViewEachRow, the keyboard handler and highlight indices see the whole list.reload()inside the provider restarts from the first page, since the view refetches only its current page. The list shown at the (first) reload stays until the first page lands (abcc609).Tests
Each new test fails without its fix:
tests/react/dataview/dataViewReload.test.tsx(DataGrid reload() is a no-op: the list is never refetched #124)tests/react/dataview/selectDataViewPaging.test.tsx(SelectDataView shows nothing for a narrowed search (stale page index and count) #145)tests/react/dataview/infiniteLoad.test.tsx(DataViewInfiniteLoadProvider: pages never accumulate, infinite scroll stops after page 2 #146)Status
changes. Fixed in 7b7049a and 6431851; one minorwon't fix(static filter in the infinite-load key, reason in the response comment).changes. All four findings fixed in eb4eeb9 and 3dd3746.changes. All findings fixed in 844bde1, abcc609 and 31736eb; one info item accepted.approve. Four minors deferred to a follow-up (see the response comment).main(229d581). Local checks green (typecheck, 2549/2549 tests, lint 0 errors).mainas 3a6d906 (merge commit). Released in 0.1.61. Deferred minors tracked in DataView follow-ups from #162: retry loader state, HasManyDataGrid selection keying, test robustness #164.🤖 Generated with Claude Code