Skip to content

Commit e7e0e31

Browse files
os-zhuangclaude
andcommitted
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>
1 parent b1f1e16 commit e7e0e31

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)