diff --git a/packages/react/src/experimental/OneTable/TableCell/NestedCell/index.tsx b/packages/react/src/experimental/OneTable/TableCell/NestedCell/index.tsx index a8ea2ec289..9f64984595 100644 --- a/packages/react/src/experimental/OneTable/TableCell/NestedCell/index.tsx +++ b/packages/react/src/experimental/OneTable/TableCell/NestedCell/index.tsx @@ -3,6 +3,7 @@ import { ChevronDown, ChevronRight } from "lucide-react" import { F0Button } from "@/components/F0Button" import { F0ButtonDropdown } from "@/components/F0ButtonDropdown" import { Add, ArrowDown } from "@/icons/app" +import { useI18n } from "@/lib/providers/i18n" import { cn } from "@/lib/utils" import { NestedRowProps } from "@/patterns/OneDataCollection/visualizations/collection/Table/components/Row" @@ -38,6 +39,7 @@ export const NestedCell = ({ children, onClick, }: NestedCellProps) => { + const { collections } = useI18n() const firstCellWithChildren = isFirstCellWithChildren( firstCell, !!nestedRowProps?.rowWithChildren @@ -157,7 +159,7 @@ export const NestedCell = ({ variant="ghost" size="md" icon={ArrowDown} - label="See more" + label={collections.table.seeMoreChildren} onClick={(e) => { e.stopPropagation() onLoadMoreChildren?.() diff --git a/packages/react/src/lib/providers/i18n/i18n-provider-defaults.ts b/packages/react/src/lib/providers/i18n/i18n-provider-defaults.ts index 5711589b1f..9019f5fad3 100644 --- a/packages/react/src/lib/providers/i18n/i18n-provider-defaults.ts +++ b/packages/react/src/lib/providers/i18n/i18n-provider-defaults.ts @@ -242,6 +242,7 @@ export const defaultTranslations = { viewSelectorLabel: "Select view", }, table: { + seeMoreChildren: "See more", settings: { showAllColumns: "Show all", hideAllColumns: "Hide all", diff --git a/packages/react/src/patterns/OneDataCollection/__stories__/mockData.tsx b/packages/react/src/patterns/OneDataCollection/__stories__/mockData.tsx index 57a93cf607..c534072c58 100644 --- a/packages/react/src/patterns/OneDataCollection/__stories__/mockData.tsx +++ b/packages/react/src/patterns/OneDataCollection/__stories__/mockData.tsx @@ -78,6 +78,8 @@ import { import { OnBulkActionCallback } from "../types" import { Visualization, VisualizationType } from "../visualizations/collection" +const CHILDREN_PER_PAGE = 2 + // Mock data for nested subfilters (office → space → desk) const OFFICES = [ { id: 101, name: "Barcelona HQ" }, @@ -1549,23 +1551,30 @@ export const ExampleComponent = ({ dataAdapter: dataAdapterMemoized, itemsWithChildren: (item) => !!item?.children?.length, childrenCount: ({ item }) => item?.children?.length, - fetchChildren: async ({ item }) => { + fetchChildren: async ({ item, pagination }) => { await new Promise((resolve) => setTimeout(resolve, 1000)) - return item.children - ? { - records: item.children, - type: item.detailed ? "detailed" : "basic", - paginationInfo: { - cursor: "aaa", - total: item.children.length, - perPage: 2, - currentPage: 1, - pagesCount: 1, - hasMore: true, - }, - } - : { records: [] } + if (!item.children) return { records: [] } + + // A real page, not the whole list echoed back with a pinned `hasMore`: + // that shape made "See more" append the same records forever, so it + // never exercised the pagination it was meant to demo. + const perPage = pagination?.perPage ?? CHILDREN_PER_PAGE + const currentPage = (pagination?.currentPage ?? 0) + 1 + const start = (currentPage - 1) * perPage + const total = item.children.length + + return { + records: item.children.slice(start, start + perPage), + type: item.detailed ? "detailed" : "basic", + paginationInfo: { + total, + perPage, + currentPage, + pagesCount: Math.max(1, Math.ceil(total / perPage)), + hasMore: start + perPage < total, + }, + } }, lanes: [ { id: "eng", filters: { department: ["Engineering"] } }, diff --git a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/hooks/__tests__/useLoadChildren.test.tsx b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/hooks/__tests__/useLoadChildren.test.tsx new file mode 100644 index 0000000000..59b9e2513d --- /dev/null +++ b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/hooks/__tests__/useLoadChildren.test.tsx @@ -0,0 +1,251 @@ +import { act, renderHook } from "@testing-library/react" +import { ReactNode } from "react" +import { Observable } from "zen-observable-ts" +import { beforeEach, describe, expect, it, vi } from "vitest" + +import { RecordType } from "@/hooks/datasource" +import { ChildrenResponse } from "@/hooks/datasource/types/nested.typings" +import { DataCollectionSource } from "@/patterns/OneDataCollection/hooks/useDataCollectionSource/types" +import { PromiseState } from "@/lib/promise-to-observable" + +import { NestedDataProvider } from "../../providers/NestedProvider" +import { useLoadChildren } from "../useLoadChildren" + +type Row = RecordType & { id: string } + +const row = (id: string): Row => ({ id }) + +const PARENT = row("parent") + +/** + * A source whose children arrive as one long-lived observable per requested + * page, mirroring how a consumer keeps pushing updates for rows already on + * screen. `emit(page, ...)` pushes a new payload into a specific page's + * subscription, which is the case the flat-accumulator implementation could not + * express. + */ +const makeSource = (perPage: number) => { + const subscribers = new Map< + number, + ZenObservable.SubscriptionObserver>> + >() + const teardowns: number[] = [] + let all: Row[] = [] + + const payload = (page: number): ChildrenResponse => { + const start = (page - 1) * perPage + return { + records: all.slice(start, start + perPage), + type: "basic", + paginationInfo: { + total: all.length, + perPage, + currentPage: page, + pagesCount: Math.max(1, Math.ceil(all.length / perPage)), + hasMore: start + perPage < all.length, + }, + } + } + + const fetchChildren = vi.fn( + ({ pagination }: { pagination?: { currentPage: number } }) => { + const page = (pagination?.currentPage ?? 0) + 1 + + return new Observable>>( + (subscriber) => { + subscribers.set(page, subscriber) + subscriber.next({ + loading: false, + error: undefined, + data: payload(page), + }) + return () => { + teardowns.push(page) + subscribers.delete(page) + } + } + ) + } + ) + + return { + fetchChildren, + teardowns, + livePages: () => [...subscribers.keys()].sort((a, b) => a - b), + setChildren: (next: Row[]) => { + all = next + }, + /** Re-emit a page from the CURRENT children, as a realtime sync would. */ + emit: (page: number) => + subscribers + .get(page) + ?.next({ loading: false, error: undefined, data: payload(page) }), + source: (filters: object, sortings: object) => + ({ + fetchChildren, + currentFilters: filters, + currentSortings: sortings, + currentNavigationFilters: {}, + }) as unknown as DataCollectionSource< + Row, + never, + never, + never, + never, + never, + never + >, + } +} + +const wrapper = ({ children }: { children: ReactNode }) => ( + {children} +) + +const renderLoadChildren = ( + source: DataCollectionSource +) => + renderHook( + ({ source: currentSource }) => + useLoadChildren({ + rowId: "row-0", + item: PARENT, + source: currentSource, + onClearFetchedData: () => {}, + }), + { initialProps: { source }, wrapper } + ) + +describe("useLoadChildren", () => { + let fake: ReturnType + + beforeEach(() => { + fake = makeSource(2) + fake.setChildren([row("a"), row("b"), row("c"), row("d"), row("e")]) + }) + + const ids = (children: Row[]) => children.map((child) => child.id) + + it("appends each page and stops when the last one lands", async () => { + const { result } = renderLoadChildren(fake.source({}, {})) + + await act(async () => void result.current.loadChildren()) + expect(ids(result.current.children)).toEqual(["a", "b"]) + expect(result.current.paginationInfo?.hasMore).toBe(true) + + await act(async () => void result.current.loadChildren()) + expect(ids(result.current.children)).toEqual(["a", "b", "c", "d"]) + + await act(async () => void result.current.loadChildren()) + expect(ids(result.current.children)).toEqual(["a", "b", "c", "d", "e"]) + expect(result.current.paginationInfo?.hasMore).toBe(false) + }) + + it("keeps every loaded page subscribed", async () => { + const { result } = renderLoadChildren(fake.source({}, {})) + + await act(async () => void result.current.loadChildren()) + await act(async () => void result.current.loadChildren()) + + expect(fake.livePages()).toEqual([1, 2]) + expect(fake.teardowns).toEqual([]) + }) + + it("applies an update to a page loaded before the current one", async () => { + const { result } = renderLoadChildren(fake.source({}, {})) + + await act(async () => void result.current.loadChildren()) + await act(async () => void result.current.loadChildren()) + + fake.setChildren([row("a2"), row("b"), row("c"), row("d"), row("e")]) + await act(async () => void fake.emit(1)) + + expect(ids(result.current.children)).toEqual(["a2", "b", "c", "d"]) + }) + + it("does not duplicate a row that shifts across the page boundary", async () => { + const { result } = renderLoadChildren(fake.source({}, {})) + + await act(async () => void result.current.loadChildren()) + await act(async () => void result.current.loadChildren()) + expect(ids(result.current.children)).toEqual(["a", "b", "c", "d"]) + + // A team inserted at the top pushes "b" from page 1 into page 2. + fake.setChildren([ + row("new"), + row("a"), + row("b"), + row("c"), + row("d"), + row("e"), + ]) + await act(async () => void fake.emit(1)) + await act(async () => void fake.emit(2)) + + expect(ids(result.current.children)).toEqual(["new", "a", "b", "c"]) + }) + + it("drops rows removed from an earlier page", async () => { + const { result } = renderLoadChildren(fake.source({}, {})) + + await act(async () => void result.current.loadChildren()) + await act(async () => void result.current.loadChildren()) + + fake.setChildren([row("a"), row("b")]) + await act(async () => void fake.emit(1)) + await act(async () => void fake.emit(2)) + + expect(ids(result.current.children)).toEqual(["a", "b"]) + }) + + it("lets the frontier page own the pagination state", async () => { + const { result } = renderLoadChildren(fake.source({}, {})) + + await act(async () => void result.current.loadChildren()) + await act(async () => void result.current.loadChildren()) + await act(async () => void result.current.loadChildren()) + expect(result.current.paginationInfo?.currentPage).toBe(3) + expect(result.current.paginationInfo?.hasMore).toBe(false) + + // Page 1 re-emitting must not rewind the cursor and re-offer "See more". + await act(async () => void fake.emit(1)) + expect(result.current.paginationInfo?.currentPage).toBe(3) + expect(result.current.paginationInfo?.hasMore).toBe(false) + }) + + it("unsubscribes every page and starts over when the filters change", async () => { + const source = fake.source({}, {}) + const { result, rerender } = renderLoadChildren(source) + + await act(async () => void result.current.loadChildren()) + await act(async () => void result.current.loadChildren()) + + await act(async () => + rerender({ source: fake.source({ team: ["a"] }, {}) }) + ) + + expect(result.current.children).toEqual([]) + expect(result.current.paginationInfo).toBeUndefined() + expect(fake.livePages()).toEqual([]) + expect(fake.teardowns.sort()).toEqual([1, 2]) + + await act(async () => void result.current.loadChildren()) + expect(ids(result.current.children)).toEqual(["a", "b"]) + }) + + it("supersedes a duplicate request for the same page", async () => { + const { result } = renderLoadChildren(fake.source({}, {})) + + // A row opened by the default policy asks twice in the same tick: the + // chevron handler and the effect that covers rows opened without one. + await act(async () => { + result.current.loadChildren() + result.current.loadChildren() + }) + + expect(fake.fetchChildren).toHaveBeenCalledTimes(2) + expect(ids(result.current.children)).toEqual(["a", "b"]) + expect(fake.livePages()).toEqual([1]) + expect(fake.teardowns).toEqual([1]) + }) +}) diff --git a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/hooks/useLoadChildren.ts b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/hooks/useLoadChildren.ts index aca7fbdce1..9fc67387a0 100644 --- a/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/hooks/useLoadChildren.ts +++ b/packages/react/src/patterns/OneDataCollection/visualizations/collection/Table/hooks/useLoadChildren.ts @@ -95,15 +95,42 @@ export const useLoadChildren = < updateFetchedData, clearFetchedData, } = useNestedDataContext() - const [children, setChildren] = useState( - getChildren(nestedFetchedData?.[rowId]) - ) + + const restoredData = nestedFetchedData?.[rowId] + const restoredChildren = getChildren(restoredData) + + const [children, setChildren] = useState(restoredChildren) const [paginationInfo, setPaginationInfo] = useState< ChildrenPaginationInfo | undefined - >(nestedFetchedData?.[rowId]?.paginationInfo) + >(restoredData?.paginationInfo) const [isLoading, setIsLoading] = useState(false) const [childrenType, setChildrenType] = useState( - getChildrenType(nestedFetchedData?.[rowId]) + getChildrenType(restoredData) + ) + + // Children kept per requested page, so a page that re-emits REPLACES its own + // slice: one flat list made a re-emission append to whatever it held when that + // page subscribed, freezing earlier rows and duplicating reordered ones. + // Page 0 is the cache restored on remount — nothing live can re-emit it. + const pagesRef = useRef>( + new Map(restoredChildren.length > 0 ? [[0, restoredChildren]] : []) + ) + // Only the highest page loaded owns `hasMore`/`currentPage`, so an earlier page + // re-emitting cannot rewind the cursor. + const frontierRef = useRef<{ + page: number + type: NestedVariant + paginationInfo?: ChildrenPaginationInfo + }>({ + page: restoredData?.paginationInfo?.currentPage ?? 0, + type: getChildrenType(restoredData), + paginationInfo: restoredData?.paginationInfo, + }) + + // One subscription per page: loading the next page must not silence the earlier + // ones, which are how the consumer pushes updates for rows still on screen. + const subscriptionsRef = useRef>( + new Map() ) const previousFiltersRef = useRef(source.currentFilters) @@ -118,6 +145,17 @@ export const useLoadChildren = < previousNavigationFiltersRef.current !== source.currentNavigationFilters if (filtersChanged || sortingsChanged || navigationFiltersChanged) { + subscriptionsRef.current.forEach((subscription) => + subscription.unsubscribe() + ) + subscriptionsRef.current.clear() + pagesRef.current.clear() + frontierRef.current = { + page: 0, + type: "basic", + paginationInfo: undefined, + } + setChildren([]) setPaginationInfo(undefined) setChildrenType("basic") @@ -136,34 +174,48 @@ export const useLoadChildren = < onClearFetchedData, ]) - const subscriptionRef = useRef() - const processChildrenData = useCallback( - (data: ChildrenResponse | undefined) => { + (page: number, data: ChildrenResponse | undefined) => { const loadedChildren = getChildren(data) - const updatedChildren = [...children, ...loadedChildren] + pagesRef.current.set(page, loadedChildren) + + const updatedChildren = [...pagesRef.current.entries()] + .sort(([a], [b]) => a - b) + .flatMap(([, records]) => records) setChildren(updatedChildren) + if (page >= frontierRef.current.page) { + frontierRef.current = { + page, + type: getChildrenType(data), + paginationInfo: data?.paginationInfo, + } + setChildrenType(frontierRef.current.type) + setPaginationInfo(frontierRef.current.paginationInfo) + } + const updatedData: ChildrenResponse = { records: updatedChildren, - type: data?.type, - paginationInfo: data?.paginationInfo, + type: frontierRef.current.type, + paginationInfo: frontierRef.current.paginationInfo, } updateFetchedData(rowId, updatedData) - setChildrenType(getChildrenType(data)) - setPaginationInfo(data?.paginationInfo) return loadedChildren }, - [children, rowId, updateFetchedData] + [rowId, updateFetchedData] ) const loadChildren = useCallback(() => { if (children.length > 0 && !paginationInfo?.hasMore) return children - // Cancel any existing subscription - subscriptionRef.current?.unsubscribe() + // The page about to be requested — the same cursor handed to the consumer. + const page = (paginationInfo?.currentPage ?? 0) + 1 + + // Replace only THIS page's subscription, so earlier pages keep listening. + subscriptionsRef.current.get(page)?.unsubscribe() + subscriptionsRef.current.delete(page) setIsLoading(true) @@ -182,7 +234,7 @@ export const useLoadChildren = < // Handle synchronous data (not a Promise or Observable) if (!("then" in result) && !("subscribe" in result)) { - const loadedChildren = processChildrenData(result) + const loadedChildren = processChildrenData(page, result) setIsLoading(false) return loadedChildren } @@ -191,33 +243,38 @@ export const useLoadChildren = < const observable: Observable>> = "subscribe" in result ? result : promiseToObservable(result) - subscriptionRef.current = observable.subscribe({ - next: (state) => { - if (state.loading) { - setIsLoading(true) - } else if (state.error) { - setIsLoading(false) - } else if (state.data) { - processChildrenData(state.data) + subscriptionsRef.current.set( + page, + observable.subscribe({ + next: (state) => { + if (state.loading) { + setIsLoading(true) + } else if (state.error) { + setIsLoading(false) + } else if (state.data) { + processChildrenData(page, state.data) + setIsLoading(false) + } + }, + error: (error) => { setIsLoading(false) - } - }, - error: (error) => { - setIsLoading(false) - console.error("Error loading children:", error) - }, - complete: () => { - subscriptionRef.current = undefined - }, - }) + console.error("Error loading children:", error) + }, + complete: () => { + subscriptionsRef.current.delete(page) + }, + }) + ) return [] }, [children, item, source, paginationInfo, processChildrenData]) - // Cleanup subscription on unmount + // Cleanup subscriptions on unmount useEffect(() => { + const subscriptions = subscriptionsRef.current return () => { - subscriptionRef.current?.unsubscribe() + subscriptions.forEach((subscription) => subscription.unsubscribe()) + subscriptions.clear() } }, [])