diff --git a/frontend/src/components/data-table/DataTable.jsx b/frontend/src/components/data-table/DataTable.jsx index da80224549..4e3bdde5a6 100644 --- a/frontend/src/components/data-table/DataTable.jsx +++ b/frontend/src/components/data-table/DataTable.jsx @@ -413,6 +413,22 @@ function DataTable({ * order-less state. */ sortDirections, + /** + * antd's `expandable={{ expandedRowRender, expandedRowKeys, rowExpandable, + * showExpandColumn, onExpand, … }}` — a full-width extra row rendered under + * the record it belongs to. + * + * Declared for the same reason as `onRow`, `showHeader`, `scroll`, + * `bordered`, `locale` and `sortDirections` above, and it is the widest + * silent drop of the set: undeclared, the whole object fell into `...props` + * and onto the wrapper
, so `expandedRowRender` was never called and + * the table rendered as if the prop had not been passed. HITL's review + * editor is the visible casualty — an array or object inside a table cell + * shows a truncated JSON blob with an expand button beside it, and clicking + * that button did nothing at all, leaving nested values unreadable in Table + * view (UN-4124). + */ + expandable, ...props }) { const empty = locale?.emptyText ?? emptyText; @@ -420,10 +436,144 @@ function DataTable({ const [selection, setSelection] = React.useState({}); const rows = React.useMemo(() => dataSource ?? [], [dataSource]); - const cols = React.useMemo( - () => toColumns(columns, rowSelection), - [columns, rowSelection], + + /* + * Expansion, in antd's shape. Keys are compared as strings because that is + * what TanStack's `getRowId` (and so `row.id`) produces from whatever + * `rowKey` resolves to — a call-site numbering its rows `key: index` passes + * numbers, and `[0].includes("0")` is false. + */ + const expandedRowRender = expandable?.expandedRowRender; + const canExpand = typeof expandedRowRender === "function"; + const controlledExpandedKeys = expandable?.expandedRowKeys; + const [ownExpandedKeys, setOwnExpandedKeys] = React.useState( + () => expandable?.defaultExpandedRowKeys ?? [], + ); + const expandedKeys = React.useMemo( + () => new Set((controlledExpandedKeys ?? ownExpandedKeys).map(String)), + [controlledExpandedKeys, ownExpandedKeys], + ); + + const isRowExpandable = React.useCallback( + (record) => + canExpand && + (typeof expandable?.rowExpandable === "function" + ? Boolean(expandable.rowExpandable(record)) + : true), + [canExpand, expandable?.rowExpandable], + ); + + const onExpandCb = expandable?.onExpand; + const onExpandedRowsChange = expandable?.onExpandedRowsChange; + const toggleExpanded = React.useCallback( + (key, record, originalKey) => { + const willExpand = !expandedKeys.has(key); + /* + * Report the caller's own key values, never the normalized strings: + * normalization exists to match TanStack's `row.id` and must not leak + * out. A controlled caller that passed `[1]` and then tests + * `next.includes(1)` would never match `["1"]`. + */ + const source = controlledExpandedKeys ?? ownExpandedKeys; + const next = willExpand + ? [...source, originalKey] + : source.filter((k) => String(k) !== key); + // A controlled `expandedRowKeys` belongs to the parent: report, never set. + if (controlledExpandedKeys === undefined) { + setOwnExpandedKeys(next); + } + onExpandCb?.(willExpand, record); + onExpandedRowsChange?.(next); + }, + [ + expandedKeys, + controlledExpandedKeys, + ownExpandedKeys, + onExpandCb, + onExpandedRowsChange, + ], + ); + + /* + * The untouched key behind `row.id`. `getRowId` stringifies whatever `rowKey` + * resolves to, so this is the only way back to the value the call-site + * actually holds — falling back to the row index, which is what `getRowId` + * itself uses when the record carries no key. + */ + const originalRowKey = React.useCallback( + (record, index) => { + const raw = + typeof rowKey === "function" ? rowKey(record) : record?.[rowKey]; + return raw === undefined || raw === null ? index : raw; + }, + [rowKey], ); + + /* + * antd hides the toggle column for `showExpandColumn: false` — the idiom for + * a table driven entirely by its own controls, which is how HITL opens a + * cell's nested table from a button inside the cell. + */ + const showExpandColumn = canExpand && expandable?.showExpandColumn !== false; + const expandIcon = expandable?.expandIcon; + const cols = React.useMemo(() => { + const base = toColumns(columns, rowSelection); + if (!showExpandColumn) { + return base; + } + return [ + { + id: "__expand", + header: () => null, + enableSorting: false, + size: 48, + cell: ({ row }) => { + const record = row.original; + if (!isRowExpandable(record)) { + return null; + } + const expanded = expandedKeys.has(row.id); + const onExpand = (event) => { + // The row itself may carry an onRow click handler. + event?.stopPropagation?.(); + toggleExpanded(row.id, record, originalRowKey(record, row.index)); + }; + if (typeof expandIcon === "function") { + return expandIcon({ + expanded, + record, + onExpand: (_record, event) => onExpand(event), + }); + } + return ( + + ); + }, + }, + ...base, + ]; + }, [ + columns, + rowSelection, + showExpandColumn, + expandIcon, + expandedKeys, + isRowExpandable, + toggleExpanded, + originalRowKey, + ]); const leaves = React.useMemo(() => leafColumns(columns), [columns]); /* @@ -858,35 +1008,65 @@ function DataTable({ ) : table.getRowModel().rows.length ? ( - table.getRowModel().rows.map((row) => ( - - {row.getVisibleCells().map((cell) => ( - { + const expanded = + expandedKeys.has(row.id) && isRowExpandable(row.original); + return ( + /* + * The expanded row is a SIBLING , not a nested one: a + * table row may only contain cells, so antd's full-width + * panel has to be its own row spanning every column. + */ + + - {flexRender( - cell.column.columnDef.cell, - cell.getContext(), - )} - - ))} - - )) + {row.getVisibleCells().map((cell) => ( + + {flexRender( + cell.column.columnDef.cell, + cell.getContext(), + )} + + ))} + + {expanded ? ( + + + {expandedRowRender(row.original, row.index, 0, true)} + + + ) : null} + + ); + }) ) : ( { expect(screen.getByText("plan: LLM Whisperer Free")).toBeInTheDocument(); }); }); + +/* + * `expandable` was the widest of the silently dropped antd props: the whole + * object fell into `...props` and onto the wrapper
, so `expandedRowRender` + * was never called. HITL's review editor shows a truncated JSON blob plus an + * expand button for an array or object inside a table cell, and clicking that + * button did nothing whatsoever — nested values were unreadable in Table view + * (UN-4124). + */ +describe("DataTable expandable", () => { + const detail = (record) =>
detail for {record.name}
; + + it("renders the expanded row for a controlled expandedRowKeys", () => { + render( + , + ); + expect(screen.getByText("detail for Row 2")).toBeInTheDocument(); + expect(screen.queryByText("detail for Row 1")).not.toBeInTheDocument(); + }); + + it("matches numeric keys against the string row ids TanStack produces", () => { + // A call-site numbering its rows `key: index` passes numbers, and + // `[0].includes("0")` is false — the mismatch that hid every expansion. + render( + , + ); + expect(screen.getByText("detail for Row 1")).toBeInTheDocument(); + }); + + it("spans every column so the panel is full width", () => { + render( + , + ); + const panel = screen.getByText("detail for Row 1").closest("td"); + // Two data columns plus the toggle column the shim adds. + expect(panel).toHaveAttribute("colspan", "3"); + }); + + it("honours rowExpandable", () => { + render( + record.id === 1, + }} + />, + ); + expect(screen.getByText("detail for Row 1")).toBeInTheDocument(); + expect(screen.queryByText("detail for Row 2")).not.toBeInTheDocument(); + }); + + it("toggles from its own column when the keys are uncontrolled", async () => { + const user = userEvent.setup(); + const onExpand = vi.fn(); + render( + , + ); + expect(screen.queryByText("detail for Row 1")).not.toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Expand row" })); + expect(screen.getByText("detail for Row 1")).toBeInTheDocument(); + expect(onExpand).toHaveBeenCalledWith( + true, + expect.objectContaining({ id: 1 }), + ); + + await user.click(screen.getByRole("button", { name: "Collapse row" })); + await waitFor(() => + expect(screen.queryByText("detail for Row 1")).not.toBeInTheDocument(), + ); + }); + + it("leaves the toggle column out for showExpandColumn: false", () => { + render( + , + ); + expect(screen.getByText("detail for Row 1")).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /row$/ }), + ).not.toBeInTheDocument(); + }); + + it("reports a controlled toggle without moving on its own", async () => { + const user = userEvent.setup(); + const onExpandedRowsChange = vi.fn(); + render( + , + ); + await user.click(screen.getByRole("button", { name: "Expand row" })); + /* + * The caller's own key type, not the string TanStack normalizes it to: + * these rows key on a numeric `id`, and a parent testing `includes(1)` + * against a reported `["1"]` would never match. + */ + expect(onExpandedRowsChange).toHaveBeenCalledWith([1]); + // The parent did not move its keys, so neither did the table. + expect(screen.queryByText("detail for Row 1")).not.toBeInTheDocument(); + }); + + it("keeps the caller's key types when collapsing too", async () => { + const user = userEvent.setup(); + const onExpandedRowsChange = vi.fn(); + render( + , + ); + await user.click( + screen.getAllByRole("button", { name: "Collapse row" })[0], + ); + // The surviving key keeps its number type rather than being stringified. + expect(onExpandedRowsChange).toHaveBeenCalledWith([2]); + }); + + it("leaves the prop off the DOM", () => { + const { container } = render( + , + ); + expect(container.querySelector("[expandable]")).toBeNull(); + }); +}); diff --git a/frontend/src/components/ui/shims/antd-structure.test.jsx b/frontend/src/components/ui/shims/antd-structure.test.jsx index d3fd8e6e16..575bef3106 100644 --- a/frontend/src/components/ui/shims/antd-structure.test.jsx +++ b/frontend/src/components/ui/shims/antd-structure.test.jsx @@ -6,7 +6,13 @@ import { within, } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { createRef, forwardRef, useEffect, useImperativeHandle } from "react"; +import { + createRef, + forwardRef, + useEffect, + useImperativeHandle, + useState, +} from "react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { @@ -1736,3 +1742,62 @@ describe("antd-compatible structural shims (P4)", () => { }); }); }); + +/* + * HITL's review editor drives expansion from a button inside the cell, not + * from antd's toggle column, so it passes `showExpandColumn: false` and a + * controlled `expandedRowKeys` of row indices. The shim dropped `expandable` + * entirely, so the panel never rendered: a customer reviewing a MARS + * certificate saw `{"vendor":"Frut…` with an expand button that did nothing, + * and no way to read the nested values in Table view (UN-4124). + */ +describe("Table expandable (UN-4124)", () => { + const row = { + key: 0, + headerInfo: { vendor: "Fruta" }, + sap_mapping: [{ hitl_flag: false }], + testResults: [{ result: "230" }], + }; + const columns = ["headerInfo", "sap_mapping", "testResults"].map((c) => ({ + title: c, + dataIndex: c, + key: c, + render: (v) => JSON.stringify(v).slice(0, 12), + })); + + function Harness() { + const [expanded, setExpanded] = useState({}); + return ( + expanded[k]) + .map(Number), + rowExpandable: (record) => Boolean(expanded[record.key]), + expandedRowRender: (record) => ( +
expanded {JSON.stringify(record.sap_mapping)}
+ ), + }} + onRow={(record) => ({ + onClick: () => setExpanded({ [record.key]: true }), + })} + /> + ); + } + + it("opens the nested panel when the call-site sets its own expanded keys", async () => { + const user = userEvent.setup(); + render(); + expect(screen.queryByText(/^expanded /)).not.toBeInTheDocument(); + + await user.click(screen.getByText('{"vendor":"F')); + expect( + await screen.findByText('expanded [{"hitl_flag":false}]'), + ).toBeInTheDocument(); + }); +});