From 27a70ad70f30faf30d159f56b394c01f9474c7a4 Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Fri, 20 Mar 2026 20:14:18 +0530 Subject: [PATCH] fix(app): align review file comments with diff comments (#18406) --- .../app/e2e/session/session-review.spec.ts | 114 ++++++++++++++++++ packages/app/src/pages/session/file-tabs.tsx | 12 -- 2 files changed, 114 insertions(+), 12 deletions(-) diff --git a/packages/app/e2e/session/session-review.spec.ts b/packages/app/e2e/session/session-review.spec.ts index c0421f028..07071239d 100644 --- a/packages/app/e2e/session/session-review.spec.ts +++ b/packages/app/e2e/session/session-review.spec.ts @@ -169,6 +169,70 @@ async function overflow(page: Parameters[0]["page"], file: string) } } +async function openReviewFile(page: Parameters[0]["page"], file: string) { + const row = page.locator(`[data-file="${file}"]`).first() + await expect(row).toBeVisible() + await row.hover() + + const open = row.getByRole("button", { name: /^Open file$/i }).first() + await expect(open).toBeVisible() + await open.click() + + const tab = page.getByRole("tab", { name: file }).first() + await expect(tab).toBeVisible() + await tab.click() + + const viewer = page.locator('[data-component="file"][data-mode="text"]').first() + await expect(viewer).toBeVisible() + return viewer +} + +async function fileComment(page: Parameters[0]["page"], note: string) { + const viewer = page.locator('[data-component="file"][data-mode="text"]').first() + await expect(viewer).toBeVisible() + + const line = viewer.locator('diffs-container [data-line="2"]').first() + await expect(line).toBeVisible() + await line.hover() + + const add = viewer.getByRole("button", { name: /^Comment$/ }).first() + await expect(add).toBeVisible() + await add.click() + + const area = viewer.locator('[data-slot="line-comment-textarea"]').first() + await expect(area).toBeVisible() + await area.fill(note) + + const submit = viewer.locator('[data-slot="line-comment-action"][data-variant="primary"]').first() + await expect(submit).toBeEnabled() + await submit.click() + + await expect(viewer.locator('[data-slot="line-comment-content"]').filter({ hasText: note }).first()).toBeVisible() + await expect(viewer.locator('[data-slot="line-comment-tools"]').first()).toBeVisible() +} + +async function fileOverflow(page: Parameters[0]["page"]) { + const viewer = page.locator('[data-component="file"][data-mode="text"]').first() + const view = page.locator('[role="tabpanel"] .scroll-view__viewport').first() + const pop = viewer.locator('[data-slot="line-comment-popover"][data-inline-body]').first() + const tools = viewer.locator('[data-slot="line-comment-tools"]').first() + + const [width, viewBox, popBox, toolsBox] = await Promise.all([ + view.evaluate((el) => el.scrollWidth - el.clientWidth), + view.boundingBox(), + pop.boundingBox(), + tools.boundingBox(), + ]) + + if (!viewBox || !popBox || !toolsBox) return null + + return { + width, + pop: popBox.x + popBox.width - (viewBox.x + viewBox.width), + tools: toolsBox.x + toolsBox.width - (viewBox.x + viewBox.width), + } +} + test("review applies inline comment clicks without horizontal overflow", async ({ page, withProject }) => { test.setTimeout(180_000) @@ -218,6 +282,56 @@ test("review applies inline comment clicks without horizontal overflow", async ( }) }) +test("review file comments submit on click without clipping actions", async ({ page, withProject }) => { + test.setTimeout(180_000) + + const tag = `review-file-comment-${Date.now()}` + const file = `review-file-comment-${tag}.txt` + const note = `comment ${tag}` + + await page.setViewportSize({ width: 1280, height: 900 }) + + await withProject(async (project) => { + const sdk = createSdk(project.directory) + + await withSession(sdk, `e2e review file comment ${tag}`, async (session) => { + await patch(sdk, session.id, seed([{ file, mark: tag }])) + + await expect + .poll( + async () => { + const diff = await sdk.session.diff({ sessionID: session.id }).then((res) => res.data ?? []) + return diff.length + }, + { timeout: 60_000 }, + ) + .toBe(1) + + await project.gotoSession(session.id) + await show(page) + + const tab = page.getByRole("tab", { name: /Review/i }).first() + await expect(tab).toBeVisible() + await tab.click() + + await expand(page) + await waitMark(page, file, tag) + await openReviewFile(page, file) + await fileComment(page, note) + + await expect + .poll(async () => (await fileOverflow(page))?.width ?? Number.POSITIVE_INFINITY, { timeout: 10_000 }) + .toBeLessThanOrEqual(1) + await expect + .poll(async () => (await fileOverflow(page))?.pop ?? Number.POSITIVE_INFINITY, { timeout: 10_000 }) + .toBeLessThanOrEqual(1) + await expect + .poll(async () => (await fileOverflow(page))?.tools ?? Number.POSITIVE_INFINITY, { timeout: 10_000 }) + .toBeLessThanOrEqual(1) + }) + }) +}) + test("review keeps scroll position after a live diff update", async ({ page, withProject }) => { test.skip(Boolean(process.env.CI), "Flaky in CI for now.") test.setTimeout(180_000) diff --git a/packages/app/src/pages/session/file-tabs.tsx b/packages/app/src/pages/session/file-tabs.tsx index a3379905d..e3b57fdf2 100644 --- a/packages/app/src/pages/session/file-tabs.tsx +++ b/packages/app/src/pages/session/file-tabs.tsx @@ -217,17 +217,6 @@ export function FileTabContent(props: { tab: string }) { onDelete={controls.remove} /> ), - onDraftPopoverFocusOut: (e: FocusEvent) => { - const current = e.currentTarget as HTMLDivElement - const target = e.relatedTarget - if (target instanceof Node && current.contains(target)) return - - setTimeout(() => { - if (!document.activeElement || !current.contains(document.activeElement)) { - setNote("commenting", null) - } - }, 0) - }, }) createEffect(() => { @@ -426,7 +415,6 @@ export function FileTabContent(props: { tab: string }) { commentsUi.onLineSelectionEnd(range) }} search={search} - overflow="scroll" class="select-text" media={{ mode: "auto",