feat(components,grid,list): a column-header sort orders the whole list, not the page you can see (#3106) - #3112
Merged
Merged
Conversation
…t, not the page you can see (#3106) Clicking a column header under server pagination sorted the CURRENT PAGE. The user saw "sorted by this column" and got "these fifty rows are in order; page 2 starts over". The sort was real — its scope was not the one the screen implied — and it had no way out of `data-table` at all: it lived in two `useState`s with no callback, so the layer that issues the request could not see it even in principle. DataTable gains `manualSorting` + a controlled `sort` + `onSortChange`. In that mode it sorts nothing, reports what a header click asks for, and renders `sort` as the indicator — holding NO sort state of its own, because a private copy beside a controlled prop is the shape the defect had. `manualSorting` is independent of `manualPagination`: what matters is whether `data` is a window, not who owns the pager. ObjectGrid turns that into a `$orderby` in both of its server modes (its own fetch and a parent-driven one) and returns to page 1, since a new ordering makes the old page index a different set of rows. ListView lands it in `currentSort` — the same state the toolbar's sort builder writes. One sort with two controls onto it is what makes "does a header sort outrank the saved view's sort?" a non-question rather than a precedence rule someone has to remember. Three decisions worth naming: - A header click REPLACES the order rather than appending, so the column under the cursor is the one the list is sorted by. Multi-key orders still come from the sort builder, and the headers render them numbered. - It cannot ask for "no sort". The third click clears in client mode, which is meaningful there (rows return to the order they arrived in); across a server-paged collection there is no such order (objectstack#4363), so a header offering it would hand the user a worse lie than the one being fixed. - Relational columns render no sort affordance under server sorting: a `lookup` column shows a related record's name while `$orderby` can only order by the stored id (objectstack#4256) — the same reason #3096 removed them from the toolbar's picker. Client-side sorting keys off the rendered label, so those headers stay live there. The header context menu's Sort Ascending/Descending items now route through the same write path; they wrote internal state directly, which under `manualSorting` would have been a menu item that highlights, closes, and changes nothing. Client-side tables are untouched: same three-state cycle, same local sort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
xuyushun441-sys
pushed a commit
that referenced
this pull request
Jul 31, 2026
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
1 participant
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.
Closes #3106.
What was wrong
Clicking a column header under server pagination sorted the current page. The user saw "sorted by this column" and got "these fifty rows are in order; page 2 starts over."
The sort was real. Its scope was not the one the screen implied — and it had no way out of
data-tableat all: it lived in twouseStates with no callback, so the layer that issues the request could not see it even in principle.#3096 made this more dangerous rather than less: before it, clicking a lookup header produced garbage order that nobody believed; after it, the same click produces a convincing alphabetical page. The credibility went up while the scope stayed a lie.
The shape of the fix
DataTablemanualSorting+ controlledsort+onSortChange. Sorts nothing, reports the click, renderssortas the indicator.ObjectGrid$orderbyin both server modes (own fetch, parent-driven) and returns to page 1.ListViewcurrentSort— the same state the toolbar's sort builder writes.The table keeps no sort state of its own in manual mode. A private copy beside a controlled prop is the exact shape the defect had, so a test asserts the indicator follows the prop and not the click.
manualSortingis deliberately independent ofmanualPagination: what matters is whetherdatais one window of something larger, not who owns the pager. (A windowed related list paginates itself — that is PR 4.)Decisions worth reviewing
lookupcolumn shows a related record's name while$orderbycan only order by the stored id (objectstack#4256) — the same reason lookup 列排序按裸外键 id 排：服务端 $orderby 发平面 FK 字段，单元格却显示 $expand 后的标签 #3096 removed them from the toolbar's picker. Client-side sorting keys off the rendered label, so those headers stay live there.currentSortis what makes "does a header sort outrank the saved view's sort?" a non-question rather than a precedence rule someone has to remember.Also fixed in passing
The header context menu's Sort Ascending/Descending items wrote internal state directly, bypassing the sort handler. Under
manualSortingthat would have been a menu item that highlights, closes, and changes nothing — the same class of defect one control over. Both now route through a single write path.Tests
28 new cases across the three layers, including the composition neither package's own unit tests can reach (ListView → grid, via a stub
object-grid).plugin-grid,plugin-list,components,types,coreA regression caught during the run is worth noting: the first draft put
declaredSortbehind auseMemothat sits below ObjectGrid's early returns — 91 tests failed with "Rendered more hooks than during the previous render". It is a plain expression now.Related
$orderbyreaches the server as a sort, not a list of character indices (#3106) #3109 — string$orderbyserialization (independent; broken onmaintoday).statusthe most common sort key, which is what turns that latent property violation into a daily one.🤖 Generated with Claude Code