diff --git a/packages/components/src/prototypes/contextMenuAsync/AsyncOptionMenu.browser.test.tsx b/packages/components/src/prototypes/contextMenuAsync/AsyncOptionMenu.browser.test.tsx new file mode 100644 index 0000000000..bcacb34cc7 --- /dev/null +++ b/packages/components/src/prototypes/contextMenuAsync/AsyncOptionMenu.browser.test.tsx @@ -0,0 +1,175 @@ +import { render } from "vitest-browser-react"; +import { page, userEvent } from "vitest/browser"; +import { AsyncOptionMenu } from "./AsyncOptionMenu"; +import { createFakeBackend } from "./lib/fakeBackend"; + +/* + * The point of these tests is the claim the proposal in #1851 rests on: the + * selection is independent of what is currently loaded or filtered. They drive + * the real prototype against a server-side-filtering backend, with no fake + * timers — the async paths are what is under test. + */ + +const backend = () => + createFakeBackend({ total: 60, pageSize: 10, latencyMs: 20 }); + +const openMenu = async () => { + await userEvent.click(page.getByRole("button", { name: /Labels/ })); +}; + +const searchFor = async (term: string) => { + await userEvent.fill(page.getByRole("searchbox"), term); +}; + +/** The keys of the most recent `onSelectionChange` call, sorted. */ +const lastSelection = (mock: { + mock: { lastCall?: readonly unknown[] }; +}): string[] => { + const keys = mock.mock.lastCall?.[0]; + + if (!(keys instanceof Set)) { + throw new Error("onSelectionChange was not called with a Set"); + } + + return [...keys].map(String).sort(); +}; + +test("selection survives a filter that hides the selected option", async () => { + const onSelectionChange = vitest.fn(); + + await render( + , + ); + + await openMenu(); + + // alpha-0 is on the first page. + const alpha = page.getByRole("menuitemcheckbox", { + name: "alpha-0", + exact: true, + }); + await expect.element(alpha).toBeInTheDocument(); + await userEvent.click(alpha); + + expect(lastSelection(onSelectionChange)).toEqual(["option-0"]); + + // Filter to something alpha-0 cannot match, then select a second option. + await searchFor("beta-1"); + const beta = page.getByRole("menuitemcheckbox", { + name: "beta-1", + exact: true, + }); + await expect.element(beta).toBeInTheDocument(); + + // alpha-0 is gone from the collection … + await expect + .element( + page.getByRole("menuitemcheckbox", { name: "alpha-0", exact: true }), + ) + .not.toBeInTheDocument(); + + await userEvent.click(beta); + + // … but it is still selected. + expect(lastSelection(onSelectionChange)).toEqual(["option-0", "option-1"]); +}); + +test("a selected option stays visible and uncheckable while filtered out (pin)", async () => { + await render( + , + ); + + await openMenu(); + + await userEvent.click( + page.getByRole("menuitemcheckbox", { name: "alpha-0", exact: true }), + ); + + // A search that alpha-0 does not match still shows it, pinned. + await searchFor("beta"); + + const pinned = page.getByRole("menuitemcheckbox", { + name: "alpha-0", + exact: true, + }); + await expect.element(pinned).toBeInTheDocument(); + await expect.element(pinned).toHaveAttribute("aria-checked", "true"); + + // And it can be unchecked from there. + await userEvent.click(pinned); + await expect + .element(page.getByRole("button", { name: "Labels" })) + .toBeInTheDocument(); +}); + +test("selection survives loading another page", async () => { + const onSelectionChange = vitest.fn(); + + await render( + , + ); + + await openMenu(); + + await userEvent.click( + page.getByRole("menuitemcheckbox", { name: "alpha-0", exact: true }), + ); + + await userEvent.click(page.getByRole("button", { name: "Load more" })); + + // An option from the second page. + const second = page.getByRole("menuitemcheckbox", { + name: "gamma-10", + exact: true, + }); + await expect.element(second).toBeInTheDocument(); + + // The first page's selection is still checked after the page grew. + await expect + .element( + page.getByRole("menuitemcheckbox", { name: "alpha-0", exact: true }), + ) + .toHaveAttribute("aria-checked", "true"); + + await userEvent.click(second); + + expect(lastSelection(onSelectionChange)).toEqual(["option-0", "option-10"]); +}); + +test("the trigger reports the selection count across filter changes", async () => { + await render( + , + ); + + await openMenu(); + await userEvent.click( + page.getByRole("menuitemcheckbox", { name: "alpha-0", exact: true }), + ); + + await searchFor("beta-1"); + await userEvent.click( + page.getByRole("menuitemcheckbox", { name: "beta-1", exact: true }), + ); + + await expect + .element(page.getByRole("button", { name: "Labels (2)" })) + .toBeInTheDocument(); +}); diff --git a/packages/components/src/prototypes/contextMenuAsync/AsyncOptionMenu.tsx b/packages/components/src/prototypes/contextMenuAsync/AsyncOptionMenu.tsx new file mode 100644 index 0000000000..c3764a9521 --- /dev/null +++ b/packages/components/src/prototypes/contextMenuAsync/AsyncOptionMenu.tsx @@ -0,0 +1,206 @@ +import { useRef, useState, type FC } from "react"; +import * as Aria from "react-aria-components"; +import { Button } from "@/components/Button"; +import { ContextMenuContent } from "@/components/ContextMenu"; +import { LoadingSpinner } from "@/components/LoadingSpinner"; +import { MenuItem } from "@/components/MenuItem"; +import { Popover } from "@/components/Popover/Popover"; +import { SearchField } from "@/components/SearchField"; +import { Separator } from "@/components/Separator"; +import { Text } from "@/components/Text"; +import { useOverlayController } from "@/lib/controller"; +import { mergeSelection, resolveSelection } from "./lib/selection"; +import { + useAsyncOptions, + type AsyncOption, + type AsyncOptionLoader, +} from "./lib/useAsyncOptions"; + +/** + * How an option that is selected but absent from the current page / filter is + * treated. **This is the open UX question** (issue #1851): both variants keep + * the selection itself intact — they differ only in what the user sees. + * + * - `pin` — selected options are always rendered, in their own group above the + * results. What GitHub's label picker does. The user can always undo a + * selection, but the group competes with the search results for space. + * - `inline` — selected options appear only when the current page or filter + * contains them. Quieter, but a selection can be invisible while the user + * searches, which reads as "it got lost" even though it did not. + */ +export type SelectedOptionBehavior = "pin" | "inline"; + +export interface AsyncOptionMenuProps { + /** Label of the trigger button. */ + label: string; + /** Loads one page of options for the current search term. */ + load: AsyncOptionLoader; + /** Debounce before a changed search term triggers a load. @default 250 */ + debounceMs?: number; + /** @default "pin" */ + selectedOptionBehavior?: SelectedOptionBehavior; + /** Selected keys. Uncontrolled if omitted. */ + selectedKeys?: ReadonlySet; + onSelectionChange?: (keys: ReadonlySet) => void; +} + +const EMPTY: ReadonlySet = new Set(); + +/** + * PROTOTYPE — searchable, async-loaded, multi-select option menu (issue #1851). + * + * Not exported from `public.ts` and not `@flr-generate`d: the interaction and + * visual design belong to UX, and the remote-capable shape belongs to a + * follow-up. This exists to prove the mechanics and to give UX something to + * react to. + * + * The three challenges from the issue map onto three specific places: + * + * 1. **Selection surviving a filter** — `mergeSelection` below. Measured result: + * react-aria 1.20 already preserves a selected key whose option is not in + * the current collection, so this is not a workaround for a bug. It pins the + * behaviour we depend on (the browser test fails if it ever regresses) and + * resolves the `"all"` wildcard, which _is_ collection-bound. + * 2. **Selection surviving async loading** — `selectedKeys` lives here, above + * `useAsyncOptions`, and the loader never sees it. `optionCache` keeps the + * _label_ of a selected option so it stays renderable after the page that + * introduced it is gone. + * 3. **Performance** — the collection only ever holds one page plus the pinned + * selection, so filtering is the server's job, not a client-side pass over + * everything. See the PR body for where the remaining ceiling sits. + */ +export const AsyncOptionMenu: FC = (props) => { + const { + label, + load, + debounceMs, + selectedOptionBehavior = "pin", + selectedKeys: selectedKeysFromProps, + onSelectionChange, + } = props; + + const controller = useOverlayController("Popover", { + reuseControllerFromContext: false, + }); + const triggerRef = useRef(null); + + const [search, setSearch] = useState(""); + const [uncontrolledSelection, setUncontrolledSelection] = + useState>(EMPTY); + + const selectedKeys = selectedKeysFromProps ?? uncontrolledSelection; + + const { options, loadingState, hasMore, loadMore } = useAsyncOptions( + load, + search, + { debounceMs }, + ); + + /* + * Labels of options the user has selected at some point. A selected option + * whose page is gone (search changed, or it was never on the first page) has + * no data left to render from — this is the only reason the cache exists, and + * it is why the selection can be *shown*, not just held. + */ + const optionCache = useRef(new Map()); + for (const option of options) { + optionCache.current.set(option.id, option); + } + + /* + * Not memoized: it reads `optionCache`, a ref that the loop above mutates + * during render, so a dependency array cannot describe when it changes. The + * work is one lookup per selected key. + */ + const pinnedOptions = + selectedOptionBehavior === "pin" + ? [...selectedKeys].flatMap((key) => { + const option = optionCache.current.get(key); + return option ? [option] : []; + }) + : []; + + const pinnedKeys = new Set(pinnedOptions.map((option) => option.id)); + const listedOptions = options.filter((option) => !pinnedKeys.has(option.id)); + + /* + * Exactly the keys the collection renders right now — the reference + * `mergeSelection` needs to tell "the user unchecked this" apart from "this + * is not on screen". + */ + const visibleKeys = new Set([ + ...pinnedOptions.map((option) => option.id), + ...listedOptions.map((option) => option.id), + ]); + + const handleSelectionChange = (selection: Aria.Selection) => { + const merged = mergeSelection({ + previous: selectedKeys, + next: resolveSelection(selection, visibleKeys), + visibleKeys, + }); + + setUncontrolledSelection(merged); + onSelectionChange?.(merged); + }; + + const renderOption = (option: AsyncOption) => ( + + {option.label} + + ); + + const isInitiallyLoading = loadingState === "loading"; + + return ( + <> + + + + + } + onSelectionChange={handleSelectionChange} + renderEmptyState={() => ( + + {isInitiallyLoading ? : "No options found"} + + )} + > + {pinnedOptions.map(renderOption)} + {pinnedOptions.length > 0 && listedOptions.length > 0 ? ( + + ) : null} + {listedOptions.map(renderOption)} + + + {loadingState === "loadingMore" ? ( + + + + ) : hasMore && !isInitiallyLoading ? ( + + ) : null} + + + ); +}; + +export default AsyncOptionMenu; diff --git a/packages/components/src/prototypes/contextMenuAsync/lib/fakeBackend.ts b/packages/components/src/prototypes/contextMenuAsync/lib/fakeBackend.ts new file mode 100644 index 0000000000..01a5be9d78 --- /dev/null +++ b/packages/components/src/prototypes/contextMenuAsync/lib/fakeBackend.ts @@ -0,0 +1,65 @@ +import { sleep } from "@/lib/promises/sleep"; +import type { AsyncOptionLoader, AsyncOptionPage } from "./useAsyncOptions"; + +const words = [ + "alpha", + "beta", + "gamma", + "delta", + "epsilon", + "zeta", + "eta", + "theta", +]; + +/** A stable, deterministic option universe. */ +export const createOptionUniverse = (count: number) => + Array.from({ length: count }, (_, index) => ({ + id: `option-${index}`, + label: `${words[index % words.length]}-${index}`, + })); + +export interface FakeBackendOptions { + /** How many options exist server-side. @default 5000 */ + total?: number; + /** Page size. @default 25 */ + pageSize?: number; + /** Simulated latency in ms. @default 400 */ + latencyMs?: number; +} + +/** + * A loader that filters and pages **server-side**, which is the shape the real + * thing has: the client never holds the full universe, so it also never filters + * it. Used by the stories and the browser test. + */ +export const createFakeBackend = ( + options: FakeBackendOptions = {}, +): AsyncOptionLoader => { + const { total = 5000, pageSize = 25, latencyMs = 400 } = options; + const universe = createOptionUniverse(total); + + return async ({ search, cursor, signal }): Promise => { + if (latencyMs > 0) { + await sleep(latencyMs); + } + if (signal.aborted) { + throw new Error("aborted"); + } + + const matches = search + ? universe.filter((option) => + option.label.toLowerCase().includes(search.toLowerCase()), + ) + : universe; + + const offset = cursor ? Number(cursor) : 0; + const page = matches.slice(offset, offset + pageSize); + const nextOffset = offset + page.length; + + return { + options: page, + cursor: nextOffset < matches.length ? String(nextOffset) : undefined, + }; + }; +}; diff --git a/packages/components/src/prototypes/contextMenuAsync/lib/selection.test.ts b/packages/components/src/prototypes/contextMenuAsync/lib/selection.test.ts new file mode 100644 index 0000000000..c998936a95 --- /dev/null +++ b/packages/components/src/prototypes/contextMenuAsync/lib/selection.test.ts @@ -0,0 +1,68 @@ +import type { Key } from "react-aria-components"; +import { mergeSelection, resolveSelection } from "./selection"; + +const set = (...keys: Key[]) => new Set(keys); + +describe("mergeSelection", () => { + test("keeps a selected key that the collection no longer renders", () => { + // "b" was selected, then filtered out. The collection reports only "a". + const result = mergeSelection({ + previous: set("a", "b"), + next: set("a"), + visibleKeys: set("a"), + }); + + expect(result).toEqual(set("a", "b")); + }); + + test("deselects a key the user actually unchecked", () => { + const result = mergeSelection({ + previous: set("a", "b"), + next: set("b"), + visibleKeys: set("a", "b"), + }); + + expect(result).toEqual(set("b")); + }); + + test("adds a key the user checked in a filtered collection", () => { + const result = mergeSelection({ + previous: set("b"), + next: set("a"), + visibleKeys: set("a"), + }); + + expect(result).toEqual(set("b", "a")); + }); + + test("survives a collection that renders nothing at all", () => { + // An in-flight async load empties the collection for a moment. + const result = mergeSelection({ + previous: set("a", "b"), + next: set(), + visibleKeys: set(), + }); + + expect(result).toEqual(set("a", "b")); + }); + + test("clearing a fully visible collection clears the selection", () => { + const result = mergeSelection({ + previous: set("a", "b"), + next: set(), + visibleKeys: set("a", "b"), + }); + + expect(result).toEqual(set()); + }); +}); + +describe("resolveSelection", () => { + test("resolves the 'all' wildcard against the visible keys", () => { + expect(resolveSelection("all", set("a", "b"))).toEqual(set("a", "b")); + }); + + test("passes an explicit selection through", () => { + expect(resolveSelection(set("a"), set("a", "b"))).toEqual(set("a")); + }); +}); diff --git a/packages/components/src/prototypes/contextMenuAsync/lib/selection.ts b/packages/components/src/prototypes/contextMenuAsync/lib/selection.ts new file mode 100644 index 0000000000..d47cb639aa --- /dev/null +++ b/packages/components/src/prototypes/contextMenuAsync/lib/selection.ts @@ -0,0 +1,45 @@ +import type { Key } from "react-aria-components"; + +/** + * Merges a selection change coming from a collection that only holds _part_ of + * the option universe. + * + * **Measured, not assumed:** react-aria 1.20's `SelectionManager` already + * carries over a selected key whose option is not in the rendered collection — + * `AsyncOptionMenu.browser.test.tsx` passes with this function bypassed. So + * this is not a workaround for a react-aria bug. + * + * It is kept for two reasons: + * + * - It makes "selection is independent of what is loaded" (issue #1851, + * challenges 1 and 2) an explicit, unit-tested invariant instead of a + * behaviour we inherit silently and could lose in a minor upgrade. + * - Only the visible keys are allowed to change, which is what lets + * `resolveSelection` expand react-aria's `"all"` wildcard safely. + */ +export const mergeSelection = (args: { + /** The selection before the change. */ + previous: ReadonlySet; + /** What the collection reported. */ + next: ReadonlySet; + /** The keys the collection actually rendered at the time of the change. */ + visibleKeys: ReadonlySet; +}): Set => { + const { previous, next, visibleKeys } = args; + + const carriedOver = [...previous].filter((key) => !visibleKeys.has(key)); + const stillSelected = [...next].filter((key) => visibleKeys.has(key)); + + return new Set([...carriedOver, ...stillSelected]); +}; + +/** + * `"all"` is react-aria's wildcard selection. It means "every key in the + * current collection", which for a partially loaded list is not a selection we + * can carry across a filter change — so the prototype resolves it eagerly + * against what is visible. + */ +export const resolveSelection = ( + selection: "all" | ReadonlySet, + visibleKeys: ReadonlySet, +): ReadonlySet => (selection === "all" ? visibleKeys : selection); diff --git a/packages/components/src/prototypes/contextMenuAsync/lib/useAsyncOptions.ts b/packages/components/src/prototypes/contextMenuAsync/lib/useAsyncOptions.ts new file mode 100644 index 0000000000..3e528457d8 --- /dev/null +++ b/packages/components/src/prototypes/contextMenuAsync/lib/useAsyncOptions.ts @@ -0,0 +1,145 @@ +import { useCallback, useEffect, useRef, useState } from "react"; + +export interface AsyncOption { + id: string; + label: string; +} + +export interface AsyncOptionPage { + options: readonly AsyncOption[]; + /** Cursor for the next page; `undefined` means the list is exhausted. */ + cursor?: string; +} + +export interface AsyncOptionLoaderArgs { + search: string; + cursor?: string; + signal: AbortSignal; +} + +export type AsyncOptionLoader = ( + args: AsyncOptionLoaderArgs, +) => Promise; + +export type AsyncOptionsLoadingState = + "idle" | "loading" | "loadingMore" | "error"; + +export interface UseAsyncOptionsResult { + options: readonly AsyncOption[]; + loadingState: AsyncOptionsLoadingState; + error?: unknown; + hasMore: boolean; + loadMore: () => void; + reload: () => void; +} + +/** + * Loads options for a search term, one cursor page at a time. + * + * Deliberately hand-rolled rather than `useAsyncList`: the prototype needs the + * search term to be _the_ reset trigger and the loading state to be + * distinguishable between "first page" and "next page", which is what drives + * the two different UI treatments. `useAsyncList` can express this, but not + * without fighting its `filterText`/`sortDescriptor` coupling. + * + * Two properties matter for issue #1851 and both live here rather than in the + * component: + * + * - **A superseded request never lands.** Every load runs under an + * `AbortController`, and a stale response is dropped even if it resolves + * after the one that replaced it. Without this, typing "pro" then "prod" can + * leave the "pro" page on screen. + * - **Selection is not stored here.** This hook only knows about loaded data. + * Selected keys live above it, which is exactly why they survive a reload. + */ +export const useAsyncOptions = ( + load: AsyncOptionLoader, + search: string, + options?: { debounceMs?: number }, +): UseAsyncOptionsResult => { + const debounceMs = options?.debounceMs ?? 250; + + const [pages, setPages] = useState([]); + const [cursor, setCursor] = useState(undefined); + const [isExhausted, setIsExhausted] = useState(false); + const [loadingState, setLoadingState] = + useState("loading"); + const [error, setError] = useState(undefined); + const [reloadToken, setReloadToken] = useState(0); + + const abortRef = useRef(undefined); + const loadRef = useRef(load); + loadRef.current = load; + + const run = useCallback( + async (nextCursor: string | undefined, currentSearch: string) => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; + + setLoadingState(nextCursor ? "loadingMore" : "loading"); + setError(undefined); + + try { + const page = await loadRef.current({ + search: currentSearch, + cursor: nextCursor, + signal: controller.signal, + }); + + // A newer request took over while this one was in flight. + if (controller.signal.aborted) { + return; + } + + setPages((previous) => + nextCursor ? [...previous, ...page.options] : [...page.options], + ); + setCursor(page.cursor); + setIsExhausted(page.cursor === undefined); + setLoadingState("idle"); + } catch (caught) { + if (controller.signal.aborted) { + return; + } + setError(caught); + setLoadingState("error"); + } + }, + [], + ); + + // Reset and reload whenever the search term (or an explicit reload) changes. + useEffect(() => { + setPages([]); + setCursor(undefined); + setIsExhausted(false); + setLoadingState("loading"); + + const timeout = setTimeout(() => void run(undefined, search), debounceMs); + + return () => { + clearTimeout(timeout); + }; + }, [search, debounceMs, reloadToken, run]); + + useEffect(() => () => abortRef.current?.abort(), []); + + const loadMore = useCallback(() => { + if (cursor === undefined || loadingState !== "idle") { + return; + } + void run(cursor, search); + }, [cursor, loadingState, run, search]); + + const reload = useCallback(() => setReloadToken((token) => token + 1), []); + + return { + options: pages, + loadingState, + error, + hasMore: !isExhausted, + loadMore, + reload, + }; +}; diff --git a/packages/components/src/prototypes/contextMenuAsync/stories/AsyncOptionMenu.stories.tsx b/packages/components/src/prototypes/contextMenuAsync/stories/AsyncOptionMenu.stories.tsx new file mode 100644 index 0000000000..7bf017f107 --- /dev/null +++ b/packages/components/src/prototypes/contextMenuAsync/stories/AsyncOptionMenu.stories.tsx @@ -0,0 +1,60 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import { AsyncOptionMenu } from "../AsyncOptionMenu"; +import { createFakeBackend } from "../lib/fakeBackend"; + +/** + * PROTOTYPE for issue #1851 — not a public component. The visual and + * interaction design is UX's call; this only demonstrates the mechanics. + */ +const meta: Meta = { + title: "Prototypes/AsyncOptionMenu", + component: AsyncOptionMenu, + args: { + label: "Labels", + load: createFakeBackend(), + }, + parameters: { + controls: { disable: true }, + }, +}; +export default meta; + +type Story = StoryObj; + +/** 5000 server-side options, 25 per page, 400 ms latency. */ +export const Default: Story = {}; + +/** Selected options stay pinned above the results, GitHub-style. */ +export const PinSelected: Story = { + args: { selectedOptionBehavior: "pin" }, +}; + +/** + * Selected options are only shown when the current page or filter contains + * them. The selection still survives — it is just not visible. + */ +export const InlineSelected: Story = { + args: { selectedOptionBehavior: "inline" }, +}; + +/** Slow backend — shows the loading states and that typing cancels in flight. */ +export const SlowBackend: Story = { + args: { load: createFakeBackend({ latencyMs: 1500 }) }, +}; + +/** A small universe that fits in one page — no "load more". */ +export const SinglePage: Story = { + args: { load: createFakeBackend({ total: 8, latencyMs: 200 }) }, +}; + +/** + * Where the ceiling actually is: paging switched off, so the whole universe + * lands in the collection at once. This is the shape today's `ContextMenu` + * already has when a caller renders every `MenuItem` — the prototype exists to + * avoid it, and this story is here to measure it. + */ +export const UnpagedStress: Story = { + args: { + load: createFakeBackend({ total: 2000, pageSize: 2000, latencyMs: 0 }), + }, +};