fix(web): preserve sidebar scroll position across task switches (#884)

apps/web/components/task/task-session-sidebar.tsx

@@ -570,7 +570,7 @@ export const TaskSessionSidebar = memo(function TaskSessionSidebar({
   return (
     <PanelRoot data-testid="task-sidebar">
       <SidebarFilterBar />
-      <PanelBody className="space-y-4 p-0">
+      <PanelBody className="space-y-4 p-0" data-testid="task-sidebar-scroll">
         <TaskSwitcher
           grouped={grouped}
           workflows={workflows}

apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts

@@ -0,0 +1,83 @@
+import { test, expect } from "../../fixtures/test-base";
+import { SessionPage } from "../../pages/session-page";
+
+/**
+ * Regression: when clicking another task in the sidebar, the sidebar's
+ * scroll position would jump back to the top. Dockview rebuilds panel slots
+ * on env switch, which detaches and re-attaches the sidebar's portal element.
+ * Browsers reset scrollTop on DOM detach, so the sidebar lost its scroll
+ * position. usePortalSlot now snapshots scroll positions inside each portal
+ * via a capturing scroll listener and restores them after every reattach.
+ */
+test.describe("sidebar scroll preservation across task switch", () => {
+  test("clicking another task does not reset the sidebar scroll position", async ({
+    testPage,
+    apiClient,
+    seedData,
+  }) => {
+    test.setTimeout(60_000);
+
+    // Seed enough tasks so the sidebar list overflows its viewport. Titles
+    // are zero-padded so sort order matches creation order.
+    const TASK_COUNT = 25;
+    const created: { id: string; title: string }[] = [];
+    for (let i = 0; i < TASK_COUNT; i++) {
+      const title = `Scroll Task ${String(i).padStart(2, "0")}`;
+      const task = await apiClient.createTask(seedData.workspaceId, title, {
+        workflow_id: seedData.workflowId,
+        workflow_step_id: seedData.startStepId,
+        repository_ids: [seedData.repositoryId],
+      });
+      created.push({ id: task.id, title });
+    }
+
+    // Navigate to the most recently created task (renders at the top of the
+    // sidebar since tasks sort by createdAt desc within a state bucket).
+    const navTask = created[created.length - 1];
+    await testPage.goto(`/t/${navTask.id}`);
+    const session = new SessionPage(testPage);
+    await session.waitForLoad();
+    await expect(session.sidebar).toBeVisible({ timeout: 10_000 });
+
+    // Wait for the full task list to render in the sidebar.
+    await expect(session.sidebar.getByTestId("sidebar-task-item")).toHaveCount(TASK_COUNT, {
+      timeout: 10_000,
+    });
+
+    const scrollContainer = testPage.getByTestId("task-sidebar-scroll");
+    await expect(scrollContainer).toBeVisible();
+
+    // Sanity: list overflows the scroll container.
+    const dimensions = await scrollContainer.evaluate((el) => ({
+      clientHeight: el.clientHeight,
+      scrollHeight: el.scrollHeight,
+    }));
+    expect(dimensions.scrollHeight).toBeGreaterThan(dimensions.clientHeight);
+
+    // Scroll to the bottom of the sidebar.
+    await scrollContainer.evaluate((el) => {
+      el.scrollTop = el.scrollHeight - el.clientHeight;
+    });
+    const scrollBefore = await scrollContainer.evaluate((el) => el.scrollTop);
+    expect(scrollBefore).toBeGreaterThan(0);
+
+    // Pick a task that is currently visible near the bottom (the oldest one).
+    // The first-created task appears last in the list because of desc sort.
+    const bottomTask = created[0];
+    const bottomRow = session.sidebarTaskItem(bottomTask.title).first();
+    await expect(bottomRow).toBeVisible();
+    await expect(bottomRow).toBeInViewport();
+
+    await bottomRow.click();
+
+    // URL switches to the new task once selection settles.
+    await expect.poll(() => testPage.url(), { timeout: 10_000 }).toContain(bottomTask.id);
+
+    // Sidebar should still be scrolled near the previous position. Without
+    // the fix, scrollTop would snap back to 0 after the dockview slot is
+    // re-created. Allow a small tolerance for any minor adjustments.
+    await expect
+      .poll(() => scrollContainer.evaluate((el) => el.scrollTop), { timeout: 5_000 })
+      .toBeGreaterThan(scrollBefore - 50);
+  });
+});

apps/web/lib/layout/panel-portal-host.tsx

@@ -85,10 +85,29 @@ export function usePortalSlot(
 
     const entry = panelPortalManager.acquire(panelId, component, params, props.api, envId);
 
-    // Reparent the portal element into this panel's DOM slot.
+    // Reparent the portal element into this panel's DOM slot, then restore any
+    // scroll positions captured before the previous detach. Browsers reset
+    // scrollTop/scrollLeft when an element is removed from the DOM, so without
+    // this the sidebar (and any other scrollable descendant of a portal) jumps
+    // back to the top after every dockview layout switch.
     container.appendChild(entry.element);
+    const cancelRestore = restorePortalScroll(entry.element);
+    // Track every scroll inside this portal so we always have an up-to-date
+    // snapshot to restore after the next reattach. We do this via a capturing
+    // listener because dockview can rip the DOM apart before React runs the
+    // cleanup below (removeChild fires too late to read scrollTop reliably).
+    const onScroll = (e: Event) => {
+      const target = e.target;
+      if (target instanceof Element && entry.element.contains(target)) {
+        const html = target as HTMLElement;
+        scrollSnapshots.set(target, { top: html.scrollTop, left: html.scrollLeft });
+      }
+    };
+    entry.element.addEventListener("scroll", onScroll, true);
 
     return () => {
+      cancelRestore();
+      entry.element.removeEventListener("scroll", onScroll, true);
       // Detach only — do NOT release.  The element stays in the manager
       // and will be re-adopted when the panel remounts after fromJSON().
       if (entry.element.parentNode === container) {
@@ -113,3 +132,96 @@ export function usePortalSlot(
 
   return containerRef;
 }
+
+// ---------------------------------------------------------------------------
+// Scroll preservation across portal detach/reattach
+// ---------------------------------------------------------------------------
+
+/**
+ * Per-element scroll snapshots, kept up-to-date by a capturing scroll
+ * listener attached to each portal element. Stored in a WeakMap so detached
+ * subtrees can be garbage collected if the portal is ever released.
+ */
+const scrollSnapshots = new WeakMap<Element, { top: number; left: number }>();
+
+/** Window during which we keep re-applying snapshots after attach. */
+const RESTORE_WINDOW_MS = 1500;
+
+/**
+ * Restore captured scroll positions on every scrollable descendant of `portal`,
+ * then keep re-applying for a short window so that a snapshot which gets
+ * clamped by a not-yet-laid-out container (`scrollTop` capped to
+ * `scrollHeight - clientHeight`) is corrected once layout settles.
+ *
+ * Returns a function that cancels any pending retries (call from cleanup so
+ * we never write to a detached subtree).
+ */
+function restorePortalScroll(portal: Element): () => void {
+  // Collect every descendant that has a snapshot up front so we don't re-walk
+  // the tree on each retry.
+  const targets: Array<{ el: HTMLElement; snap: { top: number; left: number } }> = [];
+  const walker = document.createTreeWalker(portal, NodeFilter.SHOW_ELEMENT);
+  // Skip the portal element itself — it is never user-scrolled, and the loop
+  // body assumes Element nodes (guaranteed by SHOW_ELEMENT for descendants).
+  let node = walker.nextNode() as HTMLElement | null;
+  while (node) {
+    const snap = scrollSnapshots.get(node);
+    if (snap && (snap.top > 0 || snap.left > 0)) {
+      targets.push({ el: node, snap });
+    }
+    node = walker.nextNode() as HTMLElement | null;
+  }
+  if (targets.length === 0) return () => {};
+
+  // Track which elements still need retrying. An element is removed from
+  // `pending` once the layout has grown enough to accommodate the snapshot,
+  // so subsequent ResizeObserver/rAF callbacks won't fight a deliberate user
+  // scroll (including scrolling upward, away from the snapshot position).
+  const pending = new Set(targets.map((t) => t.el));
+  let cancelled = false;
+  let stopId = 0;
+  let ro: ResizeObserver | null = null;
+
+  const stop = () => {
+    cancelled = true;
+    ro?.disconnect();
+    window.clearTimeout(stopId);
+  };
+
+  const apply = () => {
+    for (const { el, snap } of targets) {
+      if (!pending.has(el)) continue;
+      if (el.scrollTop < snap.top) el.scrollTop = snap.top;
+      if (el.scrollLeft < snap.left) el.scrollLeft = snap.left;
+      // Once layout can fully accommodate the snapshot, the clamping bug is
+      // resolved — drop the element from pending so future ticks stay out of
+      // the way of any user-initiated scroll.
+      const canReachTop = el.scrollHeight - el.clientHeight >= snap.top;
+      const canReachLeft = el.scrollWidth - el.clientWidth >= snap.left;
+      if (canReachTop && canReachLeft) pending.delete(el);
+    }
+    if (pending.size === 0) stop();
+  };
+  apply();
+
+  if (!cancelled) {
+    ro = new ResizeObserver(() => {
+      if (cancelled) return;
+      apply();
+    });
+    for (const { el } of targets) ro.observe(el);
+    // A short rAF chain catches the common case where layout settles within
+    // the next two frames without a measurable size change.
+    requestAnimationFrame(() => {
+      if (cancelled) return;
+      apply();
+      requestAnimationFrame(() => {
+        if (cancelled) return;
+        apply();
+      });
+    });
+    stopId = window.setTimeout(stop, RESTORE_WINDOW_MS);
+  }
+
+  return stop;
+}