Skip to content

Commit 232d975

Browse files
feat(spa): honour list_display_links in the changelist (#666)
The changelist wire emits list_display_links; the Table/RecordCardList primitives now link exactly the columns flagged isLink (set by ListPage from the wire) instead of hard-pinning the first column — and link none / make the row inert when the admin sets list_display_links = None. A pre-1.6.0 backend (field absent) keeps the legacy first-column behaviour. Table tests cover the explicit-link, no-link, and legacy-fallback paths. Note: ListPage also gains virtualizeRows on the Table for #670 (committed next). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d9a1988 commit 232d975

4 files changed

Lines changed: 156 additions & 11 deletions

File tree

frontend/apps/web/src/pages/ListPage.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,19 @@ export function ListPage() {
423423
// live here was retired in v1.3.0 along with the HTML5-DnD modal
424424
// it served (#586).
425425

426+
// list_display_links (#666): the ModelAdmin chooses which column(s) link
427+
// to the change page. The backend emits the resolved set (defaulting to
428+
// the first column); an empty array means `list_display_links = None` —
429+
// no column links and the rows are not clickable. When the field is
430+
// ABSENT (a pre-1.6.0 backend), fall back to the legacy behaviour of
431+
// linking the first non-pk column, so older deployments are unaffected.
432+
const displayLinks = data.list_display_links;
433+
const firstNonPkName = orderedDescriptors.find((c) => !isPkCol(c.name))?.name;
434+
const isLinkColumn = (name: string): boolean => {
435+
if (displayLinks === undefined) return name === firstNonPkName; // legacy fallback
436+
return displayLinks.includes(name);
437+
};
438+
426439
const columns = orderedDescriptors
427440
// The pk column is never hidden, even if a stale preference lists it.
428441
.filter((c) => isPkCol(c.name) || !hiddenCols.has(c.name))
@@ -431,6 +444,7 @@ export function ListPage() {
431444
header: c.label,
432445
sortable: c.sortable,
433446
noTruncate: isPkCol(c.name),
447+
isLink: isLinkColumn(c.name),
434448
// Frozen / sticky columns (#586): pk is implicitly always
435449
// locked + sticky; the user-locked set adds more. The Table
436450
// primitive measures pixel offsets after layout and writes
@@ -804,6 +818,10 @@ export function ListPage() {
804818
loading={loading}
805819
columnWidths={colWidths}
806820
onColumnResize={resizeColumn}
821+
// Native row windowing on the "Show all N" path (#670): up to
822+
// list_max_show_all (200) rows render at once; off-screen rows
823+
// skip layout/paint via content-visibility.
824+
virtualizeRows={showAll}
807825
/>
808826
</Card>
809827
)}

frontend/packages/ui/src/RecordCardList.tsx

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,17 @@ export function RecordCardList<Row>({
6060
// First column = identity → title; the rest become the label/value body.
6161
const [titleCol, ...detailCols] = columns;
6262

63+
// list_display_links (#666): when the caller explicitly links NO column
64+
// (`list_display_links = None` → every column carries `isLink: false`),
65+
// the title is plain text and the card is not tappable — matching the
66+
// desktop `<Table>`. Otherwise behaviour is unchanged: the card is
67+
// tappable when `onRowClick` is set, and the title is an anchor when
68+
// `rowHref` is set (the legacy default of "first column = identity link").
69+
const anyExplicitLink = columns.some((c) => c.isLink !== undefined);
70+
const linksDisabled = anyExplicitLink && !columns.some((c) => c.isLink);
71+
const titleLinks = !linksDisabled;
72+
const cardNav = linksDisabled ? undefined : onRowClick;
73+
6374
if (loading) {
6475
return (
6576
<ul className="space-y-2" aria-busy="true">
@@ -87,23 +98,23 @@ export function RecordCardList<Row>({
8798
<li
8899
key={key}
89100
onClick={
90-
onRowClick
101+
cardNav
91102
? (e) => {
92103
// A modified click (Cmd/Ctrl/Shift/Alt) is the browser's
93104
// open-in-new-tab gesture — let the title anchor handle
94105
// it; don't also navigate in-app.
95106
if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey) return;
96-
onRowClick(row);
107+
cardNav(row);
97108
}
98109
: undefined
99110
}
100111
className={`rounded-lg border bg-white p-4 ${
101112
isSelected ? 'border-primary' : 'border-gray-300'
102-
} ${onRowClick ? 'cursor-pointer hover:bg-gray-50' : ''}`}
113+
} ${cardNav ? 'cursor-pointer hover:bg-gray-50' : ''}`}
103114
>
104115
<div className="flex items-start justify-between gap-3">
105116
<div className="min-w-0 flex-1 font-medium text-gray-900">
106-
{rowHref && titleCol ? (
117+
{titleLinks && titleCol && rowHref ? (
107118
// Real anchor so native open-in-new-tab works; a plain
108119
// left-click is intercepted for in-app nav (#253).
109120
<a
@@ -113,7 +124,7 @@ export function RecordCardList<Row>({
113124
if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey) return;
114125
e.preventDefault();
115126
e.stopPropagation();
116-
onRowClick?.(row);
127+
cardNav?.(row);
117128
}}
118129
>
119130
{title}

frontend/packages/ui/src/Table.test.tsx

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,56 @@ describe('Table', () => {
139139
expect(th.getAttribute('tabindex')).toBeNull();
140140
expect(th.getAttribute('aria-sort')).toBeNull();
141141
});
142+
143+
describe('list_display_links (#666)', () => {
144+
const linkRows: Row[] = [{ id: 1, name: 'Alice' }];
145+
146+
it('links the column(s) flagged isLink, not the first column, when isLink is set explicitly', () => {
147+
const cols: TableColumn<Row>[] = [
148+
{ key: 'id', header: 'ID', isLink: false, render: (r) => `#${r.id}` },
149+
{ key: 'name', header: 'Name', isLink: true, render: (r) => r.name },
150+
];
151+
render(
152+
<Table columns={cols} rows={linkRows} rowKey={(r) => r.id} rowHref={(r) => `/x/${r.id}`} />,
153+
);
154+
// The `name` cell is the link; the `id` cell is plain text.
155+
const nameLink = screen.getByText('Alice').closest('a');
156+
expect(nameLink).not.toBeNull();
157+
expect(nameLink?.getAttribute('href')).toBe('/x/1');
158+
expect(screen.getByText('#1').closest('a')).toBeNull();
159+
});
160+
161+
it('renders NO cell links and an inert row when no column is a link (list_display_links = None)', () => {
162+
const onRowClick = vi.fn();
163+
const cols: TableColumn<Row>[] = [
164+
{ key: 'id', header: 'ID', isLink: false, render: (r) => `#${r.id}` },
165+
{ key: 'name', header: 'Name', isLink: false, render: (r) => r.name },
166+
];
167+
const { container } = render(
168+
<Table
169+
columns={cols}
170+
rows={linkRows}
171+
rowKey={(r) => r.id}
172+
rowHref={(r) => `/x/${r.id}`}
173+
onRowClick={onRowClick}
174+
/>,
175+
);
176+
expect(container.querySelector('tbody a')).toBeNull();
177+
// Row is not clickable when nothing links.
178+
fireEvent.click(screen.getByText('Alice'));
179+
expect(onRowClick).not.toHaveBeenCalled();
180+
expect((container.querySelector('tbody tr') as HTMLElement).className).not.toContain(
181+
'cursor-pointer',
182+
);
183+
});
184+
185+
it('falls back to linking the first column when no column declares isLink (legacy / pre-1.6.0)', () => {
186+
render(
187+
<Table columns={columns} rows={linkRows} rowKey={(r) => r.id} rowHref={(r) => `/x/${r.id}`} />,
188+
);
189+
// `columns` here is [id, name] with no isLink → first column links.
190+
expect(screen.getByText('1').closest('a')).not.toBeNull();
191+
expect(screen.getByText('Alice').closest('a')).toBeNull();
192+
});
193+
});
142194
});

