Skip to content

Commit bac266c

Browse files
os-zhuangclaude
andauthored
fix(detail): a related list has one sorting semantics instead of two (#3106) (#3113)
* feat(components,grid,list): a column-header sort orders the whole list, 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> * fix(detail): a related list has one sorting semantics instead of two (#3106) A related list carried two. Its own sort-button row (opt-in via `sortable`) went out as a server `$orderby` over the whole child collection; the `data-table` it embeds took `sortable`'s default of `true` and sorted the rows it was holding — which, in windowed mode, is ONE PAGE. Turning `sortable` on put both in the same card with nothing saying they meant different things. Leaving it off — the default — was worse: the page-local sort was then the only one the user could reach, and it looked exactly like the list being sorted. The embedded table's column headers now drive this list's sort in both modes: - Windowed: the header sort becomes the server `$orderby` and resets to page one, the same path the buttons took. - Client mode: this list keeps sorting in memory, where the key is the label resolved through its own id → name map (#3096) — a key the table cannot see, so its sort was the worse of the two even where both were possible. The button row survives only where there are no headers to click: a `list` (`data-list`) related list, or a caller-supplied `schema` whose contents we cannot assume. `sortable`'s doc comment now says that is what it controls. Relational columns keep #3096's rule, moved to the header: withheld while the sort is a server `$orderby` (the key would be the stored foreign-key id while the cell shows a name), live in client mode where the key is the label. Three existing suites asserted through the sort buttons — two on #3096's own guarantee, one using them as a proxy for column order. They now read the column's `sortable` flag and drive the table's `onSortChange`, so the guarantees they encode survive the control that carried them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 24e0e0a commit bac266c

6 files changed

Lines changed: 410 additions & 21 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
"@object-ui/plugin-detail": minor
3+
---
4+
5+
fix(detail): a related list has one sorting semantics instead of two — #3106
6+
7+
A related list carried two. Its own sort-button row (opt-in via `sortable`) went
8+
out as a server `$orderby` over the whole child collection; the `data-table` it
9+
embeds took `sortable`'s default of `true` and sorted the rows it was holding —
10+
which, in windowed mode, is **one page**.
11+
12+
Turning `sortable` on put both in the same card, with nothing saying they meant
13+
different things. Leaving it off — the default — was worse: the page-local sort
14+
was then the *only* one the user could reach, and it looked exactly like the
15+
list being sorted.
16+
17+
The table's column headers now drive this list's sort in both modes, so there is
18+
one order behind them:
19+
20+
- **Windowed**: the header sort becomes the server `$orderby` and resets to page
21+
one, the same path the buttons took.
22+
- **Client mode**: this list keeps sorting in memory, where its key is the label
23+
resolved through its own id → name map (#3096) — a key the embedded table
24+
cannot see, so its sort was the worse of the two even when both were possible.
25+
26+
The button row survives only where there are no headers to click: a `list`
27+
(`data-list`) related list, or a caller-supplied `schema` whose contents we
28+
cannot assume. `sortable`'s documentation now says that is what it controls.
29+
30+
Relational columns keep #3096's rule, moved to the header: no sort affordance
31+
while the sort is a server `$orderby` (the key would be the stored foreign-key
32+
id while the cell shows a name), live in client mode where the key is the label.

packages/plugin-detail/src/RelatedList.tsx

Lines changed: 97 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,17 @@ export interface RelatedListProps {
111111
* `$orderby`, keeping page windows deterministic.
112112
*/
113113
defaultSort?: string | Array<{ field: string; order: 'asc' | 'desc' }>;
114-
/** Enable column sorting */
114+
/**
115+
* Render the standalone row of sort buttons above the list.
116+
*
117+
* Only meaningful for a `list` (`data-list`) related list, which has no
118+
* column headers to click. A `grid`/`table` one sorts through its table
119+
* headers, which are live regardless of this flag — they always were, and
120+
* since objectui#3106 they sort the collection rather than the page, so the
121+
* button row above them would be a second control over the same order.
122+
*
123+
* @default false
124+
*/
115125
sortable?: boolean;
116126
/** Enable text filtering */
117127
filterable?: boolean;
@@ -591,6 +601,38 @@ export const RelatedList: React.FC<RelatedListProps> = ({
591601
}
592602
}, [sortField]);
593603

604+
/**
605+
* The order the embedded table's headers display — this list's own sort, so
606+
* the arrow on a column header and the rows underneath it come from the same
607+
* place (objectui#3106).
608+
*
609+
* Before any click that is the declared `defaultSort`, which is what the
610+
* windowed fetch above sends. A header showing nothing while the server was
611+
* asked for `created_at desc` would make the first click on that column
612+
* request `asc` on a list that is already `desc`.
613+
*/
614+
const activeSort = React.useMemo(
615+
() => (sortField ? [{ field: sortField, order: sortDirection }] : defaultSortSpec),
616+
[sortField, sortDirection, defaultSortSpec],
617+
);
618+
619+
/**
620+
* A header click from the embedded table. It arrives with the direction
621+
* already resolved against {@link activeSort}, so this assigns rather than
622+
* toggling — running it back through `handleSort`'s own toggle would undo it
623+
* whenever the two disagreed about the current state.
624+
*/
625+
const handleTableSort = React.useCallback(
626+
(next: Array<{ field: string; order: 'asc' | 'desc' }>) => {
627+
const first = next[0];
628+
if (!first) return;
629+
setCurrentPage(0);
630+
setSortField(first.field);
631+
setSortDirection(first.order);
632+
},
633+
[],
634+
);
635+
594636
// Confirm-delete dialog state. Replaces window.confirm() so the related
595637
// list matches the rest of the app's Shadcn AlertDialog UX.
596638
const [deleteTarget, setDeleteTarget] = React.useState<any | null>(null);
@@ -836,6 +878,36 @@ export const RelatedList: React.FC<RelatedListProps> = ({
836878
return pruned.slice(0, Math.max(1, maxColumns));
837879
}, [columns, objectSchema, objectName, api, resolveFieldLabel, referenceField, relatedData, maxColumns, lookupLabels, perms]);
838880

881+
/**
882+
* The same columns, with the sort affordance withheld from relational ones
883+
* while the sort is the server's.
884+
*
885+
* A windowed sort goes out as `$orderby` on the flat field name, and a
886+
* relational column stores a foreign-key id — so "sort by Owner" would order
887+
* the collection by `rec_7f3…` while the cells show names (objectui#3096;
888+
* objectstack#4256 settled that no relation join is coming). This is the rule
889+
* the sort-button row already applied; the headers inherit it rather than
890+
* re-opening the same door.
891+
*
892+
* In client mode the sort keys off the resolved label, so those headers stay
893+
* live — the same split `sortedData` makes.
894+
*/
895+
const sortableColumns = React.useMemo(() => {
896+
if (!windowed) return effectiveColumns;
897+
return effectiveColumns.map((col: any) => {
898+
const field = col.accessorKey || col.field || col.name;
899+
return field && isExpandableFieldType((objectSchema?.fields as any)?.[field])
900+
? { ...col, sortable: false }
901+
: col;
902+
});
903+
}, [effectiveColumns, windowed, objectSchema]);
904+
905+
// A `grid`/`table` list renders a real table, whose column headers carry the
906+
// sort. `list` renders `data-list`, which has none — so it keeps the button
907+
// row as its only sort control. A caller-supplied `schema` renders whatever
908+
// it says, so we cannot claim headers for it either.
909+
const hasSortableHeaders = !schema && (type === 'grid' || type === 'table');
910+
839911
const hasCustomRowActions = Array.isArray(rowActions) && rowActions.length > 0;
840912
const hasRowActions = !!onRowEdit || !!onRowDelete || hasCustomRowActions;
841913
const isMobile = useIsMobile();
@@ -883,11 +955,24 @@ export const RelatedList: React.FC<RelatedListProps> = ({
883955
return {
884956
type: 'data-table',
885957
data: paginatedData,
886-
columns: effectiveColumns,
958+
columns: sortableColumns,
887959
pagination: false, // We handle pagination ourselves
888960
pageSize: effectivePageSize || 10,
889961
searchable: false,
890962
exportable: false,
963+
// Sorting is THIS list's, in both modes (objectui#3106).
964+
//
965+
// Windowed: `data` is one page, so a table-local sort would order
966+
// that page and call it the list — the defect. Client mode: this
967+
// list's own sort resolves a relational column through the label map
968+
// it built (`lookupLabels`), which the table cannot see, so its sort
969+
// is the better one even when both are possible.
970+
//
971+
// Either way there is now ONE sort behind the headers instead of a
972+
// table-local order sitting on top of a server order.
973+
manualSorting: true,
974+
sort: activeSort,
975+
onSortChange: handleTableSort,
891976
rowActions: hasRowActions,
892977
onRowEdit,
893978
onRowDelete: onRowDelete ? handleDeleteRow : undefined,
@@ -907,7 +992,7 @@ export const RelatedList: React.FC<RelatedListProps> = ({
907992
default:
908993
return { type: 'div', children: 'No view configured' };
909994
}
910-
}, [type, paginatedData, effectiveColumns, schema, effectivePageSize, hasRowActions, hasCustomRowActions, rowActions, onRowAction, onRowEdit, onRowDelete, handleDeleteRow, onRowClick, isMobile, api, objectSchema]);
995+
}, [type, paginatedData, sortableColumns, effectiveColumns, schema, effectivePageSize, hasRowActions, hasCustomRowActions, rowActions, onRowAction, onRowEdit, onRowDelete, handleDeleteRow, onRowClick, isMobile, api, objectSchema, activeSort, handleTableSort]);
911996

912997
const headerClassName = collapsible ? 'cursor-pointer select-none' : undefined;
913998
const handleHeaderClick = collapsible ? () => setCollapsed((c) => !c) : undefined;
@@ -1008,8 +1093,15 @@ export const RelatedList: React.FC<RelatedListProps> = ({
10081093
</div>
10091094
)}
10101095

1011-
{/* Sortable column headers */}
1012-
{sortable && effectiveColumns && effectiveColumns.length > 0 && relatedData.length > 0 && (
1096+
{/* Sort buttons — only where there are no column headers to click.
1097+
A `grid`/`table` related list renders a real table whose headers now
1098+
drive this same sort (objectui#3106), so a second row of buttons
1099+
above it would be two controls over one order — and, before the
1100+
headers were wired up, two DIFFERENT orders: the buttons sorted the
1101+
collection through the server while the headers sorted the page in
1102+
the browser, in one card, with nothing saying so. `data-list` has no
1103+
headers, so there the buttons stay the only way to sort. */}
1104+
{sortable && !hasSortableHeaders && effectiveColumns && effectiveColumns.length > 0 && relatedData.length > 0 && (
10131105
<div className="flex flex-wrap gap-1 mb-3">
10141106
{effectiveColumns.map((col: any) => {
10151107
const field = col.accessorKey || col.field || col.name;

0 commit comments

Comments
 (0)