diff --git a/.changeset/session-list-scroll-reveal.md b/.changeset/session-list-scroll-reveal.md new file mode 100644 index 0000000..bcdc9ac --- /dev/null +++ b/.changeset/session-list-scroll-reveal.md @@ -0,0 +1,5 @@ +--- +"@jmfederico/pi-web": patch +--- + +Stop the session and workspace lists from re-scrolling to the selected row on live data refreshes, such as message-count updates while a session streams or workspace topology refreshes. The lists now scroll the selection into view only when the selection moves to a different row, an archived session is revealed, a restored session moves back to the current section, or a collapsed section expands. diff --git a/src/client/src/components/SessionList.scroll.test.ts b/src/client/src/components/SessionList.scroll.test.ts new file mode 100644 index 0000000..ea4973b --- /dev/null +++ b/src/client/src/components/SessionList.scroll.test.ts @@ -0,0 +1,143 @@ +// @vitest-environment happy-dom + +import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from "vitest"; +import type { SessionInfo } from "../api"; +import { SessionList } from "./SessionList"; + +let scrollIntoView: MockInstance; + +beforeEach(() => { + scrollIntoView = vi.spyOn(Element.prototype, "scrollIntoView"); +}); + +afterEach(() => { + vi.restoreAllMocks(); + document.body.replaceChildren(); +}); + +describe("SessionList selection reveal scrolling", () => { + it("scrolls the selected row into view on the first render that has a selection", async () => { + await renderSessionList({ sessions: [session("a"), session("b")], selected: session("a") }); + + expect(scrollIntoView).toHaveBeenCalledOnce(); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest" }); + }); + + it("scrolls when the selection changes to a different row", async () => { + const a = session("a"); + const b = session("b"); + const list = await renderSessionList({ sessions: [a, b], selected: a }); + scrollIntoView.mockClear(); + + list.selected = b; + await settled(list); + + expect(scrollIntoView).toHaveBeenCalledOnce(); + }); + + it("does not scroll when a status refresh bumps the selected session's messageCount", async () => { + // sessionMessageCountPatch replaces the sessions array and the selected + // object with same-id copies on every messageCount change. + const a = session("a"); + const b = session("b"); + const list = await renderSessionList({ sessions: [a, b], selected: a }); + scrollIntoView.mockClear(); + + const bumped = { ...a, messageCount: a.messageCount + 1 }; + list.sessions = [bumped, b]; + list.selected = bumped; + await settled(list); + + expect(scrollIntoView).not.toHaveBeenCalled(); + expect(list.shadowRoot?.querySelector(".action-row.selected")).not.toBeNull(); + }); + + it("does not scroll when the sessions array is replaced while the selection object stays put", async () => { + const a = session("a"); + const b = session("b"); + const list = await renderSessionList({ sessions: [a, b], selected: a }); + scrollIntoView.mockClear(); + + list.sessions = [{ ...a }, { ...b }]; + await settled(list); + + expect(scrollIntoView).not.toHaveBeenCalled(); + expect(list.shadowRoot?.querySelector(".action-row.selected")).not.toBeNull(); + }); + + it("expands the archived section and reveals the row when an archived session becomes selected", async () => { + const a = session("a"); + const archived = session("archived", { archived: true, archivedAt: "2026-06-09T00:00:00.000Z" }); + const list = await renderSessionList({ sessions: [a], selected: a }); + scrollIntoView.mockClear(); + + list.sessions = [a, archived]; + list.selected = archived; + await settled(list); + + expect(scrollIntoView).toHaveBeenCalledOnce(); + expect(list.shadowRoot?.querySelector(".action-row.selected.archived")).not.toBeNull(); + }); + + it("reveals the selected row when a restore moves it back to the current section", async () => { + const archived = session("a", { archived: true, archivedAt: "2026-06-09T00:00:00.000Z" }); + const b = session("b"); + const list = await renderSessionList({ sessions: [archived, b], selected: archived }); + // The archived reveal path already expanded the section and scrolled. + expect(list.shadowRoot?.querySelector(".action-row.selected.archived")).not.toBeNull(); + scrollIntoView.mockClear(); + + const restored = session("a"); + list.sessions = [restored, b]; + list.selected = restored; + await settled(list); + + expect(scrollIntoView).toHaveBeenCalledOnce(); + expect(list.shadowRoot?.querySelector(".action-row.selected:not(.archived)")).not.toBeNull(); + }); + + it("scrolls when the section expands, and not when it collapses", async () => { + const a = session("a"); + const list = await renderSessionList({ sessions: [a], selected: a, collapsed: true }); + expect(scrollIntoView).not.toHaveBeenCalled(); + + list.collapsed = false; + await settled(list); + expect(scrollIntoView).toHaveBeenCalledOnce(); + + scrollIntoView.mockClear(); + list.collapsed = true; + await settled(list); + expect(scrollIntoView).not.toHaveBeenCalled(); + }); +}); + +async function renderSessionList(options: { sessions: SessionInfo[]; selected?: SessionInfo; collapsed?: boolean }): Promise { + const list = new SessionList(); + list.sessions = options.sessions; + if (options.selected !== undefined) list.selected = options.selected; + list.collapsed = options.collapsed ?? false; + document.body.append(list); + await settled(list); + return list; +} + +async function settled(list: SessionList): Promise { + // Selecting an archived session schedules a follow-up render (archived + // auto-expansion) and chains its scroll on updateComplete; await both cycles. + await list.updateComplete; + await list.updateComplete; +} + +function session(id: string, overrides: Partial = {}): SessionInfo { + return { + id, + path: `/sessions/${id}.jsonl`, + cwd: "/workspace", + created: "2026-06-09T00:00:00.000Z", + modified: "2026-06-09T00:00:00.000Z", + messageCount: 1, + firstMessage: id, + ...overrides, + }; +} diff --git a/src/client/src/components/SessionList.ts b/src/client/src/components/SessionList.ts index d126260..d3d0d9a 100644 --- a/src/client/src/components/SessionList.ts +++ b/src/client/src/components/SessionList.ts @@ -94,7 +94,24 @@ export class SessionList extends LitElement implements KeyboardNavigableSection void this.updateComplete.then(() => { this.scrollSelectedIntoView(); }); return; } - if ((changed.has("selected") || changed.has("sessions") || changed.has("collapsed")) && !this.collapsed) this.scrollSelectedIntoView(); + if (this.shouldRevealSelectedRow(changed)) this.scrollSelectedIntoView(); + } + + /** + * Positive reveal triggers only: live data refreshes replace `sessions` and + * `selected` with same-id objects (status churn, renames, archive flips) and + * must never re-scroll. Reveal the selected row only when the selection + * moves to a different row (first render with a selection included), when a + * restore moves it from the archived section back to the current section + * (same id, archived flag cleared), or when the section expands. + */ + private shouldRevealSelectedRow(changed: PropertyValues): boolean { + if (this.collapsed) return false; + if (changed.has("collapsed")) return true; + if (!changed.has("selected")) return false; + const previousSelected = changed.get("selected"); + if (previousSelected?.id !== this.selected?.id) return true; + return previousSelected?.archived === true && this.selected?.archived !== true; } async focusSelectedOrFirst(): Promise { diff --git a/src/client/src/components/WorkspaceList.scroll.test.ts b/src/client/src/components/WorkspaceList.scroll.test.ts new file mode 100644 index 0000000..290407e --- /dev/null +++ b/src/client/src/components/WorkspaceList.scroll.test.ts @@ -0,0 +1,98 @@ +// @vitest-environment happy-dom + +import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from "vitest"; +import type { Workspace } from "../api"; +import { WorkspaceList } from "./WorkspaceList"; + +let scrollIntoView: MockInstance; + +beforeEach(() => { + scrollIntoView = vi.spyOn(Element.prototype, "scrollIntoView"); +}); + +afterEach(() => { + vi.restoreAllMocks(); + document.body.replaceChildren(); +}); + +describe("WorkspaceList selection reveal scrolling", () => { + it("scrolls the selected row into view on the first render that has a selection", async () => { + await renderWorkspaceList({ workspaces: [workspace("a"), workspace("b")], selected: workspace("a") }); + + expect(scrollIntoView).toHaveBeenCalledOnce(); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest" }); + }); + + it("scrolls when the selection changes to a different row", async () => { + const a = workspace("a"); + const b = workspace("b"); + const list = await renderWorkspaceList({ workspaces: [a, b], selected: a }); + scrollIntoView.mockClear(); + + list.selected = b; + await settled(list); + + expect(scrollIntoView).toHaveBeenCalledOnce(); + }); + + it("does not scroll when a topology refresh replaces the workspaces array for the same selection", async () => { + // Browser-resume topology refreshes swap in a new workspaces array, and the + // selected workspace may be swapped for a same-id object with it. + const a = workspace("a"); + const b = workspace("b"); + const list = await renderWorkspaceList({ workspaces: [a, b], selected: a }); + scrollIntoView.mockClear(); + + list.workspaces = [{ ...a }, { ...b }]; + list.selected = { ...a }; + await settled(list); + + expect(scrollIntoView).not.toHaveBeenCalled(); + expect(list.shadowRoot?.querySelector(".action-row.selected")).not.toBeNull(); + }); + + it("scrolls when the section expands, and not when it collapses", async () => { + const a = workspace("a"); + const list = await renderWorkspaceList({ workspaces: [a], selected: a, collapsed: true }); + expect(scrollIntoView).not.toHaveBeenCalled(); + + list.collapsed = false; + await settled(list); + expect(scrollIntoView).toHaveBeenCalledOnce(); + + scrollIntoView.mockClear(); + list.collapsed = true; + await settled(list); + expect(scrollIntoView).not.toHaveBeenCalled(); + }); +}); + +async function renderWorkspaceList(options: { workspaces: Workspace[]; selected?: Workspace; collapsed?: boolean }): Promise { + const list = new WorkspaceList(); + list.workspaces = options.workspaces; + if (options.selected !== undefined) list.selected = options.selected; + list.collapsed = options.collapsed ?? false; + document.body.append(list); + await settled(list); + return list; +} + +async function settled(list: WorkspaceList): Promise { + // Mirror of SessionList's settled(): await two update cycles so any render + // scheduled from within updated() has completed before asserting. + await list.updateComplete; + await list.updateComplete; +} + +function workspace(id: string, overrides: Partial = {}): Workspace { + return { + id, + projectId: "project-1", + path: `/workspaces/${id}`, + label: id, + isMain: false, + isGitRepo: true, + isGitWorktree: true, + ...overrides, + }; +} diff --git a/src/client/src/components/WorkspaceList.ts b/src/client/src/components/WorkspaceList.ts index d901bf9..89a9bc9 100644 --- a/src/client/src/components/WorkspaceList.ts +++ b/src/client/src/components/WorkspaceList.ts @@ -46,7 +46,19 @@ export class WorkspaceList extends LitElement implements KeyboardNavigableSectio protected override updated(changed: PropertyValues): void { if (changed.has("workspaces") && this.openMenuWorkspaceId !== undefined && !this.workspaces.some((workspace) => workspace.id === this.openMenuWorkspaceId)) this.openMenuWorkspaceId = undefined; if (changed.has("collapsed") && this.collapsed) this.openMenuWorkspaceId = undefined; - if ((changed.has("selected") || changed.has("workspaces") || changed.has("collapsed")) && !this.collapsed) this.scrollSelectedIntoView(); + if (this.shouldRevealSelectedRow(changed)) this.scrollSelectedIntoView(); + } + + /** + * Positive reveal triggers only: topology refreshes replace `workspaces` + * with a new array for the same selection and must never re-scroll. Reveal + * the selected row only when the selection moves to a different row (first + * render with a selection included) or when the section expands. + */ + private shouldRevealSelectedRow(changed: PropertyValues): boolean { + if (this.collapsed) return false; + if (changed.has("collapsed")) return true; + return changed.has("selected") && changed.get("selected")?.id !== this.selected?.id; } async focusSelectedOrFirst(): Promise {