frontend/packages/ui/src/Table.tsx

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ export interface TableColumn<Row> {
4141
* scrolling content (#586 — column-lock / frozen-cols feature).
4242
*/
4343
sticky?: boolean;
44+
/**
45+
* Render this column's cell as a link to the row's detail/change page
46+
* (the `rowHref` target). Mirrors Django's `list_display_links` (#666):
47+
* the ModelAdmin chooses which column(s) link, not the SPA. When NO
48+
* column sets this, the table renders no cell links and rows are not
49+
* clickable (matching `list_display_links = None`). Falls back to a
50+
* sensible default (the first non-selection column) only when the caller
51+
* leaves every column's `isLink` unset AND `rowHref` is provided — see
52+
* the Table body for the back-compat path.
53+
*/
54+
isLink?: boolean;
4455
}
4556

4657
export interface TableProps<Row> {
@@ -97,8 +108,27 @@ export interface TableProps<Row> {
97108
* table keeps roughly its prior height.
98109
*/
99110
skeletonRows?: number;
111+
/**
112+
* Row virtualization for very long lists (#670 — the `?all` "Show all N"
113+
* path renders up to `list_max_show_all`, 200 by default, rows at once).
114+
* When set, each body row gets `content-visibility: auto` +
115+
* `contain-intrinsic-size`, so the browser skips layout/paint for
116+
* off-screen rows (native windowing) while keeping them in the DOM for
117+
* Ctrl-F / a11y / native scroll. Cheaper and far less fragile than a
118+
* JS windowing library given the sticky-column offset measurement this
119+
* primitive already does. Off (undefined) for normal paginated lists,
120+
* which are short enough that the optimisation isn't worth the
121+
* intrinsic-size estimate.
122+
*/
123+
virtualizeRows?: boolean;
100124
}
101125

126+
// Estimated row height for `contain-intrinsic-size` when virtualizing
127+
// (#670). Only an estimate — it sets the placeholder size for an
128+
// off-screen, not-yet-laid-out row so the scrollbar stays proportional;
129+
// the real height is used once the row scrolls into view.
130+
const ESTIMATED_ROW_HEIGHT_PX = 41;
131+
102132
const ALIGN_CLASSES = {
103133
left: 'text-left',
104134
right: 'text-right',
@@ -123,6 +153,7 @@ export function Table<Row>({
123153
skeletonRows,
124154
columnWidths,
125155
onColumnResize,
156+
virtualizeRows = false,
126157
}: TableProps<Row>) {
127158
// Hooks must run unconditionally and in stable order on every
128159
// render — the empty-state early-return below has to come AFTER
@@ -189,6 +220,26 @@ export function Table<Row>({
189220
const hasWidths = columnWidths != null && Object.keys(columnWidths).length > 0;
190221
const resizable = onColumnResize != null;
191222

223+
// list_display_links (#666): which columns render their cell as a link to
224+
// `rowHref(row)`. The caller (ListPage) sets `isLink` per the wire's
225+
// `list_display_links`. Back-compat: when NO column declares `isLink` at
226+
// all, fall back to linking the first column (the historic behaviour) so
227+
// existing callers that don't pass the flag keep their links. When the
228+
// caller DOES set `isLink` on some column(s), we honour exactly those —
229+
// and when it explicitly sets none-to-true (e.g. `list_display_links =
230+
// None`), no cell links and the rows are not clickable.
231+
const anyExplicitLink = columns.some((c) => c.isLink !== undefined);
232+
const isLinkCol = (col: TableColumn<Row>, index: number): boolean => {
233+
if (!rowHref) return false;
234+
if (anyExplicitLink) return col.isLink === true;
235+
return index === 0; // legacy default
236+
};
237+
// When the caller explicitly links NO column (`list_display_links = None`
238+
// → every column carries `isLink: false`), the rows are inert. Otherwise
239+
// behaviour is unchanged: the row is clickable when `onRowClick` is set.
240+
const linksDisabled = anyExplicitLink && !columns.some((c) => c.isLink === true);
241+
const rowClickActive = linksDisabled ? undefined : onRowClick;
242+
192243
// Helpers to build the sticky `<th>` / `<td>` style + className.
193244
// `style.left` is set so the browser knows where to pin during
194245
// horizontal scroll; the cell needs a background colour so content
@@ -371,13 +422,24 @@ export function Table<Row>({
371422
return (
372423
<tr
373424
key={key}
374-
onClick={onRowClick ? () => onRowClick(row) : undefined}
425+
onClick={rowClickActive ? () => rowClickActive(row) : undefined}
375426
// `data-selected` propagates the checkbox state to
376427
// both the row's bg AND the frozen-cells' bg via
377428
// the `--dar-row-bg` custom property (apps/web/
378429
// src/index.css #613).
379430
data-selected={isSelected ? 'true' : undefined}
380-
className={onRowClick ? 'cursor-pointer hover:bg-gray-50' : ''}
431+
// Native row windowing on the show-all path (#670): skip
432+
// layout/paint for off-screen rows, keeping them in the
433+
// DOM for find-in-page / a11y / native scroll.
434+
style={
435+
virtualizeRows
436+
? {
437+
contentVisibility: 'auto',
438+
containIntrinsicSize: `auto ${ESTIMATED_ROW_HEIGHT_PX}px`,
439+
}
440+
: undefined
441+
}
442+
className={rowClickActive ? 'cursor-pointer hover:bg-gray-50' : ''}
381443
>
382444
{selectable && (
383445
<td
@@ -414,10 +476,12 @@ export function Table<Row>({
414476
: 'max-w-[16rem] truncate'
415477
}
416478
>
417-
{ci === 0 && rowHref ? (
418-
// Real anchor on the first cell so the browser's
419-
// native open-in-new-tab works (#253); a plain
420-
// left-click is intercepted for in-app nav.
479+
{isLinkCol(col, ci) && rowHref ? (
480+
// Real anchor on each list_display_links column so
481+
// the browser's native open-in-new-tab works
482+
// (#253); a plain left-click is intercepted for
483+
// in-app nav. Which column(s) link is the
484+
// ModelAdmin's choice (#666).
421485
<a
422486
href={rowHref(row)}
423487
className="text-inherit no-underline hover:underline"

0 commit comments

Comments
 (0)