diff --git a/packages/react/src/patterns/OneDataCollection/__stories__/visualizations/table/table.mdx b/packages/react/src/patterns/OneDataCollection/__stories__/visualizations/table/table.mdx index f802fd8759..6f51566884 100644 --- a/packages/react/src/patterns/OneDataCollection/__stories__/visualizations/table/table.mdx +++ b/packages/react/src/patterns/OneDataCollection/__stories__/visualizations/table/table.mdx @@ -909,10 +909,13 @@ nestedTable.isExpanded("node-42") nestedTable.getExpandedItems() ``` -`expand`, `collapse` and `toggle` target rows by item `id` or with a -predicate, and apply to the rows currently rendered. `expandAll` installs a -policy that also applies to rows revealed later, and an explicit `collapse` -always wins over an active policy. +`expand`, `collapse` and `toggle` target rows by their identity or with a +predicate, and apply to the rows currently rendered. The identity is +resolved like everywhere else in the collection: `source.idProvider(item)` +when the source defines one, otherwise the item's `id` (string-normalized, +so `42` and `"42"` are equivalent; identities must be unique among sibling +rows). `expandAll` installs a policy that also applies to rows revealed +later, and an explicit `collapse` always wins over an active policy. `expandTo` expands a specific path of item ids (ancestor → descendant) without touching the rest of the tree. Ids not rendered yet stay pending and diff --git a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/__tests__/NestedExpansionControl.test.tsx b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/__tests__/NestedExpansionControl.test.tsx index 476c4cd106..e4ac851eb2 100644 --- a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/__tests__/NestedExpansionControl.test.tsx +++ b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/__tests__/NestedExpansionControl.test.tsx @@ -825,6 +825,63 @@ describe("Nested table expansion control", () => { expect(screen.getByText("Child Six")).toBeInTheDocument() }) }) + + it("restores the policy's eager load mode when re-expanding via expandTo", async () => { + const pending: Array<() => void> = [] + const fetchChildren = vi.fn( + ({ + item, + pagination, + }: { + item: Person + pagination?: ChildrenPaginationInfo + }) => + new Promise((resolve) => { + pending.push(() => { + const all = item.children ?? [] + const currentPage = (pagination?.currentPage ?? 0) + 1 + const start = (currentPage - 1) * CHILDREN_PER_PAGE + resolve({ + records: all.slice(start, start + CHILDREN_PER_PAGE), + paginationInfo: { + total: all.length, + perPage: CHILDREN_PER_PAGE, + currentPage, + pagesCount: Math.ceil(all.length / CHILDREN_PER_PAGE), + hasMore: currentPage * CHILDREN_PER_PAGE < all.length, + }, + }) + }) + }) + ) + const table = renderNestedTable( + { + defaultExpanded: (ctx) => ctx.item.id === "p2", + defaultExpandedChildren: "all", + }, + { fetchChildren } as unknown as Partial + ) + await waitForRootRows() + await waitFor(() => expect(fetchChildren).toHaveBeenCalledTimes(1)) + + // Collapse while page 1 is in flight, then let it land in the cache. + act(() => table.control.collapse("p2")) + await act(async () => { + pending.shift()?.() + }) + + // Re-expanding via expandTo WITHOUT options must also clear the stale + // collapse marker and restore the policy's eager mode (parity with + // expand/toggle): the remaining page loads instead of "show more". + act(() => table.control.expandTo(["p2"])) + await waitFor(() => expect(fetchChildren).toHaveBeenCalledTimes(2)) + await act(async () => { + pending.shift()?.() + }) + await waitFor(() => { + expect(screen.getByText("Child Six")).toBeInTheDocument() + }) + }) }) describe("user interaction", () => { @@ -1086,6 +1143,95 @@ describe("Nested table expansion control", () => { }) expect(screen.getByText("A1a")).toBeInTheDocument() }) + + it("keeps a row expanded when its siblings are reordered by a refetch", async () => { + // Stable filter/navigation references: swapping the dataAdapter below + // must NOT look like a filters change (which resets overrides by + // design) — only the data order changes. + const stableSourceBits = { + currentFilters: {}, + currentNavigationFilters: {}, + } as unknown as Partial + let api!: { + control: NestedTableController + setOverrides: (overrides: Partial) => void + } + render( + { + api = a + }} + /> + ) + await waitForRootRows() + + act(() => api.control.expand("p2")) + await waitFor(() => { + expect(screen.getByText("Child Three")).toBeInTheDocument() + }) + + // A refetch returns the same items in reverse order (no filters or + // sortings change, so no expansion reset is involved): the row keys + // by identity, not by position, so p2 must stay expanded. + act(() => + api.setOverrides({ + dataAdapter: { + fetchData: async () => ({ records: [...tree].reverse() }), + }, + } as unknown as Partial) + ) + await waitFor(() => { + const parents = screen.getAllByText(/^Parent/) + expect(parents[0]).toHaveTextContent("Parent Two") + }) + expect(screen.getByText("Child Three")).toBeInTheDocument() + }) + + it("targets rows through source.idProvider when the items have no id", async () => { + type PersonByEmail = { + name: string + email: string + children?: PersonByEmail[] + } + const people: PersonByEmail[] = [ + { + name: "Alice", + email: "alice@corp.com", + children: [{ name: "Alice Jr", email: "alice.jr@corp.com" }], + }, + { + name: "Bob", + email: "bob@corp.com", + children: [{ name: "Bob Jr", email: "bob.jr@corp.com" }], + }, + ] + const table = renderNestedTable(undefined, { + idProvider: (item: PersonByEmail) => item.email, + dataAdapter: { fetchData: async () => ({ records: people }) }, + fetchChildren: ({ item }: { item: PersonByEmail }) => ({ + records: item.children ?? [], + paginationInfo: { + total: item.children?.length ?? 0, + perPage: 10, + currentPage: 1, + pagesCount: 1, + hasMore: false, + }, + }), + } as unknown as Partial) + await waitFor(() => { + expect(screen.getByText("Bob")).toBeInTheDocument() + }) + + // The univocal column designated by idProvider is the target identity + act(() => table.control.expand("bob@corp.com")) + await waitFor(() => { + expect(screen.getByText("Bob Jr")).toBeInTheDocument() + }) + expect(screen.queryByText("Alice Jr")).not.toBeInTheDocument() + expect(table.control.isExpanded("bob@corp.com")).toBe(true) + }) }) describe("stale fetches and misbehaving adapters", () => { @@ -1150,6 +1296,57 @@ describe("Nested table expansion control", () => { }) }) + it("retries a failed children fetch when the search changes", async () => { + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => {}) + const fetchChildren = vi.fn(({ search }: { search?: string }) => { + if (search === undefined) return Promise.reject(new Error("boom")) + return Promise.resolve({ + records: [{ id: `c-${search}`, name: `Child ${search}` }], + paginationInfo: { + total: 1, + perPage: 2, + currentPage: 1, + pagesCount: 1, + hasMore: false, + }, + }) + }) + let api!: { + control: NestedTableController + setSearch: (search: string | undefined) => void + } + render( + ctx.item.id === "p1" }} + fetchChildren={ + fetchChildren as unknown as TestSource["fetchChildren"] + } + onApi={(a) => { + api = a + }} + /> + ) + await waitForRootRows() + await waitFor(() => expect(fetchChildren).toHaveBeenCalledTimes(1)) + // Let the rejection settle into hasError (no retry loop: still 1 call) + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + expect(fetchChildren).toHaveBeenCalledTimes(1) + + // The search change clears the settled error and must re-arm the + // auto-load guard: the policy-open row retries with the new term + // instead of staying open and empty forever. + act(() => api.setSearch("Grand")) + await waitFor(() => expect(fetchChildren).toHaveBeenCalledTimes(2)) + await waitFor(() => { + expect(screen.getByText("Child Grand")).toBeInTheDocument() + }) + consoleError.mockRestore() + }) + it("stops eager loading when a page adds no records despite hasMore: true", async () => { const fetchChildren = vi.fn(() => ({ records: [], diff --git a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/components/NestedRow.tsx b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/components/NestedRow.tsx index 8167c51bbc..4e071c918e 100644 --- a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/components/NestedRow.tsx +++ b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/components/NestedRow.tsx @@ -162,13 +162,26 @@ const NestedRowContent = < const sentinelRef = useRef(null) const addRow = useAddRow() + // Data identity resolved the same way as the rest of the collection + // (Kanban's getKey, item navigation): `source.idProvider` when the source + // defines one, otherwise the item's `id`. Undefined for id-less items, + // which key by position and cannot be targeted by id from the controller. + const itemKey = props.source.idProvider + ? String(props.source.idProvider(props.item, props.index)) + : "id" in props.item && + props.item.id !== undefined && + props.item.id !== null + ? String(props.item.id) + : undefined + // Row identity for the expansion overrides, the children cache and the - // controller registry. Prefixed with the parent's rowId (or the group for - // root rows) so equal depth/index positions in different branches or - // groups can never collide — e.g. the first id-less child of two expanded - // parents used to share the same key. + // controller registry: the parent row's id (or the group for roots) + // namespaces the item's own identity, so equal positions in different + // branches or groups can never collide and — unlike a positional key — + // the expansion survives sibling reorders. Identities must be unique + // among siblings; id-less items fall back to their position. const parentKey = props.nestedRowProps?.parentRowId ?? `g${props.groupIndex}` - const rowId = `${parentKey}/${props.nestedRowProps?.depth ?? 0}-${"id" in props.item ? props.item.id + "-" + props.index : props.index}` + const rowId = `${parentKey}/${itemKey ?? `i${props.index}`}` const { setRowExpanded, @@ -196,8 +209,8 @@ const NestedRowContent = < // and onExpandedChange must see current field values, not the first-render // snapshot. Re-registering on item identity change is two cheap Map ops. useEffect( - () => registerNestedRow(rowId, props.item, depth), - [registerNestedRow, rowId, props.item, depth] + () => registerNestedRow(rowId, props.item, depth, itemKey), + [registerNestedRow, rowId, props.item, depth, itemKey] ) /** @@ -236,13 +249,21 @@ const NestedRowContent = < // since `hasFetched` never became true in that case. const previousHasFetchedRef = useRef(hasFetched) const previousIsLoadingRef = useRef(isLoading) + const previousHasErrorRef = useRef(hasError) useEffect(() => { const fetchInvalidated = previousHasFetchedRef.current && !hasFetched const fetchCancelled = previousIsLoadingRef.current && !isLoading && !hasFetched && !hasError + // A filters/search reset also clears a settled error (hasError drops to + // false with nothing fetched and nothing in flight — a new fetch clears + // it too, but with isLoading true): re-arm so a row kept open by a + // policy retries with the new context instead of staying empty forever. + const errorCleared = + previousHasErrorRef.current && !hasError && !hasFetched && !isLoading previousHasFetchedRef.current = hasFetched previousIsLoadingRef.current = isLoading - if (fetchInvalidated || fetchCancelled) { + previousHasErrorRef.current = hasError + if (fetchInvalidated || fetchCancelled || errorCleared) { autoLoadRequestedRef.current = false } }, [hasFetched, isLoading, hasError]) diff --git a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/nested/internal-types.ts b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/nested/internal-types.ts index 09aefcdbd6..0246ddb29b 100644 --- a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/nested/internal-types.ts +++ b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/nested/internal-types.ts @@ -67,6 +67,13 @@ export interface NestedTableControllerInternal< export type NestedRowRegistryEntry = { item: R depth: number + /** + * Resolved data identity — `source.idProvider(item)` when the source + * defines one (same convention as Kanban and item navigation), otherwise + * the item's `id`. Controller targets match against it. `undefined` for + * id-less items, which key by position and cannot be targeted by id. + */ + itemKey?: string } /** diff --git a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/nested/types.ts b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/nested/types.ts index 24011586fd..faf79d31e1 100644 --- a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/nested/types.ts +++ b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/nested/types.ts @@ -41,7 +41,11 @@ export type NestedExpansionCriteria = /** * Identifies the row(s) an imperative operation applies to. - * - `string | number`: matched against the item's `id` property + * - `string | number`: matched against the item's identity — + * `source.idProvider(item)` when the source defines one (same convention + * as the Kanban visualization and item navigation), otherwise the item's + * `id` property. The comparison is string-normalized, so `42` and `"42"` + * are equivalent. Identities must be unique among sibling rows. * - predicate: matched against every currently rendered expandable row * * Only rows that are currently rendered can be targeted — a row hidden under diff --git a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/providers/NestedProvider.tsx b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/providers/NestedProvider.tsx index 39878d2c34..deac3a0fc4 100644 --- a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/providers/NestedProvider.tsx +++ b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/providers/NestedProvider.tsx @@ -51,7 +51,12 @@ interface NestedDataContextValue { depth: number ) => ResolvedRowExpansion /** Registers a rendered expandable row so imperative operations can target it. Returns an unregister cleanup. */ - registerNestedRow: (rowId: string, item: R, depth: number) => () => void + registerNestedRow: ( + rowId: string, + item: R, + depth: number, + itemKey?: string + ) => () => void expandAnimation: NestedExpandAnimation } @@ -89,9 +94,10 @@ const matchesTarget = ( if (typeof target === "function") { return target({ item: entry.item, depth: entry.depth, hasActiveFilters }) } - // Normalized comparison (same as the table's own getRowKey) so a string - // target — e.g. an id read from a URL param — matches numeric item ids. - return "id" in entry.item && String(entry.item.id) === String(target) + // Matched against the row's resolved data identity (source.idProvider or + // item.id, stringified) so a string target — e.g. an id read from a URL + // param — matches numeric item ids too. + return entry.itemKey !== undefined && entry.itemKey === String(target) } const buildExpandAllCriteria = ( @@ -203,9 +209,10 @@ export const NestedDataProvider = ({ const clearFetchedData = useCallback(() => { setFetchedData({}) - // Explicit overrides are positional (depth-id-index) so they cannot be - // trusted after a refetch; the declarative policy still applies. Pending - // expandTo paths are dropped too, as the new data may not contain them. + // A filters/search change can reshape the dataset entirely, so explicit + // overrides are dropped and the declarative policy re-applies (documented + // behavior). Pending expandTo paths are dropped too, as the new data may + // not contain them. pendingExpandRef.current.clear() commitExpansionState({ overrides: {}, eager: {} }) }, [commitExpansionState]) @@ -244,16 +251,15 @@ export const NestedDataProvider = ({ /** * Consumes a pending `expandTo` entry for the given row, expanding it (and - * flagging eager loading when requested). No-op when the row's item id has - * no pending request. + * flagging eager loading when requested). No-op when the row's identity + * has no pending request (or the item has no identity at all). */ const applyPendingExpansion = useCallback( - (rowId: string, item: R, depth: number) => { - if (!("id" in item)) return - const itemId = String(item.id) - const pendingOptions = pendingExpandRef.current.get(itemId) + (rowId: string, item: R, depth: number, itemKey?: string) => { + if (itemKey === undefined) return + const pendingOptions = pendingExpandRef.current.get(itemKey) if (pendingOptions === undefined) return - pendingExpandRef.current.delete(itemId) + pendingExpandRef.current.delete(itemKey) const current = expansionStateRef.current const resolved = resolveExpansion(current, policyRef.current, rowId, { @@ -264,10 +270,17 @@ export const NestedDataProvider = ({ const eager = pendingOptions.children === "all" if (resolved.expanded && (!eager || resolved.eager)) return + // Same as setRowExpanded/applyToTargets: expanding clears a stale + // collapse marker so the policy's declared load mode applies again. + const nextEager = { ...current.eager } + if (eager) { + nextEager[rowId] = true + } else if (nextEager[rowId] === false) { + delete nextEager[rowId] + } commitExpansionState({ - ...current, overrides: { ...current.overrides, [rowId]: true }, - eager: eager ? { ...current.eager, [rowId]: true } : current.eager, + eager: nextEager, }) if (!resolved.expanded) emitExpandedChange(rowId, true) }, @@ -275,9 +288,9 @@ export const NestedDataProvider = ({ ) const registerNestedRow = useCallback( - (rowId: string, item: R, depth: number) => { - registryRef.current.set(rowId, { item, depth }) - applyPendingExpansion(rowId, item, depth) + (rowId: string, item: R, depth: number, itemKey?: string) => { + registryRef.current.set(rowId, { item, depth, itemKey }) + applyPendingExpansion(rowId, item, depth, itemKey) return () => { registryRef.current.delete(rowId) } @@ -388,7 +401,7 @@ export const NestedDataProvider = ({ // Apply immediately to the rows already rendered; the rest of the // path is consumed as lazy loading registers each revealed level. registryRef.current.forEach((entry, rowId) => - applyPendingExpansion(rowId, entry.item, entry.depth) + applyPendingExpansion(rowId, entry.item, entry.depth, entry.itemKey) ) }, isExpanded: (target) => {