mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 04:25:52 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The new-task composer lets users select an assignee and override its model. > - On phones, these selectors open as large sheets outside the task dialog DOM. > - The parent dialog's scroll lock cancels touch drags in those sheets. > - This pull request gives each mobile sheet its own modal scroll boundary. > - Users can scroll to an option, select it, and continue editing their task. ## Linked Issues or Issue Description **What happened?** An iPhone Safari user could not drag through the assignee or model list in the new-task composer. The list stayed at the top. A touch-enabled Chromium reproduction also showed canceled touchmove events and an unchanged scroll position. **Expected behavior** A finger drag scrolls the list. A tap selects an option. Closing the picker preserves the task draft and selected values. **Steps to reproduce** 1. At phone width, open New Task in a company with enough agents to overflow the picker. 2. Open Assignee and drag upward through the list. 3. Select a Codex agent, open Codex options, choose Custom, and open the model selector. 4. Repeat the drag with enough models to overflow the available viewport. **Paperclip version or commit** Reproduced on `e5bf9d49a`. The fix is rebased onto `b3eb03fcb`. **Deployment mode** Local development in an isolated test instance. The user reported iPhone Safari. Browser automation uses native Chromium touch input at phone dimensions; a physical iPhone was not available. Related: #14250 introduced the large mobile entity picker sheets. Duplicate search found no existing fix for their touch scroll boundary. ## What Changed - Use modal Radix popovers for mobile entity sheets. Their lists can scroll while the background stays locked. - Suppress opening when Radix restores trigger focus after dismissal. Escape and outside taps now close the picker without reopening it. - Add a failing-before/fixed-after regression for a mobile sheet inside a parent dialog. - Add a browser test for native touch scrolling in both lists, tap selection, draft retention, close-button/Escape/outside dismissal, and desktop mouse/keyboard selection. - Document the browser test command. ## Verification - Passed `pnpm -r typecheck`, `pnpm build`, and `pnpm check:token-gates`. - Passed 41 focused tests for `InlineEntitySelector` and `NewIssueDialog`. - Passed the new browser test against a disposable instance running this worktree. It uses 22 real fixture agents and a fixed 24-model catalog. It covers 390×844 and 390×430 phone viewports and a 1280×900 desktop viewport. - Inspected the rendered new-task form and both selectors. Selections returned to the draft with its title intact. - `pnpm test:run` was attempted and stopped after confirming that local database suites were skipping because macOS has exhausted its system semaphore limit (`initdb`: `could not create semaphores: No space left on device`). It did not complete locally. The complete test matrix passed in Linux CI, including all 71 tool-gateway tests. - All 150 browser tests passed in CI, including the new native-touch regression. - Greptile reviewed the latest commit at 5/5. Its desktop focus finding is fixed and the review thread is resolved. All checks on `2ab22727ca768b4134c9c840bbc8e4c80d7da674` are complete: 53 passed, 2 intentionally skipped Storybook deployment checks, and the Snyk status passed. ## Risks Low risk. Mobile selectors now own focus and scroll isolation. This changes their modal behavior, so the tests cover nested dismissal and focus return. Desktop selectors remain non-modal. No schema, API, or dependency changes. ## Model Used OpenAI Codex (GPT-6), with reasoning, repository inspection, code execution, and browser testing. The exact backend deployment ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
384 lines
13 KiB
TypeScript
384 lines
13 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { createRoot } from "react-dom/client";
|
|
import { flushSync } from "react-dom";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { Dialog, DialogContent, DialogTitle } from "./ui/dialog";
|
|
import { InlineEntitySelector } from "./InlineEntitySelector";
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
async function act(callback: () => void | Promise<void>) {
|
|
let result: void | Promise<void> = undefined;
|
|
flushSync(() => {
|
|
result = callback();
|
|
});
|
|
await result;
|
|
}
|
|
|
|
describe("InlineEntitySelector", () => {
|
|
let container: HTMLDivElement;
|
|
let originalMatchMedia: typeof window.matchMedia;
|
|
|
|
beforeEach(() => {
|
|
originalMatchMedia = window.matchMedia;
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
});
|
|
|
|
afterEach(() => {
|
|
window.matchMedia = originalMatchMedia;
|
|
container.remove();
|
|
document.body.innerHTML = "";
|
|
});
|
|
|
|
it("allows touch scrolling in a mobile picker portalled outside a parent dialog", async () => {
|
|
window.matchMedia = vi.fn().mockImplementation((query: string) => ({
|
|
matches: query === "(max-width: 40rem)",
|
|
media: query,
|
|
addEventListener: vi.fn(),
|
|
removeEventListener: vi.fn(),
|
|
}));
|
|
const root = createRoot(container);
|
|
await act(() => {
|
|
root.render(
|
|
<Dialog defaultOpen>
|
|
<DialogContent aria-describedby={undefined}>
|
|
<DialogTitle>New task</DialogTitle>
|
|
<InlineEntitySelector
|
|
value=""
|
|
options={Array.from({ length: 20 }, (_, index) => ({ id: `agent-${index}`, label: `Agent ${index}` }))}
|
|
placeholder="Assignee"
|
|
noneLabel="No assignee"
|
|
searchPlaceholder="Search assignees..."
|
|
emptyMessage="No assignees found."
|
|
onChange={vi.fn()}
|
|
triggerTestId="nested-assignee-picker"
|
|
openOnFocus={false}
|
|
disablePortal
|
|
/>
|
|
</DialogContent>
|
|
</Dialog>,
|
|
);
|
|
});
|
|
try {
|
|
await act(() => {
|
|
document.querySelector<HTMLButtonElement>("[data-testid=nested-assignee-picker]")!.click();
|
|
});
|
|
// Let both real Radix scroll locks install their document listeners.
|
|
await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); });
|
|
const list = document.querySelector<HTMLElement>("[data-mobile-entity-picker-list]")!;
|
|
expect(list).not.toBeNull();
|
|
expect(document.querySelector("[data-slot=dialog-content]")!.contains(list)).toBe(false);
|
|
// jsdom has no layout; supply the geometry of a list with more rows than fit.
|
|
list.style.overflowY = "auto";
|
|
Object.defineProperties(list, {
|
|
scrollHeight: { configurable: true, value: 1000 },
|
|
clientHeight: { configurable: true, value: 200 },
|
|
});
|
|
const option = list.querySelector("button")!;
|
|
const touch = (type: string, y: number) => {
|
|
const event = new Event(type, { bubbles: true, cancelable: true });
|
|
Object.defineProperties(event, {
|
|
touches: { value: [{ clientX: 100, clientY: y }] },
|
|
changedTouches: { value: [{ clientX: 100, clientY: y }] },
|
|
});
|
|
option.dispatchEvent(event);
|
|
return event;
|
|
};
|
|
touch("touchstart", 150);
|
|
expect(touch("touchmove", 100).defaultPrevented).toBe(false);
|
|
// The background stays locked while the picker is open.
|
|
const outsideMove = new Event("touchmove", { bubbles: true, cancelable: true });
|
|
Object.defineProperties(outsideMove, {
|
|
touches: { value: [{ clientX: 100, clientY: 100 }] },
|
|
changedTouches: { value: [{ clientX: 100, clientY: 100 }] },
|
|
});
|
|
document.body.dispatchEvent(outsideMove);
|
|
expect(outsideMove.defaultPrevented).toBe(true);
|
|
} finally {
|
|
await act(() => root.unmount());
|
|
}
|
|
});
|
|
|
|
it("opens on the next keyboard focus after a desktop outside dismissal", async () => {
|
|
const root = createRoot(container);
|
|
await act(() => {
|
|
root.render(<>
|
|
<button type="button" data-testid="outside-picker">Outside</button>
|
|
<InlineEntitySelector
|
|
value=""
|
|
options={[{ id: "agent-1", label: "Agent One" }]}
|
|
placeholder="Assignee"
|
|
noneLabel="No assignee"
|
|
searchPlaceholder="Search assignees..."
|
|
emptyMessage="No assignees found."
|
|
onChange={vi.fn()}
|
|
triggerTestId="desktop-assignee-picker"
|
|
/>
|
|
</>);
|
|
});
|
|
try {
|
|
const trigger = container.querySelector<HTMLButtonElement>("[data-testid=desktop-assignee-picker]")!;
|
|
const outside = container.querySelector<HTMLButtonElement>("[data-testid=outside-picker]")!;
|
|
await act(() => trigger.click());
|
|
await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); });
|
|
await act(() => {
|
|
outside.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, cancelable: true }));
|
|
outside.focus();
|
|
outside.click();
|
|
});
|
|
await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); });
|
|
expect(document.querySelector("[data-mobile-entity-picker]")).toBeNull();
|
|
expect(document.activeElement).toBe(outside);
|
|
await act(() => trigger.focus());
|
|
expect(document.querySelector("[data-mobile-entity-picker]")).not.toBeNull();
|
|
} finally {
|
|
await act(() => root.unmount());
|
|
}
|
|
});
|
|
|
|
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(
|
|
<InlineEntitySelector
|
|
value=""
|
|
options={[
|
|
{ id: "agent:agent-1", label: "CodexCoder" },
|
|
{ id: "agent:agent-2", label: "DesignBot" },
|
|
]}
|
|
placeholder="Responsible"
|
|
noneLabel="No responsible"
|
|
searchPlaceholder="Search responsible..."
|
|
emptyMessage="No responsible found."
|
|
onChange={onChange}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
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 responsible..."]') 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();
|
|
});
|
|
});
|
|
|
|
it("focuses the search input when opened on coarse pointers", async () => {
|
|
window.matchMedia = vi.fn().mockImplementation((query: string) => ({
|
|
matches: query === "(pointer: coarse)" || query === "(max-width: 40rem)",
|
|
media: query,
|
|
onchange: null,
|
|
addEventListener: vi.fn(),
|
|
removeEventListener: vi.fn(),
|
|
addListener: vi.fn(),
|
|
removeListener: vi.fn(),
|
|
dispatchEvent: vi.fn(),
|
|
}));
|
|
|
|
const root = createRoot(container);
|
|
|
|
act(() => {
|
|
root.render(
|
|
<InlineEntitySelector
|
|
value=""
|
|
options={[
|
|
{ id: "agent:agent-1", label: "CodexCoder" },
|
|
{ id: "agent:agent-2", label: "DesignBot" },
|
|
]}
|
|
placeholder="Responsible"
|
|
noneLabel="No responsible"
|
|
searchPlaceholder="Search responsible..."
|
|
emptyMessage="No responsible found."
|
|
onChange={vi.fn()}
|
|
disablePortal
|
|
/>,
|
|
);
|
|
});
|
|
|
|
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 responsible..."]') as HTMLInputElement | null;
|
|
expect(searchInput).not.toBeNull();
|
|
expect(searchInput?.className).toContain("text-base");
|
|
expect(document.querySelector("[data-mobile-entity-picker]")).not.toBeNull();
|
|
expect(container.contains(document.querySelector("[data-mobile-entity-picker]"))).toBe(false);
|
|
expect(document.querySelector("[data-mobile-entity-picker-header]")?.textContent).toContain("Responsible");
|
|
expect(document.querySelector('button[aria-label="Close selector"]')).not.toBeNull();
|
|
expect(document.activeElement).toBe(searchInput);
|
|
|
|
await act(async () => {
|
|
(document.querySelector('button[aria-label="Close selector"]') as HTMLButtonElement | null)?.click();
|
|
await Promise.resolve();
|
|
});
|
|
expect(document.querySelector("[data-mobile-entity-picker]")).toBeNull();
|
|
|
|
act(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("opens on programmatic focus without toggling an open popover closed", async () => {
|
|
const root = createRoot(container);
|
|
|
|
act(() => {
|
|
root.render(
|
|
<InlineEntitySelector
|
|
value=""
|
|
options={[{ id: "project-1", label: "Project One" }]}
|
|
placeholder="Project"
|
|
noneLabel="No project"
|
|
searchPlaceholder="Search projects..."
|
|
emptyMessage="No projects found."
|
|
onChange={vi.fn()}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const trigger = container.querySelector("button") as HTMLButtonElement | null;
|
|
expect(trigger).not.toBeNull();
|
|
|
|
await act(async () => {
|
|
trigger?.focus();
|
|
await Promise.resolve();
|
|
});
|
|
expect(
|
|
document.querySelector('input[placeholder="Search projects..."]'),
|
|
).not.toBeNull();
|
|
|
|
await act(async () => {
|
|
trigger?.focus();
|
|
await Promise.resolve();
|
|
});
|
|
expect(
|
|
document.querySelector('input[placeholder="Search projects..."]'),
|
|
).not.toBeNull();
|
|
|
|
act(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("does not open the popover when disabled", async () => {
|
|
const root = createRoot(container);
|
|
const onChange = vi.fn();
|
|
|
|
act(() => {
|
|
root.render(
|
|
<InlineEntitySelector
|
|
value=""
|
|
options={[{ id: "agent:agent-1", label: "CodexCoder" }]}
|
|
placeholder="Responsible"
|
|
noneLabel="No responsible"
|
|
searchPlaceholder="Search responsible..."
|
|
emptyMessage="No responsible found."
|
|
onChange={onChange}
|
|
disabled
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const trigger = container.querySelector("button") as HTMLButtonElement | null;
|
|
expect(trigger).not.toBeNull();
|
|
expect(trigger?.disabled).toBe(true);
|
|
|
|
await act(async () => {
|
|
trigger?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
|
|
});
|
|
|
|
expect(document.querySelector('input[placeholder="Search responsible..."]')).toBeNull();
|
|
expect(onChange).not.toHaveBeenCalled();
|
|
|
|
act(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("filters options as the user types in the search box", async () => {
|
|
const root = createRoot(container);
|
|
const onChange = vi.fn();
|
|
|
|
act(() => {
|
|
root.render(
|
|
<InlineEntitySelector
|
|
value=""
|
|
options={[
|
|
{ id: "agent:agent-1", label: "CodexCoder" },
|
|
{ id: "agent:agent-2", label: "DesignBot" },
|
|
]}
|
|
placeholder="Responsible"
|
|
noneLabel="No responsible"
|
|
searchPlaceholder="Search responsible..."
|
|
emptyMessage="No responsible found."
|
|
onChange={onChange}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const trigger = container.querySelector("button") as HTMLButtonElement | null;
|
|
await act(async () => {
|
|
trigger?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
|
|
});
|
|
|
|
const searchInput = document.querySelector('input[placeholder="Search responsible..."]') as HTMLInputElement | null;
|
|
expect(searchInput).not.toBeNull();
|
|
|
|
const nativeInputValue = Object.getOwnPropertyDescriptor(
|
|
window.HTMLInputElement.prototype,
|
|
"value",
|
|
)?.set;
|
|
await act(async () => {
|
|
nativeInputValue?.call(searchInput, "design");
|
|
searchInput?.dispatchEvent(new Event("input", { bubbles: true }));
|
|
});
|
|
|
|
const optionLabels = Array.from(document.querySelectorAll("[role='dialog'] button, .max-h-56 button")).map(
|
|
(el) => el.textContent ?? "",
|
|
);
|
|
const joined = optionLabels.join("|");
|
|
expect(joined).toContain("DesignBot");
|
|
expect(joined).not.toContain("CodexCoder");
|
|
|
|
act(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
});
|