From 5dbb32aa42cb5be69723ae1b9c89abb5eeea65f6 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 27 Apr 2026 16:06:35 -0500 Subject: [PATCH] Stabilize selector and company selection regressions --- .../components/InlineEntitySelector.test.tsx | 80 +++++++++++ ui/src/components/InlineEntitySelector.tsx | 28 +++- ui/src/context/CompanyContext.test.tsx | 133 +++++++++++++++++- 3 files changed, 232 insertions(+), 9 deletions(-) create mode 100644 ui/src/components/InlineEntitySelector.test.tsx diff --git a/ui/src/components/InlineEntitySelector.test.tsx b/ui/src/components/InlineEntitySelector.test.tsx new file mode 100644 index 00000000000..f127f41c330 --- /dev/null +++ b/ui/src/components/InlineEntitySelector.test.tsx @@ -0,0 +1,80 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { InlineEntitySelector } from "./InlineEntitySelector"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +describe("InlineEntitySelector", () => { + let container: HTMLDivElement; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + }); + + afterEach(() => { + container.remove(); + document.body.innerHTML = ""; + }); + + it("keeps handled search navigation keys inside the popover", async () => { + const root = createRoot(container); + const onChange = vi.fn(); + const documentKeyDown = vi.fn(); + document.addEventListener("keydown", documentKeyDown); + + act(() => { + root.render( + , + ); + }); + + const trigger = container.querySelector("button") as HTMLButtonElement | null; + expect(trigger).not.toBeNull(); + + await act(async () => { + trigger?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + const searchInput = document.querySelector('input[placeholder="Search assignees..."]') as HTMLInputElement | null; + expect(searchInput).not.toBeNull(); + searchInput?.focus(); + + await act(async () => { + await Promise.resolve(); + }); + + await act(async () => { + searchInput?.dispatchEvent(new KeyboardEvent("keydown", { bubbles: true, key: "ArrowDown" })); + }); + + expect(documentKeyDown).not.toHaveBeenCalled(); + + await act(async () => { + searchInput?.dispatchEvent(new KeyboardEvent("keydown", { bubbles: true, key: "Enter" })); + }); + + expect(documentKeyDown).not.toHaveBeenCalled(); + expect(onChange).toHaveBeenCalledWith("agent:agent-1"); + + document.removeEventListener("keydown", documentKeyDown); + act(() => { + root.unmount(); + }); + }); +}); diff --git a/ui/src/components/InlineEntitySelector.tsx b/ui/src/components/InlineEntitySelector.tsx index 6bbcf64940f..cae1897712e 100644 --- a/ui/src/components/InlineEntitySelector.tsx +++ b/ui/src/components/InlineEntitySelector.tsx @@ -29,6 +29,8 @@ interface InlineEntitySelectorProps { openOnFocus?: boolean; } +const EMPTY_RECENT_OPTION_IDS: string[] = []; + export const InlineEntitySelector = forwardRef( function InlineEntitySelector( { @@ -43,7 +45,7 @@ export const InlineEntitySelector = forwardRef(null); const shouldPreventCloseAutoFocusRef = useRef(false); const isPointerDownRef = useRef(false); @@ -72,10 +75,16 @@ export const InlineEntitySelector = forwardRef option.id === value) ?? null; + function setHighlightedIndexValue(next: number | ((current: number) => number)) { + const resolved = typeof next === "function" ? next(highlightedIndexRef.current) : next; + highlightedIndexRef.current = resolved; + setHighlightedIndex(resolved); + } + useEffect(() => { if (!open) return; const selectedIndex = filteredOptions.findIndex((option) => option.id === value); - setHighlightedIndex(selectedIndex >= 0 ? selectedIndex : 0); + setHighlightedIndexValue(selectedIndex >= 0 ? selectedIndex : 0); }, [filteredOptions, open, value]); const commitSelection = (index: number, moveNext: boolean) => { @@ -153,14 +162,16 @@ export const InlineEntitySelector = forwardRef { if (event.key === "ArrowDown") { event.preventDefault(); - setHighlightedIndex((current) => + event.stopPropagation(); + setHighlightedIndexValue((current) => filteredOptions.length === 0 ? 0 : (current + 1) % filteredOptions.length, ); return; } if (event.key === "ArrowUp") { event.preventDefault(); - setHighlightedIndex((current) => { + event.stopPropagation(); + setHighlightedIndexValue((current) => { if (filteredOptions.length === 0) return 0; return current <= 0 ? filteredOptions.length - 1 : current - 1; }); @@ -168,16 +179,19 @@ export const InlineEntitySelector = forwardRef setHighlightedIndex(index)} + onMouseEnter={() => setHighlightedIndexValue(index)} onClick={() => commitSelection(index, true)} > {renderOption ? renderOption(option, isSelected) : {option.label}} diff --git a/ui/src/context/CompanyContext.test.tsx b/ui/src/context/CompanyContext.test.tsx index 5e68ad3d2c0..0e26a5b82cf 100644 --- a/ui/src/context/CompanyContext.test.tsx +++ b/ui/src/context/CompanyContext.test.tsx @@ -1,10 +1,63 @@ -import { describe, expect, it } from "vitest"; -import { resolveBootstrapCompanySelection, shouldClearStoredCompanySelection } from "./CompanyContext"; +// @vitest-environment jsdom + +import { act, useEffect } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { Company } from "@paperclipai/shared"; +import { + CompanyProvider, + resolveBootstrapCompanySelection, + shouldClearStoredCompanySelection, + useCompany, +} from "./CompanyContext"; + +const mockCompaniesApi = vi.hoisted(() => ({ + list: vi.fn(), + create: vi.fn(), +})); + +vi.mock("../api/companies", () => ({ + companiesApi: mockCompaniesApi, +})); const activeCompany = { id: "company-1" }; const secondActiveCompany = { id: "company-2" }; const archivedCompany = { id: "archived-company" }; +function makeCompany(id: string): Company { + return { + id, + name: "Paperclip", + description: null, + status: "active", + pauseReason: null, + pausedAt: null, + issuePrefix: "PAP", + issueCounter: 1, + budgetMonthlyCents: 0, + spentMonthlyCents: 0, + requireBoardApprovalForNewAgents: false, + feedbackDataSharingEnabled: false, + feedbackDataSharingConsentAt: null, + feedbackDataSharingConsentByUserId: null, + feedbackDataSharingTermsVersion: null, + brandColor: null, + logoAssetId: null, + logoUrl: null, + createdAt: new Date(), + updatedAt: new Date(), + }; +} + +function Probe({ onSelectedCompanyId }: { onSelectedCompanyId: (companyId: string | null) => void }) { + const { selectedCompanyId } = useCompany(); + useEffect(() => { + onSelectedCompanyId(selectedCompanyId); + }, [onSelectedCompanyId, selectedCompanyId]); + return
; +} + describe("resolveBootstrapCompanySelection", () => { it("does not expose a stale stored company id before companies load", () => { expect(resolveBootstrapCompanySelection({ @@ -69,3 +122,79 @@ describe("shouldClearStoredCompanySelection", () => { })).toBe(true); }); }); + +describe("CompanyProvider", () => { + let container: HTMLDivElement; + let root: Root; + let queryClient: QueryClient; + + beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + localStorage.clear(); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + }, + }); + }); + + afterEach(() => { + act(() => { + root.unmount(); + }); + queryClient.clear(); + container.remove(); + vi.clearAllMocks(); + }); + + it("does not expose a stale stored company id before companies load", async () => { + localStorage.setItem("paperclip.selectedCompanyId", "stale-company"); + mockCompaniesApi.list.mockImplementation(() => new Promise(() => {})); + const seen: Array = []; + + await act(async () => { + root.render( + + + seen.push(companyId)} /> + + , + ); + }); + + expect(seen).toEqual([null]); + }); + + it("replaces a stale stored company id with the first loaded company", async () => { + localStorage.setItem("paperclip.selectedCompanyId", "stale-company"); + let resolveCompanies!: (companies: Company[]) => void; + const companiesPromise = new Promise((resolve) => { + resolveCompanies = resolve; + }); + mockCompaniesApi.list.mockReturnValue(companiesPromise); + const seen: Array = []; + + await act(async () => { + root.render( + + + seen.push(companyId)} /> + + , + ); + }); + + await act(async () => { + resolveCompanies([makeCompany("company-1")]); + await companiesPromise; + }); + + await vi.waitFor(() => { + expect(seen).toEqual([null, "company-1"]); + expect(localStorage.getItem("paperclip.selectedCompanyId")).toBe("company-1"); + }); + }); +});