Skip to content

Commit ba22976

Browse files
authored
fix: inline review comment submit and layout (anomalyco#17948)
1 parent 0afeaea commit ba22976

3 files changed

Lines changed: 131 additions & 7 deletions

File tree

packages/app/e2e/session/session-review.spec.ts

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,101 @@ async function spot(page: Parameters<typeof test>[0]["page"], file: string) {
123123
}, file)
124124
}
125125

126+
async function comment(page: Parameters<typeof test>[0]["page"], file: string, note: string) {
127+
const row = page.locator(`[data-file="${file}"]`).first()
128+
await expect(row).toBeVisible()
129+
130+
const line = row.locator('diffs-container [data-line="2"]').first()
131+
await expect(line).toBeVisible()
132+
await line.hover()
133+
134+
const add = row.getByRole("button", { name: /^Comment$/ }).first()
135+
await expect(add).toBeVisible()
136+
await add.click()
137+
138+
const area = row.locator('[data-slot="line-comment-textarea"]').first()
139+
await expect(area).toBeVisible()
140+
await area.fill(note)
141+
142+
const submit = row.locator('[data-slot="line-comment-action"][data-variant="primary"]').first()
143+
await expect(submit).toBeEnabled()
144+
await submit.click()
145+
146+
await expect(row.locator('[data-slot="line-comment-content"]').filter({ hasText: note }).first()).toBeVisible()
147+
await expect(row.locator('[data-slot="line-comment-tools"]').first()).toBeVisible()
148+
}
149+
150+
async function overflow(page: Parameters<typeof test>[0]["page"], file: string) {
151+
const row = page.locator(`[data-file="${file}"]`).first()
152+
const view = page.locator('[data-slot="session-review-scroll"] .scroll-view__viewport').first()
153+
const pop = row.locator('[data-slot="line-comment-popover"][data-inline-body]').first()
154+
const tools = row.locator('[data-slot="line-comment-tools"]').first()
155+
156+
const [width, viewBox, popBox, toolsBox] = await Promise.all([
157+
view.evaluate((el) => el.scrollWidth - el.clientWidth),
158+
view.boundingBox(),
159+
pop.boundingBox(),
160+
tools.boundingBox(),
161+
])
162+
163+
if (!viewBox || !popBox || !toolsBox) return null
164+
165+
return {
166+
width,
167+
pop: popBox.x + popBox.width - (viewBox.x + viewBox.width),
168+
tools: toolsBox.x + toolsBox.width - (viewBox.x + viewBox.width),
169+
}
170+
}
171+
172+
test("review applies inline comment clicks without horizontal overflow", async ({ page, withProject }) => {
173+
test.setTimeout(180_000)
174+
175+
const tag = `review-comment-${Date.now()}`
176+
const file = `review-comment-${tag}.txt`
177+
const note = `comment ${tag}`
178+
179+
await page.setViewportSize({ width: 1280, height: 900 })
180+
181+
await withProject(async (project) => {
182+
const sdk = createSdk(project.directory)
183+
184+
await withSession(sdk, `e2e review comment ${tag}`, async (session) => {
185+
await patch(sdk, session.id, seed([{ file, mark: tag }]))
186+
187+
await expect
188+
.poll(
189+
async () => {
190+
const diff = await sdk.session.diff({ sessionID: session.id }).then((res) => res.data ?? [])
191+
return diff.length
192+
},
193+
{ timeout: 60_000 },
194+
)
195+
.toBe(1)
196+
197+
await project.gotoSession(session.id)
198+
await show(page)
199+
200+
const tab = page.getByRole("tab", { name: /Review/i }).first()
201+
await expect(tab).toBeVisible()
202+
await tab.click()
203+
204+
await expand(page)
205+
await waitMark(page, file, tag)
206+
await comment(page, file, note)
207+
208+
await expect
209+
.poll(async () => (await overflow(page, file))?.width ?? Number.POSITIVE_INFINITY, { timeout: 10_000 })
210+
.toBeLessThanOrEqual(1)
211+
await expect
212+
.poll(async () => (await overflow(page, file))?.pop ?? Number.POSITIVE_INFINITY, { timeout: 10_000 })
213+
.toBeLessThanOrEqual(1)
214+
await expect
215+
.poll(async () => (await overflow(page, file))?.tools ?? Number.POSITIVE_INFINITY, { timeout: 10_000 })
216+
.toBeLessThanOrEqual(1)
217+
})
218+
})
219+
})
220+
126221
test("review keeps scroll position after a live diff update", async ({ page, withProject }) => {
127222
test.skip(Boolean(process.env.CI), "Flaky in CI for now.")
128223
test.setTimeout(180_000)

packages/ui/src/components/line-comment-styles.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export const lineCommentStyles = `
1515
right: auto;
1616
display: flex;
1717
width: 100%;
18+
min-width: 0;
1819
align-items: flex-start;
1920
}
2021
@@ -64,6 +65,7 @@ export const lineCommentStyles = `
6465
z-index: var(--line-comment-popover-z, 40);
6566
min-width: 200px;
6667
max-width: none;
68+
box-sizing: border-box;
6769
border-radius: 8px;
6870
background: var(--surface-raised-stronger-non-alpha);
6971
box-shadow: var(--shadow-xxs-border);
@@ -75,9 +77,10 @@ export const lineCommentStyles = `
7577
top: auto;
7678
right: auto;
7779
margin-left: 8px;
78-
flex: 0 1 600px;
79-
width: min(100%, 600px);
80-
max-width: min(100%, 600px);
80+
flex: 1 1 0%;
81+
width: auto;
82+
max-width: 100%;
83+
min-width: 0;
8184
}
8285
8386
[data-component="line-comment"][data-inline] [data-slot="line-comment-popover"][data-inline-body] {
@@ -96,37 +99,43 @@ export const lineCommentStyles = `
9699
}
97100
98101
[data-component="line-comment"][data-inline][data-variant="editor"] [data-slot="line-comment-popover"] {
99-
flex-basis: 600px;
102+
width: 100%;
100103
}
101104
102105
[data-component="line-comment"] [data-slot="line-comment-content"] {
103106
display: flex;
104107
flex-direction: column;
105108
gap: 6px;
109+
width: 100%;
110+
min-width: 0;
106111
}
107112
108113
[data-component="line-comment"] [data-slot="line-comment-head"] {
109114
display: flex;
110115
align-items: flex-start;
111116
gap: 8px;
117+
min-width: 0;
112118
}
113119
114120
[data-component="line-comment"] [data-slot="line-comment-text"] {
115121
flex: 1;
122+
min-width: 0;
116123
font-family: var(--font-family-sans);
117124
font-size: var(--font-size-base);
118125
font-weight: var(--font-weight-regular);
119126
line-height: var(--line-height-x-large);
120127
letter-spacing: var(--letter-spacing-normal);
121128
color: var(--text-strong);
122129
white-space: pre-wrap;
130+
overflow-wrap: anywhere;
123131
}
124132
125133
[data-component="line-comment"] [data-slot="line-comment-tools"] {
126134
flex: 0 0 auto;
127135
display: flex;
128136
align-items: center;
129137
justify-content: flex-end;
138+
min-width: 0;
130139
}
131140
132141
[data-component="line-comment"] [data-slot="line-comment-label"],
@@ -137,17 +146,22 @@ export const lineCommentStyles = `
137146
line-height: var(--line-height-large);
138147
letter-spacing: var(--letter-spacing-normal);
139148
color: var(--text-weak);
140-
white-space: nowrap;
149+
min-width: 0;
150+
white-space: normal;
151+
overflow-wrap: anywhere;
141152
}
142153
143154
[data-component="line-comment"] [data-slot="line-comment-editor"] {
144155
display: flex;
145156
flex-direction: column;
146157
gap: 8px;
158+
width: 100%;
159+
min-width: 0;
147160
}
148161
149162
[data-component="line-comment"] [data-slot="line-comment-textarea"] {
150163
width: 100%;
164+
box-sizing: border-box;
151165
resize: vertical;
152166
padding: 8px;
153167
border-radius: var(--radius-md);
@@ -167,11 +181,14 @@ export const lineCommentStyles = `
167181
[data-component="line-comment"] [data-slot="line-comment-actions"] {
168182
display: flex;
169183
align-items: center;
184+
flex-wrap: wrap;
170185
gap: 8px;
171186
padding-left: 8px;
187+
min-width: 0;
172188
}
173189
174190
[data-component="line-comment"] [data-slot="line-comment-editor-label"] {
191+
flex: 1 1 220px;
175192
margin-right: auto;
176193
}
177194

packages/ui/src/components/line-comment.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,16 @@ export const LineCommentEditor = (props: LineCommentEditorProps) => {
206206
const [text, setText] = createSignal(split.value)
207207

208208
const focus = () => refs.textarea?.focus()
209+
const hold: JSX.EventHandlerUnion<HTMLButtonElement, MouseEvent> = (e) => {
210+
e.preventDefault()
211+
e.stopPropagation()
212+
}
213+
const click =
214+
(fn: VoidFunction): JSX.EventHandlerUnion<HTMLButtonElement, MouseEvent> =>
215+
(e) => {
216+
e.stopPropagation()
217+
fn()
218+
}
209219

210220
createEffect(() => {
211221
setText(split.value)
@@ -268,7 +278,8 @@ export const LineCommentEditor = (props: LineCommentEditorProps) => {
268278
type="button"
269279
data-slot="line-comment-action"
270280
data-variant="ghost"
271-
on:click={split.onCancel as any}
281+
on:mousedown={hold as any}
282+
on:click={click(split.onCancel) as any}
272283
>
273284
{split.cancelLabel ?? i18n.t("ui.common.cancel")}
274285
</button>
@@ -277,7 +288,8 @@ export const LineCommentEditor = (props: LineCommentEditorProps) => {
277288
data-slot="line-comment-action"
278289
data-variant="primary"
279290
disabled={text().trim().length === 0}
280-
on:click={submit as any}
291+
on:mousedown={hold as any}
292+
on:click={click(submit) as any}
281293
>
282294
{split.submitLabel ?? i18n.t("ui.lineComment.submit")}
283295
</button>

0 commit comments

Comments
 (0)