Skip to content

Commit ed87736

Browse files
azizmejri1claude
azizmejri1
and
claude
authored
Plan annotator (#3043)
close #2989 <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/3043" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> --------- Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent 347451d commit ed87736

16 files changed

+1662
-25
lines changed

‎e2e-tests/helpers/page-objects/components/PreviewPanel.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,63 @@ import { Timeout } from "../../constants";
99
export class PreviewPanel {
1010
constructor(public page: Page) {}
1111

12+
getPlanContent() {
13+
return this.page.getByTestId("plan-content");
14+
}
15+
16+
getPlanSelectionCommentButton() {
17+
return this.page.getByRole("button", { name: "Add comment" });
18+
}
19+
20+
getPlanCommentsButton() {
21+
return this.page.getByRole("button", { name: "View comments" });
22+
}
23+
24+
getPlanAnnotationMarks() {
25+
return this.page.locator("mark[data-annotation-id]");
26+
}
27+
28+
async selectTextInPlan(selectedText: string) {
29+
const planContent = this.getPlanContent();
30+
await expect(planContent).toBeVisible({ timeout: Timeout.MEDIUM });
31+
32+
await planContent.evaluate((container, text) => {
33+
const walker = document.createTreeWalker(
34+
container,
35+
NodeFilter.SHOW_TEXT,
36+
{
37+
acceptNode: (node) =>
38+
(node.textContent ?? "").trim().length > 0
39+
? NodeFilter.FILTER_ACCEPT
40+
: NodeFilter.FILTER_REJECT,
41+
},
42+
);
43+
44+
let currentNode: Text | null;
45+
while ((currentNode = walker.nextNode() as Text | null)) {
46+
const startOffset = currentNode.textContent?.indexOf(text) ?? -1;
47+
if (startOffset === -1) {
48+
continue;
49+
}
50+
51+
const range = document.createRange();
52+
range.setStart(currentNode, startOffset);
53+
range.setEnd(currentNode, startOffset + text.length);
54+
55+
const selection = window.getSelection();
56+
selection?.removeAllRanges();
57+
selection?.addRange(range);
58+
59+
(currentNode.parentElement ?? container).dispatchEvent(
60+
new MouseEvent("mouseup", { bubbles: true }),
61+
);
62+
return;
63+
}
64+
65+
throw new Error(`Could not find "${text}" in plan content`);
66+
}, selectedText);
67+
}
68+
1269
async selectPreviewMode(
1370
mode:
1471
| "code"

‎e2e-tests/plan_mode.spec.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,116 @@ testSkipIfWindows("plan mode - questionnaire flow", async ({ po }) => {
8888
// Snapshot the messages
8989
await po.snapshotMessages();
9090
});
91+
92+
testSkipIfWindows(
93+
"plan mode - add and review plan annotations",
94+
async ({ po }) => {
95+
await po.setUpDyadPro({ localAgent: true });
96+
await po.importApp("minimal");
97+
await po.chatActions.selectChatMode("plan");
98+
99+
await po.sendPrompt("tc=local-agent/accept-plan");
100+
101+
await expect(
102+
po.page.getByRole("button", { name: "Accept Plan" }),
103+
).toBeVisible({
104+
timeout: Timeout.MEDIUM,
105+
});
106+
107+
await po.previewPanel.selectTextInPlan("Step two");
108+
109+
const addCommentButton = po.previewPanel.getPlanSelectionCommentButton();
110+
await expect(addCommentButton).toBeVisible({ timeout: Timeout.MEDIUM });
111+
await addCommentButton.click();
112+
await expect(po.page.getByRole("button", { name: "Cancel" })).toBeVisible();
113+
await po.page.getByRole("button", { name: "Cancel" }).click();
114+
115+
await expect(po.page.getByPlaceholder("Add your comment...")).toBeHidden();
116+
await expect(addCommentButton).toBeVisible({ timeout: Timeout.MEDIUM });
117+
await addCommentButton.click();
118+
119+
await po.page
120+
.getByPlaceholder("Add your comment...")
121+
.fill("Add more detail for step two.");
122+
123+
await po.previewPanel.getPlanContent().evaluate((container) => {
124+
let scrollParent: HTMLElement | null = container.parentElement;
125+
126+
while (scrollParent) {
127+
const { overflowY } = window.getComputedStyle(scrollParent);
128+
const isScrollable =
129+
overflowY === "auto" ||
130+
overflowY === "scroll" ||
131+
overflowY === "overlay";
132+
if (isScrollable) {
133+
scrollParent.scrollTop += 48;
134+
scrollParent.dispatchEvent(new Event("scroll"));
135+
return;
136+
}
137+
138+
scrollParent = scrollParent.parentElement;
139+
}
140+
141+
throw new Error("Could not find a scrollable plan container");
142+
});
143+
144+
await expect(po.page.getByPlaceholder("Add your comment...")).toHaveValue(
145+
"Add more detail for step two.",
146+
);
147+
await po.page.getByRole("button", { name: "Add Comment" }).click();
148+
149+
const commentsButton = po.previewPanel.getPlanCommentsButton();
150+
await expect(commentsButton).toBeVisible({ timeout: Timeout.MEDIUM });
151+
await expect(po.previewPanel.getPlanAnnotationMarks()).toHaveCount(1);
152+
await expect(
153+
po.previewPanel.getPlanAnnotationMarks().first(),
154+
).toContainText("Step two");
155+
await expect(
156+
po.previewPanel.getPlanAnnotationMarks().first(),
157+
).toHaveAttribute("role", "button");
158+
159+
await commentsButton.click();
160+
await expect(po.page.getByText("Comments (1)")).toBeVisible({
161+
timeout: Timeout.MEDIUM,
162+
});
163+
await expect(
164+
po.page.getByText("Add more detail for step two."),
165+
).toBeVisible();
166+
167+
await commentsButton.click();
168+
await expect(po.page.getByText("Comments (1)")).toBeHidden();
169+
170+
await po.previewPanel.getPlanAnnotationMarks().first().press("Enter");
171+
const commentDialog = po.page.getByRole("dialog", {
172+
name: "Comment on selected text",
173+
});
174+
await expect(commentDialog).toBeVisible({ timeout: Timeout.MEDIUM });
175+
await expect(
176+
po.page.getByRole("button", { name: "Edit comment" }),
177+
).toBeVisible({ timeout: Timeout.MEDIUM });
178+
await expect(
179+
po.page.getByRole("button", { name: "Edit comment" }),
180+
).toBeFocused();
181+
await expect(
182+
po.page.getByText("Add more detail for step two."),
183+
).toBeVisible();
184+
185+
// Close the comment dialog and send the annotations
186+
await po.page.keyboard.press("Escape");
187+
await expect(commentDialog).toBeHidden();
188+
189+
await commentsButton.click();
190+
await expect(po.page.getByText("Comments (1)")).toBeVisible({
191+
timeout: Timeout.MEDIUM,
192+
});
193+
await po.page.getByRole("button", { name: "Send Comments" }).click();
194+
195+
// Wait for annotations to be cleared (indicates send succeeded)
196+
await expect(po.previewPanel.getPlanAnnotationMarks()).toHaveCount(0, {
197+
timeout: Timeout.MEDIUM,
198+
});
199+
200+
// Verify the request sent to the server contains the correctly formatted comments
201+
await po.snapshotServerDump("last-message");
202+
},
203+
);

‎e2e-tests/snapshots/plan_mode.spec.ts_plan-mode---add-and-review-plan-annotations-1.txt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
===
2+
role: user
3+
message: I have the following comments on the plan:
4+
5+
**Comment 1:**
6+
> Step two
7+
8+
Add more detail for step two.
9+
10+
Please update the plan based on these comments.

‎src/__tests__/plan_annotation_dom.test.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import { beforeEach, describe, expect, it } from "vitest";
2+
import type { PlanAnnotation } from "@/atoms/planAtoms";
3+
import {
4+
applyPlanAnnotationHighlights,
5+
clearPlanAnnotationHighlights,
6+
getPlanSelectionSnapshot,
7+
} from "@/components/preview_panel/plan/planAnnotationDom";
8+
9+
function createAnnotation(overrides: Partial<PlanAnnotation>): PlanAnnotation {
10+
return {
11+
id: "annotation-1",
12+
chatId: 1,
13+
selectedText: "",
14+
comment: "comment",
15+
createdAt: 1,
16+
startOffset: 0,
17+
selectionLength: 0,
18+
...overrides,
19+
};
20+
}
21+
22+
describe("planAnnotationDom", () => {
23+
beforeEach(() => {
24+
document.body.innerHTML = "";
25+
});
26+
27+
it("computes selection offsets from rendered plan text while ignoring plan UI chrome", () => {
28+
const container = document.createElement("div");
29+
container.innerHTML =
30+
"<p>Intro</p><div data-plan-annotation-ignore>Copy</div><p> Hello world </p>";
31+
document.body.appendChild(container);
32+
33+
const textNode = container.querySelectorAll("p")[1]?.firstChild;
34+
expect(textNode).not.toBeNull();
35+
36+
const range = document.createRange();
37+
range.setStart(textNode!, 0);
38+
range.setEnd(textNode!, textNode!.textContent?.length ?? 0);
39+
40+
expect(getPlanSelectionSnapshot(container, range)).toEqual({
41+
selectedText: "Hello world",
42+
startOffset: 7,
43+
selectionLength: 11,
44+
});
45+
});
46+
47+
it("highlights text that spans multiple inline nodes", () => {
48+
const container = document.createElement("div");
49+
container.innerHTML = `<p>Hello <strong>bold</strong> world</p>`;
50+
document.body.appendChild(container);
51+
52+
applyPlanAnnotationHighlights(container, [
53+
createAnnotation({
54+
id: "annotation-1",
55+
selectedText: "bold world",
56+
startOffset: 6,
57+
selectionLength: 10,
58+
}),
59+
]);
60+
61+
const marks = [
62+
...container.querySelectorAll<HTMLElement>(
63+
'mark[data-annotation-id="annotation-1"]',
64+
),
65+
];
66+
67+
expect(marks).toHaveLength(2);
68+
expect(marks.map((mark) => mark.textContent).join("")).toBe("bold world");
69+
expect(marks[0]?.getAttribute("role")).toBe("button");
70+
expect(marks[0]?.getAttribute("tabindex")).toBe("0");
71+
expect(marks[0]?.getAttribute("aria-haspopup")).toBe("dialog");
72+
expect(marks[0]?.getAttribute("aria-label")).toBe(
73+
"View comment for bold world",
74+
);
75+
76+
clearPlanAnnotationHighlights(container);
77+
expect(container.querySelectorAll("mark[data-annotation-id]")).toHaveLength(
78+
0,
79+
);
80+
expect(container.textContent).toBe("Hello bold world");
81+
});
82+
83+
it("skips stale or overlapping annotations instead of corrupting the DOM", () => {
84+
const container = document.createElement("div");
85+
container.innerHTML = `<p>Hello brave new world</p>`;
86+
document.body.appendChild(container);
87+
88+
applyPlanAnnotationHighlights(container, [
89+
createAnnotation({
90+
id: "valid",
91+
selectedText: "brave",
92+
startOffset: 6,
93+
selectionLength: 5,
94+
}),
95+
createAnnotation({
96+
id: "stale",
97+
selectedText: "planet",
98+
startOffset: 12,
99+
selectionLength: 6,
100+
}),
101+
createAnnotation({
102+
id: "overlap",
103+
selectedText: "ave new",
104+
startOffset: 8,
105+
selectionLength: 7,
106+
}),
107+
]);
108+
109+
const marks = [
110+
...container.querySelectorAll<HTMLElement>("mark[data-annotation-id]"),
111+
];
112+
113+
expect(marks).toHaveLength(1);
114+
expect(marks[0]?.getAttribute("data-annotation-id")).toBe("valid");
115+
expect(marks[0]?.textContent).toBe("brave");
116+
});
117+
});

‎src/__tests__/selectionCommentButtonPosition.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { describe, expect, it } from "vitest";
2+
import { getSelectionCommentAnchorRect } from "@/components/preview_panel/plan/selectionCommentButtonPosition";
3+
4+
describe("getSelectionCommentAnchorRect", () => {
5+
it("anchors multi-line selections to the last client rect", () => {
6+
const firstRect = new DOMRect(10, 20, 100, 18);
7+
const lastRect = new DOMRect(12, 44, 60, 18);
8+
const fallbackRect = new DOMRect(10, 20, 140, 42);
9+
const range = {
10+
getClientRects: () =>
11+
[firstRect, lastRect] as unknown as DOMRectList | DOMRect[],
12+
getBoundingClientRect: () => fallbackRect,
13+
};
14+
15+
expect(getSelectionCommentAnchorRect(range)).toBe(lastRect);
16+
});
17+
18+
it("falls back to the bounding rect when client rects are empty", () => {
19+
const fallbackRect = new DOMRect(8, 16, 120, 32);
20+
const range = {
21+
getClientRects: () => [] as unknown as DOMRectList | DOMRect[],
22+
getBoundingClientRect: () => fallbackRect,
23+
};
24+
25+
expect(getSelectionCommentAnchorRect(range)).toBe(fallbackRect);
26+
});
27+
});

0 commit comments

Comments
 (0)