mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The issue and routine editor surfaces share document annotation behavior for commentable text. > - The annotation layer currently observes document selections inside its container and converts those selections into pending comment anchors. > - Routine editor fields can live inside that same annotated document container while still needing normal native text selection behavior. > - When a user selects text inside an editable routine field, the annotation layer should leave that selection alone instead of preparing a comment. > - This pull request teaches the annotation layer to ignore selections touching editable controls and covers that case with focused component tests. > - The benefit is that routine editing remains editable text UX, while document annotation selection continues to work for normal read-only document text. ## Linked Issues or Issue Description No public GitHub issue exists for this bug. Inline bug report follows. ### Pre-submission checklist - [x] I have searched existing open and closed issues and this is not a duplicate. - [x] I am on current `master` for this PR branch. - [x] I have confirmed the error originates in Paperclip itself, not in an agent adapter, API provider, or local configuration. ### What happened? Selecting text inside an editable routine field could be interpreted as a document annotation selection. That meant the annotation layer could prepare a pending comment anchor or show annotation affordances while the user was just selecting text to edit routine content. ### Expected behavior Editable controls should keep native text selection behavior. Selecting text inside `input`, `textarea`, `select`, or contenteditable routine editor regions should not build a document annotation anchor or show the document annotation toolbar. ### Steps to reproduce 1. Render document annotation behavior around a document/editor container that also contains an editable routine text region. 2. Select text inside the editable region. 3. Observe that the document annotation layer treats the selection as commentable text instead of ignoring it. ### Paperclip version or commit Reproduced and fixed against `master` at `5356b7d73`, with PR head `4f53f83f1`. ### Deployment mode Local dev / component test environment. ### Installation method Built from source. ### Agent adapter(s) involved Not adapter-specific; core UI behavior. ### Database mode Not database-related. ### Access context Board / human operator UI behavior. ### Relevant logs or output No runtime logs. Regression coverage is in `ui/src/components/DocumentAnnotationLayer.test.tsx`. ### Relevant config Not applicable. ### Additional context Root cause: `DocumentAnnotationLayer` filtered selections by container membership, but did not exclude editable controls or all valid contenteditable hosts before building a pending annotation anchor. ### Privacy checklist - [x] I have reviewed all pasted output for PII and redacted where necessary. ## What Changed - Added an editable-selection guard in `DocumentAnnotationLayer` that ignores selections touching `input`, `textarea`, `select`, or contenteditable elements. - Covered both `contenteditable="true"` and bare/empty `contenteditable` hosts so routine editor selections do not build annotation anchors. - Added focused regression tests proving editable selections do not call the annotation anchor helpers or show the annotation toolbar. ## Verification - `pnpm vitest run ui/src/components/DocumentAnnotationLayer.test.tsx` - `git diff --check` - GitHub PR checks are green for head `4f53f83f1`, including policy, review, typecheck/release registry, build, general tests, serialized suites, e2e, canary dry run, security checks, and Greptile. ## Risks Low risk. The change only narrows annotation capture when a selection touches an editable element inside the annotation container. Normal read-only document annotation selections still use the existing anchor-building path. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex coding agent based on GPT-5, with shell, Git, Vitest, and GitHub connector/CLI tool use. The exact hosted runtime model identifier is 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>
301 lines
11 KiB
TypeScript
301 lines
11 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { createRoot, type Root } from "react-dom/client";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { DocumentAnnotationLayer } from "./DocumentAnnotationLayer";
|
|
|
|
const mockBuildAnchorFromContainerSelection = vi.hoisted(() => vi.fn());
|
|
const mockGetContainerTextOffset = vi.hoisted(() => vi.fn());
|
|
const mockRangesForNormalizedSpan = vi.hoisted(() => vi.fn());
|
|
|
|
vi.mock("@/lib/document-annotation-selection", () => ({
|
|
buildAnchorFromContainerSelection: mockBuildAnchorFromContainerSelection,
|
|
getContainerTextOffset: mockGetContainerTextOffset,
|
|
rangesForNormalizedSpan: mockRangesForNormalizedSpan,
|
|
}));
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
async function act(callback: () => void | Promise<void>) {
|
|
await callback();
|
|
await Promise.resolve();
|
|
await new Promise((resolve) => setTimeout(resolve, 0));
|
|
}
|
|
|
|
function makeRect(left: number, top: number, width: number, height: number): DOMRect {
|
|
return {
|
|
x: left,
|
|
y: top,
|
|
left,
|
|
top,
|
|
right: left + width,
|
|
bottom: top + height,
|
|
width,
|
|
height,
|
|
toJSON: () => ({}),
|
|
} as DOMRect;
|
|
}
|
|
|
|
function makeRange(rects: DOMRect[], commonAncestorContainer: Node = document.createTextNode("")): Range {
|
|
return {
|
|
commonAncestorContainer,
|
|
getClientRects: () => rects,
|
|
} as unknown as Range;
|
|
}
|
|
|
|
describe("DocumentAnnotationLayer", () => {
|
|
let container: HTMLDivElement;
|
|
let rectSpy: ReturnType<typeof vi.spyOn>;
|
|
let root: Root | null = null;
|
|
|
|
beforeEach(() => {
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
mockRangesForNormalizedSpan.mockReturnValue([makeRange([makeRect(8, 12, 80, 18)])]);
|
|
rectSpy = vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(makeRect(0, 0, 400, 300));
|
|
});
|
|
|
|
afterEach(async () => {
|
|
if (root) {
|
|
await act(() => root?.unmount());
|
|
root = null;
|
|
}
|
|
rectSpy.mockRestore();
|
|
container.remove();
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
it("uses solid yellow backgrounds for annotation highlights in light and dark themes", async () => {
|
|
const body = document.createElement("div");
|
|
body.textContent = "Annotated body text.";
|
|
root = createRoot(container);
|
|
|
|
await act(async () => {
|
|
root?.render(
|
|
<DocumentAnnotationLayer
|
|
containerRef={{ current: body }}
|
|
markdown="Annotated body text."
|
|
threads={[
|
|
{ id: "active", selectedText: "Annotated", status: "open", anchorState: "active" },
|
|
{ id: "focused", selectedText: "body", status: "open", anchorState: "active" },
|
|
{ id: "stale", selectedText: "text", status: "open", anchorState: "stale" },
|
|
{ id: "resolved", selectedText: "body text", status: "resolved", anchorState: "active" },
|
|
]}
|
|
focusedThreadId="focused"
|
|
onThreadFocus={vi.fn()}
|
|
pendingAnchor={null}
|
|
onPendingAnchorChange={vi.fn()}
|
|
onRequestComment={vi.fn()}
|
|
hideResolved={false}
|
|
/>,
|
|
);
|
|
await new Promise((resolve) => window.requestAnimationFrame(resolve));
|
|
});
|
|
|
|
const highlights = Array.from(container.querySelectorAll(".paperclip-doc-annotation-highlight"));
|
|
expect(highlights).toHaveLength(4);
|
|
|
|
for (const highlight of highlights) {
|
|
const backgroundClasses = Array.from(highlight.classList).filter((className) =>
|
|
/^(dark:|hover:|dark:hover:)?bg-yellow-\d+$/.test(className)
|
|
|| /^(dark:|hover:|dark:hover:)?bg-yellow-\d+\//.test(className),
|
|
);
|
|
expect(backgroundClasses.some((className) => className.includes("/"))).toBe(false);
|
|
expect(backgroundClasses.some((className) => className.startsWith("bg-yellow-"))).toBe(true);
|
|
expect(backgroundClasses.some((className) => className.startsWith("dark:bg-yellow-"))).toBe(true);
|
|
}
|
|
});
|
|
|
|
it("does not render highlights for text clipped by folded document content", async () => {
|
|
const body = document.createElement("div");
|
|
const clippedContent = document.createElement("div");
|
|
clippedContent.className = "fold-curtain__content";
|
|
const hiddenText = document.createTextNode("Hidden folded text");
|
|
clippedContent.appendChild(hiddenText);
|
|
body.appendChild(clippedContent);
|
|
mockRangesForNormalizedSpan.mockReturnValue([makeRange([makeRect(8, 60, 80, 18)], hiddenText)]);
|
|
rectSpy.mockImplementation(function (this: HTMLElement) {
|
|
if (this === clippedContent) return makeRect(0, 0, 400, 40);
|
|
return makeRect(0, 0, 400, 120);
|
|
});
|
|
root = createRoot(container);
|
|
|
|
await act(async () => {
|
|
root?.render(
|
|
<DocumentAnnotationLayer
|
|
containerRef={{ current: body }}
|
|
markdown="Hidden folded text"
|
|
threads={[
|
|
{ id: "hidden", selectedText: "Hidden folded text", status: "open", anchorState: "active" },
|
|
]}
|
|
focusedThreadId={null}
|
|
onThreadFocus={vi.fn()}
|
|
pendingAnchor={null}
|
|
onPendingAnchorChange={vi.fn()}
|
|
onRequestComment={vi.fn()}
|
|
/>,
|
|
);
|
|
await new Promise((resolve) => window.requestAnimationFrame(resolve));
|
|
});
|
|
|
|
expect(container.querySelector(".paperclip-doc-annotation-highlight")).toBeNull();
|
|
expect(container.querySelector(".paperclip-doc-annotation-hit-target")).toBeNull();
|
|
});
|
|
|
|
it("does not capture annotation comments from editable selections", async () => {
|
|
const body = document.createElement("div");
|
|
const editable = document.createElement("div");
|
|
editable.setAttribute("contenteditable", "true");
|
|
const text = document.createTextNode("Editing routine instructions");
|
|
editable.appendChild(text);
|
|
body.appendChild(editable);
|
|
|
|
const range = document.createRange();
|
|
range.setStart(text, 0);
|
|
range.setEnd(text, "Editing".length);
|
|
const getSelectionSpy = vi.spyOn(window, "getSelection").mockReturnValue({
|
|
rangeCount: 1,
|
|
isCollapsed: false,
|
|
getRangeAt: () => range,
|
|
} as unknown as Selection);
|
|
const onPendingAnchorChange = vi.fn();
|
|
root = createRoot(container);
|
|
|
|
try {
|
|
await act(async () => {
|
|
root?.render(
|
|
<DocumentAnnotationLayer
|
|
containerRef={{ current: body }}
|
|
markdown="Editing routine instructions"
|
|
threads={[]}
|
|
focusedThreadId={null}
|
|
onThreadFocus={vi.fn()}
|
|
pendingAnchor={null}
|
|
onPendingAnchorChange={onPendingAnchorChange}
|
|
onRequestComment={vi.fn()}
|
|
/>,
|
|
);
|
|
await new Promise((resolve) => window.requestAnimationFrame(resolve));
|
|
});
|
|
|
|
await act(async () => {
|
|
document.dispatchEvent(new Event("selectionchange"));
|
|
});
|
|
|
|
expect(mockGetContainerTextOffset).not.toHaveBeenCalled();
|
|
expect(mockBuildAnchorFromContainerSelection).not.toHaveBeenCalled();
|
|
expect(onPendingAnchorChange).toHaveBeenCalledWith(null);
|
|
expect(container.querySelector('[data-testid="document-annotation-selection-toolbar"]')).toBeNull();
|
|
} finally {
|
|
getSelectionSpy.mockRestore();
|
|
}
|
|
});
|
|
|
|
it("does not capture annotation comments from bare contenteditable selections", async () => {
|
|
const body = document.createElement("div");
|
|
const editable = document.createElement("div");
|
|
editable.setAttribute("contenteditable", "");
|
|
const text = document.createTextNode("Editing routine instructions");
|
|
editable.appendChild(text);
|
|
body.appendChild(editable);
|
|
|
|
const range = document.createRange();
|
|
range.setStart(text, 0);
|
|
range.setEnd(text, "Editing".length);
|
|
const getSelectionSpy = vi.spyOn(window, "getSelection").mockReturnValue({
|
|
rangeCount: 1,
|
|
isCollapsed: false,
|
|
getRangeAt: () => range,
|
|
} as unknown as Selection);
|
|
const onPendingAnchorChange = vi.fn();
|
|
root = createRoot(container);
|
|
|
|
try {
|
|
await act(async () => {
|
|
root?.render(
|
|
<DocumentAnnotationLayer
|
|
containerRef={{ current: body }}
|
|
markdown="Editing routine instructions"
|
|
threads={[]}
|
|
focusedThreadId={null}
|
|
onThreadFocus={vi.fn()}
|
|
pendingAnchor={null}
|
|
onPendingAnchorChange={onPendingAnchorChange}
|
|
onRequestComment={vi.fn()}
|
|
/>,
|
|
);
|
|
await new Promise((resolve) => window.requestAnimationFrame(resolve));
|
|
});
|
|
|
|
await act(async () => {
|
|
document.dispatchEvent(new Event("selectionchange"));
|
|
});
|
|
|
|
expect(mockGetContainerTextOffset).not.toHaveBeenCalled();
|
|
expect(mockBuildAnchorFromContainerSelection).not.toHaveBeenCalled();
|
|
expect(onPendingAnchorChange).toHaveBeenCalledWith(null);
|
|
expect(container.querySelector('[data-testid="document-annotation-selection-toolbar"]')).toBeNull();
|
|
} finally {
|
|
getSelectionSpy.mockRestore();
|
|
}
|
|
});
|
|
|
|
it("uses native CSS highlights for visual paint when the browser supports them", async () => {
|
|
const originalCss = globalThis.CSS;
|
|
const originalHighlight = (globalThis as { Highlight?: unknown }).Highlight;
|
|
const setHighlight = vi.fn();
|
|
const deleteHighlight = vi.fn();
|
|
class MockHighlight {
|
|
ranges: Range[];
|
|
|
|
constructor(...ranges: Range[]) {
|
|
this.ranges = ranges;
|
|
}
|
|
}
|
|
(globalThis as { CSS?: unknown }).CSS = {
|
|
...(originalCss ?? {}),
|
|
highlights: {
|
|
set: setHighlight,
|
|
delete: deleteHighlight,
|
|
},
|
|
};
|
|
(globalThis as { Highlight?: unknown }).Highlight = MockHighlight;
|
|
|
|
const body = document.createElement("div");
|
|
body.textContent = "Annotated body text.";
|
|
root = createRoot(container);
|
|
|
|
await act(async () => {
|
|
root?.render(
|
|
<DocumentAnnotationLayer
|
|
containerRef={{ current: body }}
|
|
markdown="Annotated body text."
|
|
threads={[
|
|
{ id: "active", selectedText: "Annotated", status: "open", anchorState: "active" },
|
|
]}
|
|
focusedThreadId={null}
|
|
onThreadFocus={vi.fn()}
|
|
pendingAnchor={null}
|
|
onPendingAnchorChange={vi.fn()}
|
|
onRequestComment={vi.fn()}
|
|
/>,
|
|
);
|
|
await new Promise((resolve) => window.requestAnimationFrame(resolve));
|
|
});
|
|
|
|
expect(container.querySelector(".paperclip-doc-annotation-highlight")).toBeNull();
|
|
expect(container.querySelector(".paperclip-doc-annotation-hit-target")).not.toBeNull();
|
|
const openHighlightCall = setHighlight.mock.calls.find(([name]) => name === "paperclip-doc-annotation-open");
|
|
expect(openHighlightCall).toBeTruthy();
|
|
expect((openHighlightCall?.[1] as MockHighlight).ranges).toHaveLength(1);
|
|
|
|
await act(async () => root?.unmount());
|
|
root = null;
|
|
expect(deleteHighlight).toHaveBeenCalledWith("paperclip-doc-annotation-open");
|
|
|
|
(globalThis as { CSS?: unknown }).CSS = originalCss;
|
|
(globalThis as { Highlight?: unknown }).Highlight = originalHighlight;
|
|
});
|
|
});
|