diff --git a/.changeset/tree-rows-in-custom-components.md b/.changeset/tree-rows-in-custom-components.md new file mode 100644 index 0000000000..2dbfb08daf --- /dev/null +++ b/.changeset/tree-rows-in-custom-components.md @@ -0,0 +1,7 @@ +--- +"@frontify/fondue-components": minor +"@frontify/fondue": minor +--- + +feat(Tree): allow rows inside custom components and fragments +fix(Tree): the first row is reachable with Tab when rows arrive after mount diff --git a/packages/components/src/components/Tree/Tree.metadata.json b/packages/components/src/components/Tree/Tree.metadata.json index 8cdd4a6947..0ed08712cb 100644 --- a/packages/components/src/components/Tree/Tree.metadata.json +++ b/packages/components/src/components/Tree/Tree.metadata.json @@ -1,7 +1,7 @@ { "category": "navigation", "description": "A hierarchical list of items and nested folders. Supports keyboard navigation, single-select with a highlighted row or multi-select with cascading folder checkboxes (folders without loaded children are checkable as their own entity), optional drag-and-drop reordering, inline renaming, and async loading rows.", - "instructions": "Compose with Tree.Root wrapping Tree.Item leaves and Tree.Folder nodes. Each row needs a stable `id` and declares its content via anatomy parts: (plain text, required) plus optional (rows render no icon otherwise), for passive badges/status icons hugging the label text (clicks bubble to the row; hidden while renaming), and for trailing controls (overflow menu, edit/delete; clicks do not bubble to the row). For items the parts are direct children; for folders they go inside , and every other folder child is a nested row. Render as a child of a folder (or of Tree.Root for the top-level slot) to show a translated 'Loading…' row while async content is fetched. Pass `multiSelect` for cascading checkboxes and `reorderable` to enable drag-and-drop; gate drops with an `accepts` predicate on Tree.Root or Tree.Folder, and tag draggable rows via the `tags` prop so the predicate can match by category instead of identity. State is controlled: render from the latest `TreeChangeState` and persist it from `onChange`. `isSelected` is the unified selection prop — with `multiSelect` it checks the row's checkbox, without `multiSelect` it highlights the row, and either mode emits the updated flag via `onChange`. In `multiSelect`, a folder with children derives its checkbox from its contents and ignores its own `isSelected`; a folder with no loaded children (empty, or collapsed while lazy-loading) is instead checkable as its own entity — `isSelected` is honored, `onSelectChange` fires on the folder, and it counts as one unit toward ancestor states. When the children of a checked folder load later, the consumer has to carry the selected state over to the new items by passing `isSelected` to all of them. Inline renaming is controlled too: provide `onRename` and flip `isRenaming` (e.g. from a Tree.Action menu) to swap the label for a text input — Enter or blur commits (firing `onRename` and `onChange` with the new name), Escape cancels, and `onRenamingChange(false)` signals the consumer to clear its flag. Per-item `onSelectChange`, `onExpandChange`, and `onMove` callbacks are fanned out from the global `onChange` when individual rows need to react to their own state changing.", + "instructions": "Compose with Tree.Root wrapping Tree.Item leaves and Tree.Folder nodes. Each row needs a stable `id` and declares its content via anatomy parts: (plain text, required) plus optional (rows render no icon otherwise), for passive badges/status icons hugging the label text (clicks bubble to the row; hidden while renaming), and for trailing controls (overflow menu, edit/delete; clicks do not bubble to the row). For items the parts are direct children; for folders they go inside , and every other folder child is a nested row. Rows may also sit inside your own components or fragments, with four limits: and row parts stay direct children of their folder or item; rows rendered through a portal are not found; row parts (Icon, Decorator, Action) render under Tree.Root, so they do not see a context provider placed around rows; when server-rendered, custom components add React useLayoutEffect warnings. Render as a child of a folder (or of Tree.Root for the top-level slot) to show a translated 'Loading…' row while async content is fetched. Pass `multiSelect` for cascading checkboxes and `reorderable` to enable drag-and-drop; gate drops with an `accepts` predicate on Tree.Root or Tree.Folder, and tag draggable rows via the `tags` prop so the predicate can match by category instead of identity. State is controlled: render from the latest `TreeChangeState` and persist it from `onChange`. `isSelected` is the unified selection prop — with `multiSelect` it checks the row's checkbox, without `multiSelect` it highlights the row, and either mode emits the updated flag via `onChange`. In `multiSelect`, a folder with children derives its checkbox from its contents and ignores its own `isSelected`; a folder with no loaded children (empty, or collapsed while lazy-loading) is instead checkable as its own entity — `isSelected` is honored, `onSelectChange` fires on the folder, and it counts as one unit toward ancestor states. When the children of a checked folder load later, the consumer has to carry the selected state over to the new items by passing `isSelected` to all of them. Inline renaming is controlled too: provide `onRename` and flip `isRenaming` (e.g. from a Tree.Action menu) to swap the label for a text input — Enter or blur commits (firing `onRename` and `onChange` with the new name), Escape cancels, and `onRenamingChange(false)` signals the consumer to clear its flag. Per-item `onSelectChange`, `onExpandChange`, and `onMove` callbacks are fanned out from the global `onChange` when individual rows need to react to their own state changing.", "name": "Tree", "relatedComponents": ["Accordion", "OrderableList", "Dropdown"], "storyFilePaths": ["src/components/Tree/Tree.stories.tsx"], diff --git a/packages/components/src/components/Tree/Tree.stories.tsx b/packages/components/src/components/Tree/Tree.stories.tsx index 6071cb633e..67c686a01f 100644 --- a/packages/components/src/components/Tree/Tree.stories.tsx +++ b/packages/components/src/components/Tree/Tree.stories.tsx @@ -9,7 +9,7 @@ import { IconTrashBin, } from '@frontify/fondue-icons'; import { type Meta, type StoryObj } from '@storybook/react-vite'; -import { useState, type ReactNode } from 'react'; +import { useEffect, useState, type ReactNode } from 'react'; import { action } from 'storybook/actions'; import { Badge, Button, Dropdown } from '#/index'; @@ -1272,6 +1272,64 @@ export const LoadMore: Story = { }, }; +const FolderContents = ({ folderId }: { folderId: string }) => { + const [names, setNames] = useState(null); + + useEffect(() => { + const timer = setTimeout(() => setNames(['q1.pdf', 'q2.pdf', 'q3.pdf']), 600); + return () => clearTimeout(timer); + }, []); + + if (!names) { + return ; + } + return ( + <> + {names.map((name) => ( + + + + + {name} + + ))} + + ); +}; + +export const RowsInCustomComponents: Story = { + parameters: { + docs: { + description: { + story: + 'Rows may sit inside your own components and fragments, so a component can own the data for a folder ' + + 'and render its rows, as `FolderContents` does here with its own fetch state. Limits: `Tree.FolderHeader` ' + + 'and row parts stay direct children of their folder or item. Rows rendered through a portal are not ' + + 'found. Row parts (`Tree.Icon`, `Tree.Decorator`, `Tree.Action`) render under `Tree.Root`, so they do ' + + 'not see a context provider placed around rows. When the tree is server-rendered, custom components ' + + 'add React `useLayoutEffect` warnings.', + }, + }, + }, + render: (args) => { + const [isExpanded, setIsExpanded] = useState(false); + + return ( + + + + + + + Reports + + {isExpanded && } + + + ); + }, +}; + export const DisabledRows: Story = { parameters: { docs: { diff --git a/packages/components/src/components/Tree/components/TreeCollector.spec.tsx b/packages/components/src/components/Tree/components/TreeCollector.spec.tsx new file mode 100644 index 0000000000..85ea00c6f4 --- /dev/null +++ b/packages/components/src/components/Tree/components/TreeCollector.spec.tsx @@ -0,0 +1,224 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { act, render, screen } from '@testing-library/react'; +import { Profiler, type ReactNode, useEffect, useState } from 'react'; +import { renderToString } from 'react-dom/server'; +import { describe, expect, it, vi } from 'vitest'; + +import { Tree } from '../Tree'; + +import { COLLECT_ATTR } from './TreeCollector'; + +const Leaf = ({ id }: { id: string }) => ( + + {id} + +); + +let treeRowRenders = 0; +vi.mock(import('./TreeRow'), async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + TreeRow: (props: Parameters[0]) => { + treeRowRenders += 1; + return mod.TreeRow(props); + }, + }; +}); + +const Row = ({ id }: { id: string }) => ( + + {id} + +); + +const Rows = ({ ids }: { ids: string[] }) => ( + <> + {ids.map((id) => ( + + ))} + +); + +const GrowingRows = () => { + const [ids, setIds] = useState(['a', 'b', 'c']); + useEffect(() => { + const timer = setTimeout(() => setIds((prev) => [...prev, 'd']), 10); + return () => clearTimeout(timer); + }, []); + return ; +}; + +const Pair = () => ( + <> + + + +); + +describe('TreeCollector', () => { + it('renders to a string without throwing when rows sit inside a custom component', () => { + expect(() => + renderToString( + + + , + ), + ).not.toThrow(); + }); + + it('adds no commit on re-render when no custom components are used', () => { + let commits = 0; + // A fresh element each call, so React cannot bail out on referential equality. + const ui = () => ( + { + commits += 1; + }} + > + + + A + + + + ); + const { rerender } = render(ui()); + commits = 0; + rerender(ui()); + expect(commits).toBe(1); + }); + + it('does not reuse rows from an earlier collect pass', () => { + const Wrap = ({ children }: { children: ReactNode }) => <>{children}; + const onRenamingChange = vi.fn(); + const ui = (wrapped: boolean, isRenaming: boolean) => { + const row = ( + {}} isRenaming={isRenaming} onRenamingChange={onRenamingChange}> + x + + ); + if (wrapped) { + return ( + + {row} + + ); + } + return {row}; + }; + const { rerender } = render(ui(true, true)); + rerender(ui(false, false)); + expect(onRenamingChange).toHaveBeenCalledWith(false); + onRenamingChange.mockClear(); + rerender(ui(true, false)); + expect(onRenamingChange).not.toHaveBeenCalled(); + }); + + it('collects rows of custom components after mount', () => { + render( + + + A + + + , + ); + expect(screen.getAllByRole('treeitem').map((row) => row.textContent?.trim())).toEqual(['A', 'B', 'C']); + expect(document.querySelectorAll(`[${COLLECT_ATTR}]`)).toHaveLength(3); + }); + + it('keeps fragment rows of a folder distinct in the collect pass', () => { + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const ui = (showX: boolean) => ( + + + + + f + + {showX && ( + + x + + )} + <> + + a + + + a2 + + + <> + + b + + + b2 + + + + + ); + const { rerender } = render(ui(true)); + rerender(ui(false)); + expect(screen.getAllByRole('treeitem').map((row) => row.textContent?.trim())).toEqual([ + 'z', + 'f', + 'a', + 'a2', + 'b', + 'b2', + ]); + expect(spy.mock.calls.some((call) => String(call[0]).includes('same key'))).toBe(false); + spy.mockRestore(); + }); + + describe('collect pass render cost', () => { + it('renders rows twice per consumer commit at most', () => { + const ui = () => ( + + + + ); + const { rerender } = render(ui()); + treeRowRenders = 0; + rerender(ui()); + expect(treeRowRenders / 3).toBeLessThanOrEqual(2); + }); + + it('renders rows no more often than the static path when a row is added', () => { + const staticUi = (ids: string[]) => ( + + {ids.map((id) => ( + + {id} + + ))} + + ); + const staticView = render(staticUi(['a', 'b', 'c'])); + treeRowRenders = 0; + staticView.rerender(staticUi(['a', 'b', 'c', 'd'])); + const staticRenders = treeRowRenders; + staticView.unmount(); + + vi.useFakeTimers(); + render( + + + , + ); + treeRowRenders = 0; + act(() => { + vi.runAllTimers(); + }); + expect(screen.getAllByRole('treeitem')).toHaveLength(4); + expect(treeRowRenders).toBeLessThanOrEqual(staticRenders); + vi.useRealTimers(); + }); + }); +}); diff --git a/packages/components/src/components/Tree/components/TreeCollector.tsx b/packages/components/src/components/Tree/components/TreeCollector.tsx new file mode 100644 index 0000000000..e739e1d899 --- /dev/null +++ b/packages/components/src/components/Tree/components/TreeCollector.tsx @@ -0,0 +1,138 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { createContext, useContext, useId, useLayoutEffect, useMemo, useRef, useState, type ReactNode } from 'react'; +import { flushSync } from 'react-dom'; + +import { ROOT_ID } from '../constants'; +import { type TreeFolderProps, type TreeItemData, type TreeItemProps } from '../types'; +import { type ParsedChildren, toFolderRowData, toItemData } from '../utils/parseChildren'; + +export const COLLECT_ATTR = 'data-tree-collect-key'; + +type CollectedEntry = + | { kind: 'item'; parentId: string; props: TreeItemProps } + | { kind: 'folder'; parentId: string; props: TreeFolderProps } + | { kind: 'loading'; parentId: string }; + +type RowEntry = Exclude; + +export type CollectStore = { + entries: Map; + requestFlush: () => void; +}; + +const TreeCollectContext = createContext(null); +TreeCollectContext.displayName = 'TreeCollectContext'; +export const TreeParentContext = createContext(ROOT_ID); +TreeParentContext.displayName = 'TreeParentContext'; + +/** + * Registers a row rendered inside the collect pass. Returns the marker key and whether a + * collect pass is active; outside one, Tree parts stay inert markers read by `parseChildren`. + */ +export const useCollectedEntry = ( + build: (parentId: string) => CollectedEntry, +): { key: string; isCollecting: boolean } => { + const store = useContext(TreeCollectContext); + const parentId = useContext(TreeParentContext); + const markerId = useId(); + useLayoutEffect(() => { + if (!store) { + return; + } + store.entries.set(markerId, build(parentId)); + store.requestFlush(); + return () => { + store.entries.delete(markerId); + store.requestFlush(); + }; + }); + return { key: markerId, isCollecting: store !== null }; +}; + +export const TreeCollector = ({ + onCollect, + children, +}: { + onCollect: (parsed: ParsedChildren) => void; + children: ReactNode; +}) => { + const containerRef = useRef(null); + const observerRef = useRef(null); + const onCollectRef = useRef(onCollect); + const [flushTick, setFlushTick] = useState(0); + const store = useMemo( + () => ({ entries: new Map(), requestFlush: () => setFlushTick((tick) => tick + 1) }), + [], + ); + + useLayoutEffect(() => { + onCollectRef.current = onCollect; + }); + + useLayoutEffect(() => { + if (!containerRef.current) { + return; + } + onCollectRef.current(buildCollectedItems(containerRef.current, store.entries)); + // Mutations this build already covered must not trigger another flush. + observerRef.current?.takeRecords(); + }, [flushTick, store]); + + useLayoutEffect(() => { + const container = containerRef.current; + if (!container) { + return; + } + // Rows reordered with unchanged props move their markers without re-rendering; flush before paint. + // eslint-disable-next-line @eslint-react/dom-no-flush-sync + const observer = new MutationObserver(() => flushSync(() => store.requestFlush())); + observer.observe(container, { childList: true, subtree: true }); + observerRef.current = observer; + return () => { + observer.disconnect(); + observerRef.current = null; + }; + }, [store]); + + return ( + + ); +}; + +/** Builds the flat item list from the markers' document order. */ +export const buildCollectedItems = (container: HTMLElement, entries: Map): ParsedChildren => { + const loadingParents = new Set(); + for (const entry of entries.values()) { + if (entry.kind === 'loading') { + loadingParents.add(entry.parentId); + } + } + const ordered: RowEntry[] = []; + for (const marker of container.querySelectorAll(`[${COLLECT_ATTR}]`)) { + const entry = entries.get(marker.getAttribute(COLLECT_ATTR) ?? ''); + if (entry && entry.kind !== 'loading') { + ordered.push(entry); + } + } + const childIdsByParent = new Map(); + for (const entry of ordered) { + const siblings = childIdsByParent.get(entry.parentId) ?? []; + siblings.push(entry.props.id); + childIdsByParent.set(entry.parentId, siblings); + } + const items: TreeItemData[] = ordered.map((entry) => { + if (entry.kind === 'folder') { + return toFolderRowData( + entry.props, + entry.parentId, + childIdsByParent.get(entry.props.id) ?? [], + loadingParents.has(entry.props.id), + ); + } + return toItemData(entry.props, entry.parentId); + }); + return { items, parentIsLoading: loadingParents.has(ROOT_ID), hasForeignRows: true }; +}; diff --git a/packages/components/src/components/Tree/components/TreeFolder.spec.ts b/packages/components/src/components/Tree/components/TreeFolder.spec.tsx similarity index 56% rename from packages/components/src/components/Tree/components/TreeFolder.spec.ts rename to packages/components/src/components/Tree/components/TreeFolder.spec.tsx index 7dfda53e61..b03f17ab25 100644 --- a/packages/components/src/components/Tree/components/TreeFolder.spec.ts +++ b/packages/components/src/components/Tree/components/TreeFolder.spec.tsx @@ -1,12 +1,14 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ +import { render } from '@testing-library/react'; import { describe, expect, it } from 'vitest'; import { TreeFolder } from './TreeFolder'; describe('TreeFolder', () => { - it('renders null', () => { - expect(TreeFolder({ id: 'f', children: null })).toBe(null); + it('renders nothing outside a Tree collect pass', () => { + const { container } = render({null}); + expect(container).toBeEmptyDOMElement(); }); it('declares displayName="Tree.Folder"', () => { diff --git a/packages/components/src/components/Tree/components/TreeFolder.ts b/packages/components/src/components/Tree/components/TreeFolder.ts deleted file mode 100644 index d7070bd033..0000000000 --- a/packages/components/src/components/Tree/components/TreeFolder.ts +++ /dev/null @@ -1,6 +0,0 @@ -/* (c) Copyright Frontify Ltd., all rights reserved. */ - -import { type TreeFolderProps } from '../types'; - -export const TreeFolder = (_props: TreeFolderProps): null => null; -TreeFolder.displayName = 'Tree.Folder'; diff --git a/packages/components/src/components/Tree/components/TreeFolder.tsx b/packages/components/src/components/Tree/components/TreeFolder.tsx new file mode 100644 index 0000000000..ea4a69fd7e --- /dev/null +++ b/packages/components/src/components/Tree/components/TreeFolder.tsx @@ -0,0 +1,19 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { type TreeFolderProps } from '../types'; + +import { COLLECT_ATTR, TreeParentContext, useCollectedEntry } from './TreeCollector'; + +export const TreeFolder = (props: TreeFolderProps) => { + const { key, isCollecting } = useCollectedEntry((parentId) => ({ kind: 'folder', parentId, props })); + if (!isCollecting) { + return null; + } + return ( + <> + + {props.children} + + ); +}; +TreeFolder.displayName = 'Tree.Folder'; diff --git a/packages/components/src/components/Tree/components/TreeItem.spec.ts b/packages/components/src/components/Tree/components/TreeItem.spec.tsx similarity index 70% rename from packages/components/src/components/Tree/components/TreeItem.spec.ts rename to packages/components/src/components/Tree/components/TreeItem.spec.tsx index 70d54617ac..58877a49b1 100644 --- a/packages/components/src/components/Tree/components/TreeItem.spec.ts +++ b/packages/components/src/components/Tree/components/TreeItem.spec.tsx @@ -1,5 +1,6 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ +import { render } from '@testing-library/react'; import { describe, expect, it } from 'vitest'; import { TreeItem } from './TreeItem'; @@ -11,8 +12,9 @@ import { TreeItem } from './TreeItem'; */ describe('TreeItem', () => { - it('renders null', () => { - expect(TreeItem({ id: 'x', children: 'X' })).toBe(null); + it('renders nothing outside a Tree collect pass', () => { + const { container } = render(X); + expect(container).toBeEmptyDOMElement(); }); it('declares displayName="Tree.Item"', () => { diff --git a/packages/components/src/components/Tree/components/TreeItem.ts b/packages/components/src/components/Tree/components/TreeItem.ts deleted file mode 100644 index dc05863787..0000000000 --- a/packages/components/src/components/Tree/components/TreeItem.ts +++ /dev/null @@ -1,6 +0,0 @@ -/* (c) Copyright Frontify Ltd., all rights reserved. */ - -import { type TreeItemProps } from '../types'; - -export const TreeItem = (_props: TreeItemProps): null => null; -TreeItem.displayName = 'Tree.Item'; diff --git a/packages/components/src/components/Tree/components/TreeItem.tsx b/packages/components/src/components/Tree/components/TreeItem.tsx new file mode 100644 index 0000000000..109906d7e2 --- /dev/null +++ b/packages/components/src/components/Tree/components/TreeItem.tsx @@ -0,0 +1,14 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { type TreeItemProps } from '../types'; + +import { COLLECT_ATTR, useCollectedEntry } from './TreeCollector'; + +export const TreeItem = (props: TreeItemProps) => { + const { key, isCollecting } = useCollectedEntry((parentId) => ({ kind: 'item', parentId, props })); + if (!isCollecting) { + return null; + } + return ; +}; +TreeItem.displayName = 'Tree.Item'; diff --git a/packages/components/src/components/Tree/components/TreeLoading.spec.ts b/packages/components/src/components/Tree/components/TreeLoading.spec.tsx similarity index 59% rename from packages/components/src/components/Tree/components/TreeLoading.spec.ts rename to packages/components/src/components/Tree/components/TreeLoading.spec.tsx index 3311af1336..f435cf974d 100644 --- a/packages/components/src/components/Tree/components/TreeLoading.spec.ts +++ b/packages/components/src/components/Tree/components/TreeLoading.spec.tsx @@ -1,12 +1,14 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ +import { render } from '@testing-library/react'; import { describe, expect, it } from 'vitest'; import { TreeLoading } from './TreeLoading'; describe('TreeLoading', () => { - it('renders null', () => { - expect(TreeLoading()).toBe(null); + it('renders nothing outside a Tree collect pass', () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); }); it('declares displayName="Tree.Loading"', () => { diff --git a/packages/components/src/components/Tree/components/TreeLoading.ts b/packages/components/src/components/Tree/components/TreeLoading.ts deleted file mode 100644 index 52d8eda338..0000000000 --- a/packages/components/src/components/Tree/components/TreeLoading.ts +++ /dev/null @@ -1,4 +0,0 @@ -/* (c) Copyright Frontify Ltd., all rights reserved. */ - -export const TreeLoading = (): null => null; -TreeLoading.displayName = 'Tree.Loading'; diff --git a/packages/components/src/components/Tree/components/TreeLoading.tsx b/packages/components/src/components/Tree/components/TreeLoading.tsx new file mode 100644 index 0000000000..7348bd0855 --- /dev/null +++ b/packages/components/src/components/Tree/components/TreeLoading.tsx @@ -0,0 +1,9 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { useCollectedEntry } from './TreeCollector'; + +export const TreeLoading = (): null => { + useCollectedEntry((parentId) => ({ kind: 'loading', parentId })); + return null; +}; +TreeLoading.displayName = 'Tree.Loading'; diff --git a/packages/components/src/components/Tree/components/TreeRoot.tsx b/packages/components/src/components/Tree/components/TreeRoot.tsx index b77b5c43d9..8c12123abf 100644 --- a/packages/components/src/components/Tree/components/TreeRoot.tsx +++ b/packages/components/src/components/Tree/components/TreeRoot.tsx @@ -1,7 +1,7 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ import { AssistiveTreeDescription } from '@headless-tree/react'; -import { Fragment, useId, useMemo, type ReactNode } from 'react'; +import { Fragment, useId, useMemo, useState, type ReactNode } from 'react'; import { useTranslation } from '#/hooks/useTranslation'; @@ -11,13 +11,20 @@ import { type TreeChangeState, type TreeDropCandidate } from '../types'; import { computeCheckedStates, getCheckedUnitIds } from '../utils/computeCheckedStates'; import { computeLoadingInsertions } from '../utils/computeLoadingInsertions'; import { isNoopDrop } from '../utils/isNoopDrop'; -import { parseChildren } from '../utils/parseChildren'; +import { type ParsedChildren, parseChildren } from '../utils/parseChildren'; +import { TreeCollector } from './TreeCollector'; import { TreeDragLine } from './TreeDragLine'; import { TreeLoadingRow } from './TreeLoadingRow'; import { TreeRow } from './TreeRow'; export type TreeRootProps = { + /** + * Rows may sit inside custom components and fragments. Limits: `Tree.FolderHeader` and row + * parts stay direct children of their folder or item; rows rendered through a portal are not + * found; row parts (Icon, Decorator, Action) do not see a context provider placed around + * rows; server rendering adds React `useLayoutEffect` warnings for custom components. + */ children: ReactNode; /** Fires with the full tree state, at most once per user interaction. */ onChange?: (state: TreeChangeState) => void; @@ -64,7 +71,15 @@ export const TreeRoot = ({ }: TreeRootProps) => { const { t } = useTranslation(); const rowHintId = useId(); - const { items, parentIsLoading: rootIsLoading } = useMemo(() => parseChildren(children), [children]); + const parsed = useMemo(() => parseChildren(children), [children]); + // Rows inside custom components are found only by rendering `children` in a hidden + // collect pass; the static parse stays the first-render (and server) value. + const [collected, setCollected] = useState(null); + // A snapshot from an earlier collect pass must not seed the next one. + if (!parsed.hasForeignRows && collected !== null) { + setCollected(null); + } + const { items, parentIsLoading: rootIsLoading } = parsed.hasForeignRows && collected ? collected : parsed; const tree = useTreeController({ items, onChange, @@ -94,7 +109,7 @@ export const TreeRoot = ({ .filter(Boolean) .join(' '); - return ( + const treeElement = (
{rowHint && ( @@ -135,5 +150,13 @@ export const TreeRoot = ({ )}
); + + // One stable shape with the tree first: mounting the collect pass never remounts it or shifts `:first-child`. + return ( + <> + {treeElement} + {parsed.hasForeignRows && {children}} + + ); }; TreeRoot.displayName = 'TreeRoot'; diff --git a/packages/components/src/components/Tree/components/TreeWrapper.ct.tsx b/packages/components/src/components/Tree/components/TreeWrapper.ct.tsx new file mode 100644 index 0000000000..284ea24fea --- /dev/null +++ b/packages/components/src/components/Tree/components/TreeWrapper.ct.tsx @@ -0,0 +1,205 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { expect, test } from '@playwright/experimental-ct-react'; + +import { Tree } from '../Tree'; + +import { + AllWrapperRoot, + ContextWrapperTree, + DeferredMixedRoot, + LazyFoldersTree, + LazyWrapperTree, + MixedRoot, + MultiSelectWrapperTree, + NestedWrapperTree, + ReorderWrapperTree, + TimedReorderWrapperTree, + ToggleWrapperTree, +} from './testutils/WrapperFixtures'; + +const rowNames = (component: { getByRole: (role: 'treeitem') => { allInnerTexts: () => Promise } }) => + component.getByRole('treeitem').allInnerTexts(); + +test.describe('Tree rows inside custom components', () => { + test('keeps document order when wrapped rows sit between direct rows', async ({ mount }) => { + const component = await mount(); + await expect(component.getByRole('treeitem')).toHaveCount(4); + const names = await rowNames(component); + expect(names.map((text) => text.trim())).toEqual(['A', 'B', 'C', 'D']); + await expect(component.locator('[role="tree"]')).toHaveCount(1); + const isFirst = await component + .locator('[role="tree"]') + .evaluate((el) => el.parentElement?.firstElementChild === el); + expect(isFirst).toBe(true); + }); + + test('shows wrapped rows in the same task as direct rows (no intermediate paint)', async ({ mount, page }) => { + const component = await mount(); + await page.evaluate(() => { + const win = window as unknown as { firstSeen?: number }; + const observer = new MutationObserver(() => { + const count = document.querySelectorAll('[role="treeitem"]').length; + if (count > 0 && win.firstSeen === undefined) { + win.firstSeen = count; + observer.disconnect(); + } + }); + observer.observe(document.body, { childList: true, subtree: true }); + }); + await component.getByRole('button', { name: 'show' }).click(); + await expect.poll(() => page.evaluate(() => (window as unknown as { firstSeen?: number }).firstSeen)).toBe(4); + }); + + test('makes the first wrapped row the tab stop when every root row is wrapped', async ({ mount, page }) => { + const component = await mount(); + await component.getByRole('button', { name: 'before' }).focus(); + await page.keyboard.press('Tab'); + await expect(component.getByRole('treeitem', { name: 'X' })).toBeFocused(); + }); + + test('loads folder children owned by a component with its own state', async ({ mount }) => { + const component = await mount(); + await component.getByRole('treeitem', { name: /Lazy/ }).click(); + await expect(component.getByText('Loading')).toBeVisible(); + await expect(component.getByRole('treeitem', { name: 'lazy-1' })).toBeVisible(); + await expect(component.getByRole('treeitem', { name: 'lazy-2' })).toHaveAttribute('aria-level', '2'); + await expect(component.getByText('Loading')).toHaveCount(0); + }); + + test('keeps keyboard focus on the folder when expanding mounts its first custom component', async ({ + mount, + page, + }) => { + const component = await mount(); + const folder = component.getByRole('treeitem', { name: /Lazy/ }); + await folder.focus(); + await page.keyboard.press('ArrowRight'); + await expect(component.getByRole('treeitem', { name: 'lazy-1' })).toBeVisible(); + await expect(folder).toBeFocused(); + await expect(folder).toHaveAttribute('aria-expanded', 'true'); + }); + + test('keeps focus when a new folder expands after an earlier collect pass ended', async ({ mount, page }) => { + const component = await mount(); + await component.getByRole('treeitem', { name: /^a$/ }).click(); + await expect(component.getByRole('treeitem', { name: 'a-1' })).toBeVisible(); + await component.getByRole('treeitem', { name: /^a$/ }).click(); + await expect(component.getByRole('treeitem', { name: 'a-1' })).toHaveCount(0); + await component.getByRole('button', { name: 'add b' }).click(); + const folder = component.getByRole('treeitem', { name: /^b$/ }); + await component.getByRole('treeitem', { name: /^a$/ }).focus(); + await page.keyboard.press('ArrowDown'); + await expect(folder).toBeFocused(); + await page.keyboard.press('ArrowRight'); + await expect(component.getByRole('treeitem', { name: 'b-1' })).toBeVisible(); + await expect(folder).toBeFocused(); + }); + + test('nests folders declared through recursive components', async ({ mount }) => { + const component = await mount(); + await expect(component.getByRole('treeitem', { name: 'deep-leaf' })).toHaveAttribute('aria-level', '4'); + await expect(component.getByRole('treeitem', { name: 'after' })).toHaveAttribute('aria-level', '1'); + const names = await rowNames(component); + expect(names.map((text) => text.trim())).toEqual(['level-3', 'level-2', 'level-1', 'deep-leaf', 'after']); + }); + + test('follows a wrapper re-rendering on its own and unmounting', async ({ mount }) => { + const component = await mount(); + await expect(component.getByRole('treeitem', { name: 'g3' })).toBeVisible(); + await component.getByRole('button', { name: 'toggle' }).click(); + await expect(component.getByRole('treeitem')).toHaveCount(1); + await expect(component.getByRole('treeitem', { name: 'static' })).toBeVisible(); + }); + + test('follows a reorder of wrapped rows with unchanged props', async ({ mount }) => { + const component = await mount(); + await expect(component.getByRole('treeitem')).toHaveCount(2); + await component.getByRole('button', { name: 'reverse' }).click(); + await expect + .poll(async () => { + const names = await rowNames(component); + return names.map((text) => text.trim()); + }) + .toEqual(['b', 'a']); + }); + + test('paints a timer-driven reorder of wrapped rows without a stale frame', async ({ mount, page }) => { + const component = await mount(); + await expect(component.getByRole('treeitem')).toHaveCount(2); + await page.evaluate(() => { + const win = window as unknown as { frames_?: string[] }; + const frames: string[] = []; + win.frames_ = frames; + document.querySelector('button')!.click(); + const tick = () => { + const value = [...document.querySelectorAll('[role="treeitem"]')] + .map((el) => el.textContent?.trim()) + .join(','); + if (document.querySelector('[data-order="b,a"]')) { + frames.push(value); + } + if (value !== 'b,a' && frames.length < 200) { + requestAnimationFrame(tick); + } + }; + requestAnimationFrame(tick); + }); + await expect + .poll(() => + page.evaluate(() => { + const frames = (window as unknown as { frames_?: string[] }).frames_ ?? []; + return frames[frames.length - 1]; + }), + ) + .toBe('b,a'); + const frames = await page.evaluate(() => (window as unknown as { frames_?: string[] }).frames_ ?? []); + expect(frames.filter((value) => value === 'a,b')).toHaveLength(0); + }); + + test('cascades a folder checkbox to wrapped children', async ({ mount }) => { + const component = await mount(); + await component + .getByRole('treeitem', { name: /parent/ }) + .getByRole('checkbox') + .click(); + await expect(component.getByTestId('selected')).toHaveText('c1,c2'); + await expect(component.getByRole('treeitem', { name: 'c1' })).toHaveAttribute('aria-checked', 'true'); + }); + + test('renders row parts under Tree.Root, outside a wrapper context provider', async ({ mount }) => { + const component = await mount(); + await expect(component.getByRole('treeitem', { name: /ctx-row/ }).getByTestId('ctx')).toHaveText('outside'); + }); +}); + +test.describe('Tree without custom components', () => { + test('renders no collect pass', async ({ mount, page }) => { + await mount( + + + One + + , + ); + await expect(page.locator('[data-tree-collect-key]')).toHaveCount(0); + await expect(page.locator('[aria-hidden="true"][hidden]')).toHaveCount(0); + }); + + test('opens fragments statically', async ({ mount, page }) => { + const component = await mount( + + <> + + One + + + Two + + + , + ); + await expect(component.getByRole('treeitem')).toHaveCount(2); + await expect(page.locator('[data-tree-collect-key]')).toHaveCount(0); + }); +}); diff --git a/packages/components/src/components/Tree/components/testutils/WrapperFixtures.tsx b/packages/components/src/components/Tree/components/testutils/WrapperFixtures.tsx new file mode 100644 index 0000000000..1fe1ae384a --- /dev/null +++ b/packages/components/src/components/Tree/components/testutils/WrapperFixtures.tsx @@ -0,0 +1,270 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { Children, type ReactNode, createContext, useContext, useEffect, useState } from 'react'; + +import { Tree } from '../../Tree'; + +const Leaf = ({ + id, + isSelected, + onSelectChange, +}: { + id: string; + isSelected?: boolean; + onSelectChange?: (value: boolean) => void; +}) => ( + + {id} + +); + +const Pair = ({ a, b }: { a: string; b: string }) => ( + <> + + + +); + +export const MixedRoot = () => ( + + + A + + + + D + + +); + +export const DeferredMixedRoot = () => { + const [isShown, setIsShown] = useState(false); + return ( + <> + + {isShown && } + + ); +}; + +export const AllWrapperRoot = () => ( + <> + + + + + +); + +const FolderContents = ({ folderId }: { folderId: string }) => { + const [ids, setIds] = useState(null); + useEffect(() => { + const timer = setTimeout(() => setIds([`${folderId}-1`, `${folderId}-2`]), 400); + return () => clearTimeout(timer); + }, [folderId]); + if (!ids) { + return ; + } + return ids.map((id) => ); +}; + +export const LazyWrapperTree = () => { + const [isExpanded, setIsExpanded] = useState(false); + return ( + + + + Lazy + + {isExpanded && } + + + ); +}; + +export const LazyFoldersTree = () => { + const [folderIds, setFolderIds] = useState(['a']); + const [expanded, setExpanded] = useState>(() => new Set()); + const toggle = (id: string, value: boolean) => { + setExpanded((previous) => { + const next = new Set(previous); + if (value) { + next.add(id); + } else { + next.delete(id); + } + return next; + }); + }; + return ( + <> + + + {folderIds.map((id) => ( + toggle(id, value)} + > + + {id} + + {expanded.has(id) && } + + ))} + + + ); +}; + +const SubTree = ({ depth }: { depth: number }) => { + if (depth === 0) { + return ; + } + return ( + + + {`level-${depth}`} + + + + ); +}; + +export const NestedWrapperTree = () => ( + + + + +); + +const GrowingWrapper = () => { + const [ids, setIds] = useState(['g1', 'g2']); + useEffect(() => { + const timer = setTimeout(() => setIds((prev) => [...prev, 'g3']), 400); + return () => clearTimeout(timer); + }, []); + return ids.map((id) => ); +}; + +export const ToggleWrapperTree = () => { + const [isShown, setIsShown] = useState(true); + return ( + <> + + + + {isShown && } + + + ); +}; + +const SelectableChildren = ({ + selected, + onToggle, +}: { + selected: Set; + onToggle: (id: string, value: boolean) => void; +}) => + ['c1', 'c2'].map((id) => ( + onToggle(id, value)} /> + )); + +export const MultiSelectWrapperTree = () => { + const [selected, setSelected] = useState>(() => new Set()); + const onToggle = (id: string, value: boolean) => + setSelected((prev) => { + const next = new Set(prev); + if (value) { + next.add(id); + } else { + next.delete(id); + } + return next; + }); + return ( + <> + {[...selected].sort().join(',')} + + + + parent + + + + + + ); +}; + +const Reverse = ({ children, isReversed }: { children: ReactNode; isReversed: boolean }) => { + const items = Children.toArray(children); + return ( + <> + + {isReversed ? items.reverse() : items} + + ); +}; + +const reorderItems = [ + + a + , + + b + , +]; + +export const ReorderWrapperTree = () => { + const [isReversed, setIsReversed] = useState(false); + return ( + <> + + + {reorderItems} + + + ); +}; + +export const TimedReorderWrapperTree = () => { + const [isReversed, setIsReversed] = useState(false); + return ( + <> + + + {reorderItems} + + + ); +}; + +const ProbeContext = createContext('outside'); +ProbeContext.displayName = 'ProbeContext'; +const ContextReader = () => {useContext(ProbeContext)}; + +export const ContextWrapperTree = () => ( + + + + ctx-row + + + + + + +); diff --git a/packages/components/src/components/Tree/hooks/useTreeController.ts b/packages/components/src/components/Tree/hooks/useTreeController.ts index 05eabc8b5a..d3992ce71b 100644 --- a/packages/components/src/components/Tree/hooks/useTreeController.ts +++ b/packages/components/src/components/Tree/hooks/useTreeController.ts @@ -263,7 +263,8 @@ export const useTreeController = ({ getItem: (itemId) => itemsById.get(itemId) as TreeItemData, getChildren: (itemId) => itemsById.get(itemId)?.children ?? [], }, - state: { ...treeState, renamingItem, renamingValue }, + // `null`, not `undefined`, lets headless-tree fall back to the first row as tab stop. + state: { ...treeState, focusedItem: treeState.focusedItem ?? null, renamingItem, renamingValue }, setExpandedItems, setCheckedItems, setSelectedItems: multiSelect ? undefined : setSelectedItems, diff --git a/packages/components/src/components/Tree/utils/parseChildren.spec.tsx b/packages/components/src/components/Tree/utils/parseChildren.spec.tsx index 33c07252e1..18e714e8db 100644 --- a/packages/components/src/components/Tree/utils/parseChildren.spec.tsx +++ b/packages/components/src/components/Tree/utils/parseChildren.spec.tsx @@ -213,4 +213,22 @@ describe('parseChildren', () => { ]); expect(result.items.map((entry) => entry.id)).toEqual(['1']); }); + + it('ignores misplaced Tree parts without flagging foreign rows', () => { + const result = parseChildren([ + stray, + + h + , + + + f + + i + {item('a', 'a')} + , + ]); + expect(result.hasForeignRows).toBe(false); + expect(result.items.map((entry) => entry.id)).toEqual(['f', 'a']); + }); }); diff --git a/packages/components/src/components/Tree/utils/parseChildren.ts b/packages/components/src/components/Tree/utils/parseChildren.ts index 76359271f7..6b777789cc 100644 --- a/packages/components/src/components/Tree/utils/parseChildren.ts +++ b/packages/components/src/components/Tree/utils/parseChildren.ts @@ -1,6 +1,6 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ -import { Children, isValidElement, type ReactElement, type ReactNode } from 'react'; +import { Children, Fragment, isValidElement, type ReactElement, type ReactNode } from 'react'; import { ROOT_ID } from '../constants'; import { @@ -22,6 +22,11 @@ export type ParsedChildren = { * for `TreeRoot`'s root loading row. */ parentIsLoading: boolean; + /** + * `true` when a row position holds an element that is not a Tree part (a custom + * component or host element). Such rows can only be found by rendering them. + */ + hasForeignRows: boolean; }; const hasDisplayName = @@ -36,7 +41,15 @@ const isTreeActionElement = hasDisplayName('Tree.Action'); const isTreeDecoratorElement = hasDisplayName('Tree.Decorator'); const isTreeIconElement = hasDisplayName('Tree.Icon'); const isTreeLabelElement = hasDisplayName('Tree.Label'); -const isTreeFolderHeaderElement = hasDisplayName('Tree.FolderHeader'); +export const isTreeFolderHeaderElement = hasDisplayName('Tree.FolderHeader'); + +// `Children.toArray` keeps fragments as single elements, so open them here. +export const flattenChildren = (children: ReactNode): ReactNode[] => + Children.toArray(children).flatMap((child) => + isValidElement<{ children?: ReactNode }>(child) && child.type === Fragment + ? flattenChildren(child.props.children) + : [child], + ); type RowParts = { /** Text from ``; empty string when the part is missing. */ @@ -98,7 +111,7 @@ const sharedRowData = (props: TreeItemProps | TreeFolderProps, parentId: string) isDisabled: props.isDisabled, }); -const toItemData = (props: TreeItemProps, parentId: string): TreeItemData => { +export const toItemData = (props: TreeItemProps, parentId: string): TreeItemData => { const { name, icon, decorator, action } = extractRowParts(props.children); return { ...sharedRowData(props, parentId), @@ -115,29 +128,40 @@ type FolderParse = { descendants: TreeItemData[]; }; -const toFolderData = (props: TreeFolderProps, parentId: string): FolderParse => { +export const getFolderRows = (children: ReactNode): ReactNode[] => + flattenChildren(children).filter((child) => !(isValidElement(child) && isTreeFolderHeaderElement(child))); + +export const toFolderRowData = ( + props: TreeFolderProps, + parentId: string, + children: string[], + isLoading: boolean, +): TreeItemData => { // Row parts live in ``; everything else is nested rows. - const headerElement = Children.toArray(props.children).filter(isValidElement).find(isTreeFolderHeaderElement); + const headerElement = flattenChildren(props.children).filter(isValidElement).find(isTreeFolderHeaderElement); const { name, icon, decorator, action } = extractRowParts(headerElement?.props.children); - const rows = Children.toArray(props.children).filter( - (child) => !(isValidElement(child) && isTreeFolderHeaderElement(child)), - ); - const nested = parseChildren(rows, props.id); return { - folder: { - ...sharedRowData(props, parentId), - name, - isFolder: true, - children: nested.items.filter((item) => item.parentId === props.id).map((item) => item.id), - isExpanded: props.isExpanded, - onExpandChange: props.onExpandChange, - icon, - decorator, - actions: action, - isLoading: nested.parentIsLoading, - accepts: props.accepts, - }, + ...sharedRowData(props, parentId), + name, + isFolder: true, + children, + isExpanded: props.isExpanded, + onExpandChange: props.onExpandChange, + icon, + decorator, + actions: action, + isLoading, + accepts: props.accepts, + }; +}; + +const toFolderData = (props: TreeFolderProps, parentId: string): FolderParse & { hasForeignRows: boolean } => { + const nested = parseChildren(getFolderRows(props.children), props.id); + const childIds = nested.items.filter((item) => item.parentId === props.id).map((item) => item.id); + return { + folder: toFolderRowData(props, parentId, childIds, nested.parentIsLoading), descendants: nested.items, + hasForeignRows: nested.hasForeignRows, }; }; @@ -151,8 +175,9 @@ const toFolderData = (props: TreeFolderProps, parentId: string): FolderParse => export const parseChildren = (children: ReactNode, parentId: string = ROOT_ID): ParsedChildren => { const items: TreeItemData[] = []; let parentIsLoading = false; + let hasForeignRows = false; - for (const child of Children.toArray(children)) { + for (const child of flattenChildren(children)) { if (!isValidElement(child)) { continue; } @@ -165,10 +190,23 @@ export const parseChildren = (children: ReactNode, parentId: string = ROOT_ID): continue; } if (isTreeFolderElement(child)) { - const { folder, descendants } = toFolderData(child.props, parentId); + const { folder, descendants, hasForeignRows: nestedForeign } = toFolderData(child.props, parentId); items.push(folder, ...descendants); + hasForeignRows ||= nestedForeign; + continue; + } + if ( + isTreeLabelElement(child) || + isTreeIconElement(child) || + isTreeDecoratorElement(child) || + isTreeActionElement(child) || + isTreeFolderHeaderElement(child) + ) { + // Misplaced row parts never hold rows; ignore them as before. + continue; } + hasForeignRows = true; } - return { items, parentIsLoading }; + return { items, parentIsLoading, hasForeignRows }; };