Skip to content

Commit adc489d

Browse files
Apply PR #33846: fix(ui): sometimes outlines get clipped
2 parents 3e3bc7d + d8525d7 commit adc489d

2 files changed

Lines changed: 234 additions & 3 deletions

File tree

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
import { expect, test, type Locator, type Page } from "@playwright/test"
2+
import {
3+
assistantMessage,
4+
setupTimeline,
5+
shell,
6+
textPart,
7+
toolPart,
8+
userMessage,
9+
} from "../performance/timeline-stability/fixture"
10+
11+
for (const deviceScaleFactor of [1.25, 1.5]) {
12+
test(`keeps the shell outline inside a fractionally short virtual row at ${deviceScaleFactor}x`, async ({ page }) => {
13+
const shellID = "prt_shell_outline"
14+
const timeline = await setupTimeline(page, {
15+
messages: [userMessage(), assistantMessage([shell(shellID, "completed", "shell output")])],
16+
settings: { newLayoutDesigns: true, shellToolPartsExpanded: true },
17+
reducedMotion: true,
18+
deviceScaleFactor,
19+
})
20+
const part = page.locator(`[data-timeline-part-id="${shellID}"]`)
21+
const output = part.locator('[data-component="bash-output"]')
22+
const row = page.locator("[data-timeline-key]", { has: part })
23+
await expect(output).toBeVisible()
24+
await timeline.settle()
25+
26+
const geometry = await row.evaluate((element) => {
27+
const output = element.querySelector<HTMLElement>('[data-component="bash-output"]')
28+
if (!output) throw new Error("Shell output is unavailable")
29+
const rowRect = element.getBoundingClientRect()
30+
const outputRect = output.getBoundingClientRect()
31+
// Match a rounded-down measurement at a fractional device-pixel phase.
32+
element.style.height = `${outputRect.bottom - rowRect.top - 0.49}px`
33+
element.style.transform = "translateY(0.25px)"
34+
output.style.setProperty("--v2-border-border-base", "rgb(255, 0, 255)")
35+
output.style.setProperty("background", "rgb(0, 0, 0)", "important")
36+
const style = getComputedStyle(output)
37+
return {
38+
outputWidth: outputRect.width,
39+
outputHeight: outputRect.height,
40+
borderColor: style.borderTopColor,
41+
boxShadow: style.boxShadow,
42+
clipMargin: getComputedStyle(element).overflowClipMargin,
43+
}
44+
})
45+
await timeline.settle()
46+
47+
const clipped = await row.evaluate((element) => {
48+
const output = element.querySelector<HTMLElement>('[data-component="bash-output"]')!
49+
return output.getBoundingClientRect().bottom - element.getBoundingClientRect().bottom
50+
})
51+
expect(clipped).toBeCloseTo(0.49, 1)
52+
53+
expect(await page.evaluate(() => devicePixelRatio)).toBe(deviceScaleFactor)
54+
const edges = await captureCardEdges(page, output)
55+
56+
expect(edges.box.width).toBeCloseTo(geometry.outputWidth, 2)
57+
expect(edges.box.height).toBeCloseTo(geometry.outputHeight, 2)
58+
expect(geometry.borderColor).toBe("rgb(255, 0, 255)")
59+
expect(geometry.boxShadow).toBe("none")
60+
expect(geometry.clipMargin).toBe("0.5px")
61+
expect(edges.magenta.top).toBeGreaterThan(0.75)
62+
expect(edges.magenta.bottom).toBeGreaterThan(0.75)
63+
expect(edges.magenta.vertical).toBeGreaterThanOrEqual(2)
64+
})
65+
}
66+
67+
test("keeps the patch card inside a fractionally short virtual row", async ({ page }) => {
68+
const patchID = "prt_patch_outline"
69+
const file = {
70+
filePath: "src/outline.ts",
71+
relativePath: "src/outline.ts",
72+
type: "update",
73+
additions: 1,
74+
deletions: 1,
75+
before: "const outline = false\n",
76+
after: "const outline = true\n",
77+
}
78+
const timeline = await setupTimeline(page, {
79+
messages: [
80+
userMessage(),
81+
assistantMessage([
82+
toolPart(patchID, "apply_patch", "completed", { files: [file.filePath] }, { metadata: { files: [file] } }),
83+
]),
84+
],
85+
settings: { editToolPartsExpanded: true, newLayoutDesigns: true },
86+
reducedMotion: true,
87+
})
88+
const part = page.locator(`[data-timeline-part-id="${patchID}"]`)
89+
const card = part.locator('[data-component="accordion"][data-scope="apply-patch"]')
90+
const row = page.locator("[data-timeline-key]", { has: part })
91+
await expect(card).toBeVisible()
92+
await timeline.settle()
93+
94+
const geometry = await row.evaluate((element) => {
95+
const card = element.querySelector<HTMLElement>('[data-component="accordion"][data-scope="apply-patch"]')
96+
if (!card) throw new Error("Patch card is unavailable")
97+
const rowRect = element.getBoundingClientRect()
98+
const cardRect = card.getBoundingClientRect()
99+
element.style.height = `${cardRect.bottom - rowRect.top - 0.49}px`
100+
const clipMargin = getComputedStyle(element).overflowClipMargin
101+
const bottom = element.getBoundingClientRect().bottom
102+
return {
103+
overflow: card.getBoundingClientRect().bottom - bottom,
104+
paintOverflow: card.getBoundingClientRect().bottom - bottom - Number.parseFloat(clipMargin),
105+
clipMargin,
106+
cardWidth: cardRect.width,
107+
cardHeight: cardRect.height,
108+
}
109+
})
110+
await timeline.settle()
111+
112+
expect(geometry.overflow).toBeCloseTo(0.49, 1)
113+
expect(geometry.paintOverflow).toBeLessThanOrEqual(0)
114+
const edges = await captureCardEdges(page, card)
115+
expect(edges.box.width).toBeCloseTo(geometry.cardWidth, 2)
116+
expect(edges.box.height).toBeCloseTo(geometry.cardHeight, 2)
117+
expect(edges.luminance.top).toBeLessThan(245)
118+
expect(edges.luminance.bottom).toBeLessThan(245)
119+
expect(Math.abs(edges.luminance.bottom - edges.luminance.top)).toBeLessThan(10)
120+
expect(geometry.clipMargin).toBe("0.5px")
121+
})
122+
123+
test("allows paint rounding for every framed row but not fixed turn gaps", async ({ page }) => {
124+
const secondUserID = "msg_outline_second_user"
125+
await setupTimeline(page, {
126+
messages: [
127+
userMessage(undefined, {
128+
summary: {
129+
diffs: [
130+
{
131+
file: "src/summary.ts",
132+
additions: 1,
133+
deletions: 1,
134+
patch: "@@ -1 +1 @@\n-export const value = 1\n+export const value = 2",
135+
},
136+
],
137+
},
138+
}),
139+
assistantMessage([textPart("prt_outline_text", "Assistant text")]),
140+
userMessage(undefined, { id: secondUserID, created: 1700000010000 }),
141+
assistantMessage([], {
142+
id: "msg_outline_second_assistant",
143+
parentID: secondUserID,
144+
created: 1700000011000,
145+
}),
146+
],
147+
})
148+
await expect(page.locator('[data-timeline-row="DiffSummary"]')).toBeVisible()
149+
await expect(page.locator('[data-timeline-row="TurnGap"]')).toBeVisible()
150+
151+
const rows = await page.locator("[data-timeline-key]").evaluateAll((elements) =>
152+
elements.map((element) => ({
153+
tag: element.querySelector<HTMLElement>("[data-timeline-row]")?.dataset.timelineRow,
154+
clipMargin: getComputedStyle(element).overflowClipMargin,
155+
})),
156+
)
157+
expect(rows.filter((row) => row.tag !== "TurnGap").every((row) => row.clipMargin === "0.5px")).toBe(true)
158+
expect(rows.filter((row) => row.tag === "TurnGap")).toEqual([{ tag: "TurnGap", clipMargin: "0px" }])
159+
})
160+
161+
async function captureCardEdges(page: Page, card: Locator) {
162+
const box = await card.boundingBox()
163+
if (!box) throw new Error("Tool card bounds are unavailable")
164+
const viewport = page.viewportSize()
165+
if (!viewport) throw new Error("Viewport bounds are unavailable")
166+
const screenshot = await page.screenshot()
167+
return page.evaluate(
168+
async ({ source, box, viewport }) => {
169+
const image = new Image()
170+
image.src = source
171+
await image.decode()
172+
const canvas = document.createElement("canvas")
173+
canvas.width = image.naturalWidth
174+
canvas.height = image.naturalHeight
175+
const context = canvas.getContext("2d")
176+
if (!context) throw new Error("2D canvas is unavailable")
177+
context.drawImage(image, 0, 0)
178+
const scale = {
179+
x: image.naturalWidth / viewport.width,
180+
y: image.naturalHeight / viewport.height,
181+
}
182+
const rows = (candidates: number[]) => {
183+
const left = Math.floor((box.x + 8) * scale.x)
184+
const width = Math.floor((box.width - 16) * scale.x)
185+
return candidates.map((row) => {
186+
const pixels = context.getImageData(left, row, width, 1).data
187+
const indexes = Array.from({ length: width }, (_, index) => index * 4)
188+
return {
189+
luminance:
190+
indexes
191+
.map((index) => (pixels[index]! + pixels[index + 1]! + pixels[index + 2]!) / 3)
192+
.reduce((sum, value) => sum + value, 0) / width,
193+
magenta:
194+
indexes.filter((index) => pixels[index]! > 200 && pixels[index + 1]! < 180 && pixels[index + 2]! > 200)
195+
.length / width,
196+
}
197+
})
198+
}
199+
const pixels = context.getImageData(0, 0, image.naturalWidth, image.naturalHeight).data
200+
const columns = new Uint32Array(image.naturalWidth)
201+
for (let index = 0; index < pixels.length; index += 4) {
202+
if (pixels[index]! <= 200 || pixels[index + 1]! >= 180 || pixels[index + 2]! <= 200) continue
203+
columns[(index / 4) % image.naturalWidth] = columns[(index / 4) % image.naturalWidth]! + 1
204+
}
205+
const top = box.y * scale.y
206+
const bottom = (box.y + box.height) * scale.y
207+
const topRows = rows([Math.floor(top) - 1, Math.floor(top), Math.ceil(top)])
208+
const bottomRows = rows([Math.floor(bottom) - 2, Math.floor(bottom) - 1, Math.ceil(bottom) - 1])
209+
return {
210+
box,
211+
luminance: {
212+
top: Math.min(...topRows.map((row) => row.luminance)),
213+
bottom: rows([Math.ceil(bottom) - 1])[0]!.luminance,
214+
},
215+
magenta: {
216+
top: Math.max(...topRows.map((row) => row.magenta)),
217+
bottom: Math.max(...bottomRows.map((row) => row.magenta)),
218+
vertical: Array.from(columns).filter((count) => count > box.height * scale.y * 0.75).length,
219+
},
220+
}
221+
},
222+
{
223+
source: `data:image/png;base64,${screenshot.toString("base64")}`,
224+
viewport,
225+
box,
226+
},
227+
)
228+
}

