From ba2297656877f26c50d28977b0e7164868d6868c Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Tue, 17 Mar 2026 19:54:20 +0530 Subject: [PATCH] fix: inline review comment submit and layout (#17948) --- .../app/e2e/session/session-review.spec.ts | 95 +++++++++++++++++++ .../ui/src/components/line-comment-styles.ts | 27 +++++- packages/ui/src/components/line-comment.tsx | 16 +++- 3 files changed, 131 insertions(+), 7 deletions(-) diff --git a/packages/app/e2e/session/session-review.spec.ts b/packages/app/e2e/session/session-review.spec.ts index 28c85edb0..c0421f028 100644 --- a/packages/app/e2e/session/session-review.spec.ts +++ b/packages/app/e2e/session/session-review.spec.ts @@ -123,6 +123,101 @@ async function spot(page: Parameters[0]["page"], file: string) { }, file) } +async function comment(page: Parameters[0]["page"], file: string, note: string) { + const row = page.locator(`[data-file="${file}"]`).first() + await expect(row).toBeVisible() + + const line = row.locator('diffs-container [data-line="2"]').first() + await expect(line).toBeVisible() + await line.hover() + + const add = row.getByRole("button", { name: /^Comment$/ }).first() + await expect(add).toBeVisible() + await add.click() + + const area = row.locator('[data-slot="line-comment-textarea"]').first() + await expect(area).toBeVisible() + await area.fill(note) + + const submit = row.locator('[data-slot="line-comment-action"][data-variant="primary"]').first() + await expect(submit).toBeEnabled() + await submit.click() + + await expect(row.locator('[data-slot="line-comment-content"]').filter({ hasText: note }).first()).toBeVisible() + await expect(row.locator('[data-slot="line-comment-tools"]').first()).toBeVisible() +} + +async function overflow(page: Parameters[0]["page"], file: string) { + const row = page.locator(`[data-file="${file}"]`).first() + const view = page.locator('[data-slot="session-review-scroll"] .scroll-view__viewport').first() + const pop = row.locator('[data-slot="line-comment-popover"][data-inline-body]').first() + const tools = row.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) + + const tag = `review-comment-${Date.now()}` + const file = `review-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 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 comment(page, file, note) + + await expect + .poll(async () => (await overflow(page, file))?.width ?? Number.POSITIVE_INFINITY, { timeout: 10_000 }) + .toBeLessThanOrEqual(1) + await expect + .poll(async () => (await overflow(page, file))?.pop ?? Number.POSITIVE_INFINITY, { timeout: 10_000 }) + .toBeLessThanOrEqual(1) + await expect + .poll(async () => (await overflow(page, file))?.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/ui/src/components/line-comment-styles.ts b/packages/ui/src/components/line-comment-styles.ts index d5be67554..8fd02f088 100644 --- a/packages/ui/src/components/line-comment-styles.ts +++ b/packages/ui/src/components/line-comment-styles.ts @@ -15,6 +15,7 @@ export const lineCommentStyles = ` right: auto; display: flex; width: 100%; + min-width: 0; align-items: flex-start; } @@ -64,6 +65,7 @@ export const lineCommentStyles = ` z-index: var(--line-comment-popover-z, 40); min-width: 200px; max-width: none; + box-sizing: border-box; border-radius: 8px; background: var(--surface-raised-stronger-non-alpha); box-shadow: var(--shadow-xxs-border); @@ -75,9 +77,10 @@ export const lineCommentStyles = ` top: auto; right: auto; margin-left: 8px; - flex: 0 1 600px; - width: min(100%, 600px); - max-width: min(100%, 600px); + flex: 1 1 0%; + width: auto; + max-width: 100%; + min-width: 0; } [data-component="line-comment"][data-inline] [data-slot="line-comment-popover"][data-inline-body] { @@ -96,23 +99,27 @@ export const lineCommentStyles = ` } [data-component="line-comment"][data-inline][data-variant="editor"] [data-slot="line-comment-popover"] { - flex-basis: 600px; + width: 100%; } [data-component="line-comment"] [data-slot="line-comment-content"] { display: flex; flex-direction: column; gap: 6px; + width: 100%; + min-width: 0; } [data-component="line-comment"] [data-slot="line-comment-head"] { display: flex; align-items: flex-start; gap: 8px; + min-width: 0; } [data-component="line-comment"] [data-slot="line-comment-text"] { flex: 1; + min-width: 0; font-family: var(--font-family-sans); font-size: var(--font-size-base); font-weight: var(--font-weight-regular); @@ -120,6 +127,7 @@ export const lineCommentStyles = ` letter-spacing: var(--letter-spacing-normal); color: var(--text-strong); white-space: pre-wrap; + overflow-wrap: anywhere; } [data-component="line-comment"] [data-slot="line-comment-tools"] { @@ -127,6 +135,7 @@ export const lineCommentStyles = ` display: flex; align-items: center; justify-content: flex-end; + min-width: 0; } [data-component="line-comment"] [data-slot="line-comment-label"], @@ -137,17 +146,22 @@ export const lineCommentStyles = ` line-height: var(--line-height-large); letter-spacing: var(--letter-spacing-normal); color: var(--text-weak); - white-space: nowrap; + min-width: 0; + white-space: normal; + overflow-wrap: anywhere; } [data-component="line-comment"] [data-slot="line-comment-editor"] { display: flex; flex-direction: column; gap: 8px; + width: 100%; + min-width: 0; } [data-component="line-comment"] [data-slot="line-comment-textarea"] { width: 100%; + box-sizing: border-box; resize: vertical; padding: 8px; border-radius: var(--radius-md); @@ -167,11 +181,14 @@ export const lineCommentStyles = ` [data-component="line-comment"] [data-slot="line-comment-actions"] { display: flex; align-items: center; + flex-wrap: wrap; gap: 8px; padding-left: 8px; + min-width: 0; } [data-component="line-comment"] [data-slot="line-comment-editor-label"] { + flex: 1 1 220px; margin-right: auto; } diff --git a/packages/ui/src/components/line-comment.tsx b/packages/ui/src/components/line-comment.tsx index ff5d1df00..bc47ad940 100644 --- a/packages/ui/src/components/line-comment.tsx +++ b/packages/ui/src/components/line-comment.tsx @@ -206,6 +206,16 @@ export const LineCommentEditor = (props: LineCommentEditorProps) => { const [text, setText] = createSignal(split.value) const focus = () => refs.textarea?.focus() + const hold: JSX.EventHandlerUnion = (e) => { + e.preventDefault() + e.stopPropagation() + } + const click = + (fn: VoidFunction): JSX.EventHandlerUnion => + (e) => { + e.stopPropagation() + fn() + } createEffect(() => { setText(split.value) @@ -268,7 +278,8 @@ export const LineCommentEditor = (props: LineCommentEditorProps) => { type="button" data-slot="line-comment-action" data-variant="ghost" - on:click={split.onCancel as any} + on:mousedown={hold as any} + on:click={click(split.onCancel) as any} > {split.cancelLabel ?? i18n.t("ui.common.cancel")} @@ -277,7 +288,8 @@ export const LineCommentEditor = (props: LineCommentEditorProps) => { data-slot="line-comment-action" data-variant="primary" disabled={text().trim().length === 0} - on:click={submit as any} + on:mousedown={hold as any} + on:click={click(submit) as any} > {split.submitLabel ?? i18n.t("ui.lineComment.submit")}