mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-01 04:46:43 +08:00
Merge pull request #13273 from Kilo-Org/investigate-session-scroll-regression
fix(vscode): keep session scroll pinned during layout corrections
This commit is contained in:
@@ -2,4 +2,4 @@
|
||||
"@kilocode/kilo-ui": patch
|
||||
---
|
||||
|
||||
Keep chat pinned to the latest streaming output through layout reflows and downward scrolling.
|
||||
Keep chat pinned to the latest streaming output through virtualized layout corrections, reflows, and downward scrolling.
|
||||
|
||||
+3
@@ -0,0 +1,3 @@
|
||||
version https://git-lfs.github.com/spec/v1
|
||||
oid sha256:c913b1d73750765dd88b170364776f0a6ffbeb65f7fa5fcfce953a8082c5d46c
|
||||
size 28560
|
||||
@@ -10,6 +10,7 @@ mock.module("@solid-primitives/resize-observer", () => ({
|
||||
}))
|
||||
|
||||
const originalElement = globalThis.Element
|
||||
const originalNode = globalThis.Node
|
||||
const originalWheelEvent = globalThis.WheelEvent
|
||||
|
||||
type Listener = {
|
||||
@@ -22,12 +23,28 @@ class FakeElement {
|
||||
clientHeight = 100
|
||||
scrollTop = 0
|
||||
style = { overflowAnchor: "" }
|
||||
hovered = false
|
||||
ownerDocument!: FakeDocument
|
||||
private children = new Set<FakeElement>()
|
||||
private listeners = new Map<string, Listener[]>()
|
||||
|
||||
closest() {
|
||||
return null
|
||||
}
|
||||
|
||||
contains(node: unknown) {
|
||||
return node === this || (node instanceof FakeElement && [...this.children].some((child) => child.contains(node)))
|
||||
}
|
||||
|
||||
append(child: FakeElement) {
|
||||
child.ownerDocument = this.ownerDocument
|
||||
this.children.add(child)
|
||||
}
|
||||
|
||||
matches(selector: string) {
|
||||
return selector === ":hover" && this.hovered
|
||||
}
|
||||
|
||||
scrollTo(options: ScrollToOptions) {
|
||||
this.scrollTop = options.top ?? this.scrollTop
|
||||
}
|
||||
@@ -65,6 +82,20 @@ class FakeElement {
|
||||
}
|
||||
}
|
||||
|
||||
class FakeDocument extends FakeElement {
|
||||
body: FakeElement
|
||||
documentElement: FakeElement
|
||||
|
||||
constructor() {
|
||||
super()
|
||||
this.ownerDocument = this
|
||||
this.body = new FakeElement()
|
||||
this.body.ownerDocument = this
|
||||
this.documentElement = new FakeElement()
|
||||
this.documentElement.ownerDocument = this
|
||||
}
|
||||
}
|
||||
|
||||
class FakeWheelEvent {
|
||||
constructor(
|
||||
readonly deltaY: number,
|
||||
@@ -72,13 +103,26 @@ class FakeWheelEvent {
|
||||
) {}
|
||||
}
|
||||
|
||||
class FakeKeyboardEvent {
|
||||
readonly defaultPrevented = false
|
||||
readonly shiftKey = false
|
||||
|
||||
constructor(
|
||||
readonly key: string,
|
||||
readonly target: FakeElement,
|
||||
) {}
|
||||
}
|
||||
|
||||
globalThis.Element = FakeElement as unknown as typeof Element
|
||||
globalThis.Node = FakeElement as unknown as typeof Node
|
||||
globalThis.WheelEvent = FakeWheelEvent as unknown as typeof WheelEvent
|
||||
|
||||
const { createAutoScroll } = await import("./create-auto-scroll")
|
||||
|
||||
function setup(options?: { interacted?: () => void; working?: boolean }) {
|
||||
function setup(options?: { doc?: FakeDocument; interacted?: () => void; working?: boolean }) {
|
||||
const doc = options?.doc ?? new FakeDocument()
|
||||
const el = new FakeElement()
|
||||
el.ownerDocument = doc
|
||||
const root = createRoot((dispose) => ({
|
||||
dispose,
|
||||
scroll: createAutoScroll({
|
||||
@@ -97,7 +141,7 @@ function setup(options?: { interacted?: () => void; working?: boolean }) {
|
||||
observers.forEach((callback) => callback())
|
||||
}
|
||||
|
||||
return { ...root, el, resize }
|
||||
return { ...root, doc, el, resize }
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -107,6 +151,8 @@ beforeEach(() => {
|
||||
afterAll(() => {
|
||||
if (originalElement) globalThis.Element = originalElement
|
||||
else Reflect.deleteProperty(globalThis, "Element")
|
||||
if (originalNode) globalThis.Node = originalNode
|
||||
else Reflect.deleteProperty(globalThis, "Node")
|
||||
if (originalWheelEvent) globalThis.WheelEvent = originalWheelEvent
|
||||
else Reflect.deleteProperty(globalThis, "WheelEvent")
|
||||
})
|
||||
@@ -236,6 +282,117 @@ describe("createAutoScroll non-scrollable layouts", () => {
|
||||
ctx.dispose()
|
||||
})
|
||||
|
||||
test("continues following after a programmatic scroll correction", () => {
|
||||
const ctx = setup({ working: true })
|
||||
ctx.el.scrollHeight = 1000
|
||||
ctx.el.clientHeight = 200
|
||||
ctx.el.scrollTop = 800
|
||||
ctx.scroll.handleScroll()
|
||||
|
||||
ctx.el.scrollTop = 760
|
||||
ctx.scroll.handleScroll()
|
||||
|
||||
expect(ctx.scroll.userScrolled()).toBe(false)
|
||||
|
||||
ctx.el.scrollHeight = 1100
|
||||
ctx.resize(0)
|
||||
|
||||
expect(ctx.scroll.userScrolled()).toBe(false)
|
||||
expect(ctx.el.scrollTop).toBe(1100)
|
||||
ctx.dispose()
|
||||
})
|
||||
|
||||
test("pauses for a body-targeted keyboard scroll over the transcript", () => {
|
||||
const ctx = setup({ working: true })
|
||||
ctx.el.scrollHeight = 1000
|
||||
ctx.el.clientHeight = 200
|
||||
ctx.el.scrollTop = 800
|
||||
ctx.el.hovered = true
|
||||
|
||||
ctx.doc.fire("keydown", new FakeKeyboardEvent("PageUp", ctx.doc.body) as unknown as Event)
|
||||
ctx.el.scrollTop = 600
|
||||
ctx.scroll.handleScroll()
|
||||
|
||||
expect(ctx.scroll.userScrolled()).toBe(true)
|
||||
|
||||
ctx.el.scrollHeight = 1100
|
||||
ctx.resize(0)
|
||||
|
||||
expect(ctx.el.scrollTop).toBe(600)
|
||||
ctx.dispose()
|
||||
})
|
||||
|
||||
test("routes body-targeted keyboard scroll to the deepest hovered container", () => {
|
||||
const doc = new FakeDocument()
|
||||
const outer = setup({ doc, working: true })
|
||||
const inner = setup({ doc, working: true })
|
||||
outer.el.append(inner.el)
|
||||
outer.el.scrollHeight = 1000
|
||||
outer.el.clientHeight = 200
|
||||
outer.el.scrollTop = 800
|
||||
inner.el.scrollHeight = 500
|
||||
inner.el.clientHeight = 100
|
||||
inner.el.scrollTop = 400
|
||||
outer.el.hovered = true
|
||||
inner.el.hovered = true
|
||||
|
||||
doc.fire("keydown", new FakeKeyboardEvent("PageUp", doc.body) as unknown as Event)
|
||||
inner.el.scrollTop = 300
|
||||
inner.scroll.handleScroll()
|
||||
|
||||
expect(inner.scroll.userScrolled()).toBe(true)
|
||||
expect(outer.scroll.userScrolled()).toBe(false)
|
||||
inner.dispose()
|
||||
outer.dispose()
|
||||
})
|
||||
|
||||
test("routes keyboard scroll past a nested container boundary", () => {
|
||||
const doc = new FakeDocument()
|
||||
const outer = setup({ doc, working: true })
|
||||
const inner = setup({ doc, working: true })
|
||||
outer.el.append(inner.el)
|
||||
outer.el.scrollHeight = 1000
|
||||
outer.el.clientHeight = 200
|
||||
outer.el.scrollTop = 800
|
||||
inner.el.scrollHeight = 500
|
||||
inner.el.clientHeight = 100
|
||||
inner.el.scrollTop = 0
|
||||
outer.el.hovered = true
|
||||
inner.el.hovered = true
|
||||
|
||||
doc.fire("keydown", new FakeKeyboardEvent("PageUp", doc.body) as unknown as Event)
|
||||
outer.el.scrollTop = 700
|
||||
outer.scroll.handleScroll()
|
||||
|
||||
expect(outer.scroll.userScrolled()).toBe(true)
|
||||
expect(inner.scroll.userScrolled()).toBe(false)
|
||||
inner.dispose()
|
||||
outer.dispose()
|
||||
})
|
||||
|
||||
test("removes disposed containers from keyboard ownership", () => {
|
||||
const doc = new FakeDocument()
|
||||
const outer = setup({ doc, working: true })
|
||||
const inner = setup({ doc, working: true })
|
||||
outer.el.append(inner.el)
|
||||
outer.el.scrollHeight = 1000
|
||||
outer.el.clientHeight = 200
|
||||
outer.el.scrollTop = 800
|
||||
inner.el.scrollHeight = 500
|
||||
inner.el.clientHeight = 100
|
||||
inner.el.scrollTop = 400
|
||||
outer.el.hovered = true
|
||||
inner.el.hovered = true
|
||||
inner.dispose()
|
||||
|
||||
doc.fire("keydown", new FakeKeyboardEvent("PageUp", doc.body) as unknown as Event)
|
||||
outer.el.scrollTop = 700
|
||||
outer.scroll.handleScroll()
|
||||
|
||||
expect(outer.scroll.userScrolled()).toBe(true)
|
||||
outer.dispose()
|
||||
})
|
||||
|
||||
test("follows when initially short content starts overflowing", () => {
|
||||
const ctx = setup()
|
||||
ctx.resize()
|
||||
@@ -296,13 +453,14 @@ describe("createAutoScroll non-scrollable layouts", () => {
|
||||
ctx.dispose()
|
||||
})
|
||||
|
||||
test("pauses when a native scrollbar drag changes scroll position without input events", () => {
|
||||
test("pauses after pointer input moves the scroll position", () => {
|
||||
const ctx = setup({ working: true })
|
||||
ctx.el.scrollHeight = 1000
|
||||
ctx.el.clientHeight = 200
|
||||
ctx.el.scrollTop = 800
|
||||
ctx.scroll.handleScroll()
|
||||
|
||||
ctx.el.fire("pointerdown", new Event("pointerdown"))
|
||||
ctx.el.scrollTop = 600
|
||||
ctx.scroll.handleScroll()
|
||||
|
||||
|
||||
@@ -25,8 +25,6 @@ export function createAutoScroll(options: AutoScrollOptions) {
|
||||
let settling = false
|
||||
let settleTimer: ReturnType<typeof setTimeout> | undefined
|
||||
let cleanup: (() => void) | undefined
|
||||
let lastTop = 0
|
||||
let lastHeight = 0
|
||||
|
||||
const [store, setStore] = createStore({
|
||||
contentRef: undefined as HTMLElement | undefined,
|
||||
@@ -103,9 +101,6 @@ export function createAutoScroll(options: AutoScrollOptions) {
|
||||
|
||||
const input = userActivity.consumeScroll()
|
||||
const distance = distanceFromBottom(scroll)
|
||||
const moved = Math.abs(scroll.scrollTop - lastTop) > 1 && Math.abs(scroll.scrollHeight - lastHeight) <= 1
|
||||
lastTop = scroll.scrollTop
|
||||
lastHeight = scroll.scrollHeight
|
||||
|
||||
if (!canScroll(scroll)) return
|
||||
|
||||
@@ -114,9 +109,9 @@ export function createAutoScroll(options: AutoScrollOptions) {
|
||||
return
|
||||
}
|
||||
|
||||
// Virtualizer and layout remeasurement can emit scroll before the
|
||||
// ResizeObserver restores bottom-follow. Only user input should pause it.
|
||||
if (!store.userScrolled && !input && !userActivity.isRecent() && !moved) return
|
||||
// Virtualizer and layout corrections can move the viewport without
|
||||
// changing content height. Only an input event should pause auto-follow.
|
||||
if (!store.userScrolled && !input && !userActivity.isRecent()) return
|
||||
|
||||
stop()
|
||||
}
|
||||
@@ -209,8 +204,6 @@ export function createAutoScroll(options: AutoScrollOptions) {
|
||||
|
||||
if (!el) return
|
||||
|
||||
lastTop = el.scrollTop
|
||||
lastHeight = el.scrollHeight
|
||||
updateOverflowAnchor(el)
|
||||
cleanup = userActivity.listen(el)
|
||||
}
|
||||
|
||||
@@ -3,6 +3,9 @@ interface UserActivityOptions {
|
||||
onWheelUp: () => void
|
||||
}
|
||||
|
||||
const SCROLL_KEYS = new Set(["ArrowDown", "ArrowUp", "End", "Home", "PageDown", "PageUp", " "])
|
||||
const owners = new WeakMap<Document, Set<HTMLElement>>()
|
||||
|
||||
const isPotentialScrollInput = (event: Event) => {
|
||||
if (!(event.target instanceof Element)) return true
|
||||
const editable = event.target.closest<HTMLElement>("[contenteditable]")
|
||||
@@ -31,20 +34,40 @@ export const createUserActivity = (options: UserActivityOptions) => {
|
||||
options.onWheelUp()
|
||||
}
|
||||
|
||||
const handleKey = (event: KeyboardEvent) => {
|
||||
if (!scroll || event.defaultPrevented || !SCROLL_KEYS.has(event.key)) return
|
||||
const target = event.target
|
||||
const root = target === scroll.ownerDocument.body || target === scroll.ownerDocument.documentElement
|
||||
const up = event.key === "ArrowUp" || event.key === "Home" || event.key === "PageUp" || (event.key === " " && event.shiftKey)
|
||||
const matches = [...(owners.get(scroll.ownerDocument) ?? [])].filter((el) => {
|
||||
const owns = root ? el.matches(":hover") : target instanceof Node && el.contains(target)
|
||||
if (!owns) return false
|
||||
return up ? el.scrollTop > 1 : el.scrollHeight - el.clientHeight - el.scrollTop > 1
|
||||
})
|
||||
const owner = matches.find((el) => !matches.some((candidate) => candidate !== el && el.contains(candidate)))
|
||||
if (owner !== scroll) return
|
||||
mark(event)
|
||||
}
|
||||
|
||||
return {
|
||||
listen: (el: HTMLElement) => {
|
||||
scroll = el
|
||||
const registered = owners.get(el.ownerDocument) ?? new Set<HTMLElement>()
|
||||
registered.add(el)
|
||||
owners.set(el.ownerDocument, registered)
|
||||
el.addEventListener("wheel", handleWheel, { passive: true, capture: true })
|
||||
el.addEventListener("pointerdown", mark, { passive: true })
|
||||
el.addEventListener("keydown", mark, { passive: true })
|
||||
el.addEventListener("touchstart", mark, { passive: true })
|
||||
el.ownerDocument.addEventListener("keydown", handleKey, { passive: true })
|
||||
|
||||
return () => {
|
||||
if (scroll === el) scroll = undefined
|
||||
registered.delete(el)
|
||||
if (registered.size === 0) owners.delete(el.ownerDocument)
|
||||
el.removeEventListener("wheel", handleWheel, { capture: true })
|
||||
el.removeEventListener("pointerdown", mark)
|
||||
el.removeEventListener("keydown", mark)
|
||||
el.removeEventListener("touchstart", mark)
|
||||
el.ownerDocument.removeEventListener("keydown", handleKey)
|
||||
}
|
||||
},
|
||||
consumeScroll: () => {
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
import { expect, test, type Page } from "@playwright/test"
|
||||
|
||||
const GLOBALS = "colorScheme:dark;theme:kilo-vscode;vscodeTheme:dark-modern"
|
||||
const STORY_ID = "chat--message-list-layout-correction"
|
||||
|
||||
async function settle(page: Page, frames = 2) {
|
||||
await page.evaluate(
|
||||
(count) =>
|
||||
new Promise<void>((resolve) => {
|
||||
const next = (left: number) => {
|
||||
if (left === 0) return resolve()
|
||||
requestAnimationFrame(() => next(left - 1))
|
||||
}
|
||||
next(count)
|
||||
}),
|
||||
frames,
|
||||
)
|
||||
}
|
||||
|
||||
async function distance(page: Page) {
|
||||
return page.locator(".message-list").evaluate((el) => el.scrollHeight - el.clientHeight - el.scrollTop)
|
||||
}
|
||||
|
||||
test("keeps following after a stable-height layout correction", async ({ page }) => {
|
||||
await page.goto(`/iframe.html?id=${STORY_ID}&viewMode=story&globals=${GLOBALS}`, { waitUntil: "load" })
|
||||
const list = page.locator(".message-list")
|
||||
await expect(list).toBeVisible()
|
||||
await settle(page, 10)
|
||||
await expect.poll(() => distance(page)).toBeLessThanOrEqual(2)
|
||||
|
||||
const before = await list.evaluate((el) => ({ height: el.scrollHeight, top: el.scrollTop }))
|
||||
const corrected = await list.evaluate((el) => {
|
||||
el.scrollTop -= 120
|
||||
return { height: el.scrollHeight, top: el.scrollTop }
|
||||
})
|
||||
await settle(page)
|
||||
|
||||
expect(corrected.height).toBe(before.height)
|
||||
expect(before.top - corrected.top).toBe(120)
|
||||
await expect(page.getByRole("button", { name: "Scroll to bottom" })).toBeHidden()
|
||||
|
||||
await page.getByTestId("append-stream").click()
|
||||
|
||||
await expect.poll(() => distance(page)).toBeLessThanOrEqual(2)
|
||||
await expect(page.getByRole("button", { name: "Scroll to bottom" })).toBeHidden()
|
||||
})
|
||||
|
||||
test("keeps the reading position when the prompt rail scrolls upward", async ({ page }) => {
|
||||
await page.goto(`/iframe.html?id=${STORY_ID}&viewMode=story&globals=${GLOBALS}`, { waitUntil: "load" })
|
||||
const list = page.locator(".message-list")
|
||||
await expect(list).toBeVisible()
|
||||
await settle(page, 10)
|
||||
await expect.poll(() => distance(page)).toBeLessThanOrEqual(2)
|
||||
|
||||
await page.locator(".prompt-rail").hover()
|
||||
await page.mouse.wheel(0, -240)
|
||||
|
||||
await expect.poll(() => distance(page)).toBeGreaterThan(40)
|
||||
await expect(page.getByRole("button", { name: "Scroll to bottom" })).toBeVisible()
|
||||
const top = await list.evaluate((el) => el.scrollTop)
|
||||
|
||||
await page.getByTestId("append-stream").click()
|
||||
|
||||
await expect.poll(() => list.evaluate((el) => el.scrollTop)).toBe(top)
|
||||
await expect(page.getByRole("button", { name: "Scroll to bottom" })).toBeVisible()
|
||||
})
|
||||
@@ -1383,7 +1383,9 @@ export const MessageList: Component<MessageListProps> = (props) => {
|
||||
onLoadOlder={() => session.loadOlderMessages()}
|
||||
onWheel={(deltaY: number) => {
|
||||
const el = scrollEl()
|
||||
if (el) el.scrollTop += deltaY
|
||||
if (!el) return
|
||||
if (deltaY < 0 && el.scrollHeight - el.clientHeight > 1) autoScroll.pause()
|
||||
el.scrollTop += deltaY
|
||||
}}
|
||||
height={height}
|
||||
hasOlder={session.hasOlderMessages}
|
||||
|
||||
@@ -699,6 +699,69 @@ export const PromptRailManyPrompts: Story = {
|
||||
},
|
||||
}
|
||||
|
||||
const correctionTurns = Array.from({ length: 30 }, (_, i) =>
|
||||
railTurn(300 + i, `Virtualized prompt ${i + 1}`, `Virtualized answer ${i + 1}.`),
|
||||
)
|
||||
const correctionActive = railTurn(400, "Continue streaming", "Initial streamed response.")
|
||||
const correctionMessages = [...correctionTurns.flatMap((turn) => turn.messages), ...correctionActive.messages]
|
||||
const correctionAssistant = correctionActive.messages[1]!
|
||||
correctionAssistant.finish = "tool-calls"
|
||||
const correctionParts = Object.assign(
|
||||
{},
|
||||
...correctionTurns.map((turn) => turn.parts),
|
||||
correctionActive.parts,
|
||||
) as Record<string, any[]>
|
||||
const correctionData = {
|
||||
...defaultMockData,
|
||||
message: { [SESSION_ID]: correctionMessages },
|
||||
part: correctionParts,
|
||||
}
|
||||
|
||||
export const MessageListLayoutCorrection: Story = {
|
||||
name: "MessageList - follow after layout correction",
|
||||
render: () => {
|
||||
const [output, setOutput] = createSignal("Initial streamed response.")
|
||||
const session = {
|
||||
...mockSessionValue({ id: SESSION_ID, status: "busy" }),
|
||||
messages: () => correctionMessages,
|
||||
userMessages: () => correctionMessages.filter((msg) => msg.role === "user"),
|
||||
getParts: (id: string) => {
|
||||
if (id !== correctionAssistant.id) return correctionParts[id] ?? []
|
||||
const part = correctionParts[id]![0]!
|
||||
return [{ ...part, text: output() }]
|
||||
},
|
||||
}
|
||||
return (
|
||||
<StoryProviders data={correctionData} sessionID={SESSION_ID} status="busy" noPadding>
|
||||
<SessionContext.Provider value={session as any}>
|
||||
<div
|
||||
class="auto-scroll-correction-fixture"
|
||||
style={{ height: "100vh", display: "flex", "flex-direction": "column" }}
|
||||
>
|
||||
<style>{`
|
||||
.auto-scroll-correction-controls {
|
||||
position: fixed;
|
||||
inset: 8px 8px auto auto;
|
||||
z-index: 10;
|
||||
}
|
||||
`}</style>
|
||||
<div class="auto-scroll-correction-controls">
|
||||
<button
|
||||
type="button"
|
||||
data-testid="append-stream"
|
||||
onClick={() => setOutput((value) => `${value}\n\n${"More streamed output. ".repeat(30)}`)}
|
||||
>
|
||||
Append stream
|
||||
</button>
|
||||
</div>
|
||||
<ChatView />
|
||||
</div>
|
||||
</SessionContext.Provider>
|
||||
</StoryProviders>
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
export const MessageListToolToQueuedUserSpacing: Story = {
|
||||
name: "MessageList — queued users stay at bottom",
|
||||
render: () => {
|
||||
|
||||
Reference in New Issue
Block a user