packages/app/src/pages/session/timeline/message-timeline.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,12 +1243,13 @@ export function MessageTimeline(props: {
12431243
const initialRow = timelineRowByKey().get(props.rowKey)!
12441244
const item = createMemo(() => virtualItemByKey().get(props.rowKey) ?? initialItem)
12451245
const row = createMemo(() => timelineRowByKey().get(props.rowKey) ?? initialRow)
1246-
const asyncFile = () => {
1246+
const tool = () => {
12471247
const value = row()
1248-
if (value._tag !== "AssistantPart" || value.group.type !== "part") return false
1248+
if (value._tag !== "AssistantPart" || value.group.type !== "part") return
12491249
const part = getMsgPart(value.group.ref.messageID, value.group.ref.partID)
1250-
return part?.type === "tool" && ["edit", "write", "apply_patch"].includes(part.tool)
1250+
if (part?.type === "tool") return part
12511251
}
1252+
const asyncFile = () => ["edit", "write", "apply_patch"].includes(tool()?.tool ?? "")
12521253
const [ready, setReady] = createSignal(initialItem.size <= timelineFallbackItemSize || !asyncFile())
12531254
let contentMeasureFrame: number | undefined
12541255

@@ -1278,6 +1279,8 @@ export function MessageTimeline(props: {
12781279
width: "100%",
12791280
height: `${item().size}px`,
12801281
overflow: "clip",
1282+
// Rounded virtual measurements can otherwise clip a framed row's outer paint.
1283+
"overflow-clip-margin": row()._tag === "TurnGap" ? undefined : "0.5px",
12811284
}}
12821285
>
12831286
<div

0 commit comments

Comments
 (0)