Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions packages/app/e2e/session/session-review.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,101 @@ async function spot(page: Parameters<typeof test>[0]["page"], file: string) {
}, file)
}

async function comment(page: Parameters<typeof test>[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<typeof test>[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)
Expand Down
27 changes: 22 additions & 5 deletions packages/ui/src/components/line-comment-styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const lineCommentStyles = `
right: auto;
display: flex;
width: 100%;
min-width: 0;
align-items: flex-start;
}

Expand Down Expand Up @@ -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);
Expand All @@ -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] {
Expand All @@ -96,37 +99,43 @@ 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);
line-height: var(--line-height-x-large);
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"] {
flex: 0 0 auto;
display: flex;
align-items: center;
justify-content: flex-end;
min-width: 0;
}

[data-component="line-comment"] [data-slot="line-comment-label"],
Expand All @@ -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);
Expand All @@ -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;
}

Expand Down
16 changes: 14 additions & 2 deletions packages/ui/src/components/line-comment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,16 @@ export const LineCommentEditor = (props: LineCommentEditorProps) => {
const [text, setText] = createSignal(split.value)

const focus = () => refs.textarea?.focus()
const hold: JSX.EventHandlerUnion<HTMLButtonElement, MouseEvent> = (e) => {
e.preventDefault()
e.stopPropagation()
}
const click =
(fn: VoidFunction): JSX.EventHandlerUnion<HTMLButtonElement, MouseEvent> =>
(e) => {
e.stopPropagation()
fn()
}

createEffect(() => {
setText(split.value)
Expand Down Expand Up @@ -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")}
</button>
Expand All @@ -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")}
</button>
Expand Down
Loading