fix(desktop): preserve tab scroll position across Activity visibility cycles (MUL-2602) (#3196)

Closes #3183.

Tabs render under `<Activity mode="visible|hidden">`, which keeps React
state but drops DOM scrollTop when the subtree leaves layout. Switching
to another tab and back sent users to the top of long discussions.

`useTabScrollRestore` records the scrollTop of every element marked with
`data-tab-scroll-root` while the tab is visible (capture-phase scroll
listener) and restores them in a useLayoutEffect on the next visible
transition, before paint. Saved offsets are dropped when the tab's path
changes so intra-tab navigation lands at scroll=0 instead of inheriting
the previous route's position.

Mark scroll containers in views with `data-tab-scroll-root` (issue
detail + chat message list ship with the marker; other views can adopt
the convention as needed).

`useAutoScroll` previously called `scrollToBottom()` on every effect
mount, which would have overwritten the restored offset every time a
chat tab cycled back to visible. Guard it with a once-per-instance ref.

Co-authored-by: multica-agent <github@multica.ai>

apps/desktop/src/renderer/src/components/tab-content.tsx

@@ -3,6 +3,7 @@ import { RouterProvider } from "react-router-dom";
 import { useActiveGroup } from "@/stores/tab-store";
 import { TabNavigationProvider } from "@/platform/navigation";
 import { useTabRouterSync } from "@/hooks/use-tab-router-sync";
+import { useTabScrollRestore } from "@/hooks/use-tab-scroll-restore";
 import type { Tab } from "@/stores/tab-store";
 
 /**
@@ -15,6 +16,28 @@ function TabRouterInner({ tab }: { tab: Tab }) {
   return null;
 }
 
+/**
+ * Wraps a tab's subtree so its scroll position survives the round trip
+ * through `<Activity mode="hidden">`. Lives inside Activity so the hook's
+ * effects cycle with the tab's visibility — see `useTabScrollRestore` for
+ * the mechanism. `display: contents` keeps the wrapper transparent to
+ * the surrounding flex layout.
+ */
+function TabScrollRestoreWrapper({
+  tabPath,
+  children,
+}: {
+  tabPath: string;
+  children: React.ReactNode;
+}) {
+  const ref = useTabScrollRestore(tabPath);
+  return (
+    <div ref={ref} style={{ display: "contents" }}>
+      {children}
+    </div>
+  );
+}
+
 /**
  * Renders the active workspace's tabs using Activity for state preservation.
  * Only the active tab is visible; hidden tabs keep their DOM and React state.
@@ -44,10 +67,12 @@ export function TabContent() {
           key={tab.id}
           mode={tab.id === group.activeTabId ? "visible" : "hidden"}
         >
-          <TabNavigationProvider router={tab.router}>
-            <RouterProvider router={tab.router} />
-            <TabRouterInner tab={tab} />
-          </TabNavigationProvider>
+          <TabScrollRestoreWrapper tabPath={tab.path}>
+            <TabNavigationProvider router={tab.router}>
+              <RouterProvider router={tab.router} />
+              <TabRouterInner tab={tab} />
+            </TabNavigationProvider>
+          </TabScrollRestoreWrapper>
         </Activity>
       ))}
     </>

apps/desktop/src/renderer/src/hooks/use-tab-scroll-restore.test.tsx

@@ -0,0 +1,116 @@
+import { Activity } from "react";
+import { describe, expect, it } from "vitest";
+import { fireEvent, render } from "@testing-library/react";
+import { useTabScrollRestore } from "./use-tab-scroll-restore";
+
+function Harness({ path }: { path: string }) {
+  const ref = useTabScrollRestore(path);
+  return (
+    <div ref={ref} style={{ display: "contents" }}>
+      <div
+        data-tab-scroll-root
+        data-testid="scroller"
+        style={{ height: 100, overflow: "auto" }}
+      >
+        <div style={{ height: 1000 }} />
+      </div>
+      <div
+        data-tab-scroll-root="aside"
+        data-testid="aside"
+        style={{ height: 100, overflow: "auto" }}
+      >
+        <div style={{ height: 1000 }} />
+      </div>
+      <div
+        data-testid="unmarked"
+        style={{ height: 100, overflow: "auto" }}
+      >
+        <div style={{ height: 1000 }} />
+      </div>
+    </div>
+  );
+}
+
+function App({ visible, path }: { visible: boolean; path: string }) {
+  return (
+    <Activity mode={visible ? "visible" : "hidden"}>
+      <Harness path={path} />
+    </Activity>
+  );
+}
+
+function setScroll(el: HTMLElement, top: number) {
+  el.scrollTop = top;
+  fireEvent.scroll(el);
+}
+
+describe("useTabScrollRestore", () => {
+  it("restores scroll position when a tab cycles through hidden -> visible", () => {
+    const { rerender, getByTestId } = render(
+      <App visible={true} path="/acme/issues/1" />,
+    );
+    const scroller = getByTestId("scroller") as HTMLElement;
+
+    setScroll(scroller, 500);
+    expect(scroller.scrollTop).toBe(500);
+
+    // Simulate Activity hiding the subtree: layout would drop the offset.
+    rerender(<App visible={false} path="/acme/issues/1" />);
+    scroller.scrollTop = 0;
+
+    rerender(<App visible={true} path="/acme/issues/1" />);
+    expect(scroller.scrollTop).toBe(500);
+  });
+
+  it("restores multiple named scroll roots independently", () => {
+    const { rerender, getByTestId } = render(
+      <App visible={true} path="/acme/issues/1" />,
+    );
+    const main = getByTestId("scroller") as HTMLElement;
+    const aside = getByTestId("aside") as HTMLElement;
+
+    setScroll(main, 300);
+    setScroll(aside, 150);
+
+    rerender(<App visible={false} path="/acme/issues/1" />);
+    main.scrollTop = 0;
+    aside.scrollTop = 0;
+
+    rerender(<App visible={true} path="/acme/issues/1" />);
+    expect(main.scrollTop).toBe(300);
+    expect(aside.scrollTop).toBe(150);
+  });
+
+  it("ignores scroll on elements without the data-tab-scroll-root marker", () => {
+    const { rerender, getByTestId } = render(
+      <App visible={true} path="/acme/issues/1" />,
+    );
+    const unmarked = getByTestId("unmarked") as HTMLElement;
+
+    setScroll(unmarked, 250);
+
+    rerender(<App visible={false} path="/acme/issues/1" />);
+    unmarked.scrollTop = 0;
+
+    rerender(<App visible={true} path="/acme/issues/1" />);
+    expect(unmarked.scrollTop).toBe(0);
+  });
+
+  it("drops saved offsets when the tab path changes (intra-tab navigation)", () => {
+    const { rerender, getByTestId } = render(
+      <App visible={true} path="/acme/issues/1" />,
+    );
+    const scroller = getByTestId("scroller") as HTMLElement;
+
+    setScroll(scroller, 500);
+
+    // Navigating within the tab swaps the active route — same marker key,
+    // different page. We should NOT restore the prior page's offset.
+    rerender(<App visible={true} path="/acme/issues/2" />);
+    scroller.scrollTop = 0;
+
+    rerender(<App visible={false} path="/acme/issues/2" />);
+    rerender(<App visible={true} path="/acme/issues/2" />);
+    expect(scroller.scrollTop).toBe(0);
+  });
+});

apps/desktop/src/renderer/src/hooks/use-tab-scroll-restore.ts

@@ -0,0 +1,70 @@
+import { useEffect, useLayoutEffect, useRef } from "react";
+
+/**
+ * Persist a tab's scroll positions across <Activity> visibility transitions.
+ *
+ * Tabs render under `<Activity mode="visible|hidden">`, which keeps React
+ * state but loses DOM scrollTop — the subtree is taken out of layout while
+ * hidden and rejoins with scrollTop=0. This hook records every marked
+ * container's `scrollTop` while the tab is visible (continuously, via a
+ * capture-phase scroll listener) and restores them in a `useLayoutEffect`
+ * the next time the tab becomes visible, before the browser paints.
+ *
+ * Mark scroll containers in views with `data-tab-scroll-root`. The
+ * attribute value is the cache key — defaults to `"main"` for unnamed
+ * roots. Most pages have a single scroll container, so a bare attribute
+ * is enough; named keys are only needed when a page has multiple
+ * independently scrollable regions whose positions must all be restored.
+ *
+ * When the tab's path changes (intra-tab navigation), the saved offsets
+ * are dropped — the new route's container shares the same marker key but
+ * is a different page, and restoring the old offset would land the user
+ * somewhere arbitrary on the new page.
+ */
+export function useTabScrollRestore(tabPath: string) {
+  const containerRef = useRef<HTMLDivElement>(null);
+  const savedRef = useRef<Map<string, number>>(new Map());
+  const prevPathRef = useRef(tabPath);
+
+  if (prevPathRef.current !== tabPath) {
+    savedRef.current.clear();
+    prevPathRef.current = tabPath;
+  }
+
+  // <Activity> cleans up effects on hidden and re-mounts them on visible,
+  // so an empty-deps useLayoutEffect runs exactly on every hidden → visible
+  // transition. Restoring here (before the browser paints) avoids any
+  // flash at scrollTop=0.
+  useLayoutEffect(() => {
+    const root = containerRef.current;
+    if (!root) return;
+    const els = root.querySelectorAll<HTMLElement>("[data-tab-scroll-root]");
+    els.forEach((el) => {
+      const key = scrollKey(el);
+      const saved = savedRef.current.get(key);
+      if (saved !== undefined && el.scrollTop !== saved) {
+        el.scrollTop = saved;
+      }
+    });
+  }, []);
+
+  useEffect(() => {
+    const root = containerRef.current;
+    if (!root) return;
+    const onScroll = (e: Event) => {
+      const target = e.target;
+      if (!(target instanceof HTMLElement)) return;
+      if (!target.hasAttribute("data-tab-scroll-root")) return;
+      savedRef.current.set(scrollKey(target), target.scrollTop);
+    };
+    // Scroll events don't bubble, but capture catches them anyway.
+    root.addEventListener("scroll", onScroll, { capture: true, passive: true });
+    return () => root.removeEventListener("scroll", onScroll, true);
+  }, []);
+
+  return containerRef;
+}
+
+function scrollKey(el: HTMLElement): string {
+  return el.getAttribute("data-tab-scroll-root") || "main";
+}

packages/ui/hooks/use-auto-scroll.ts

@@ -10,6 +10,11 @@ import { type RefObject, useEffect, useRef, useCallback } from "react"
 export function useAutoScroll(ref: RefObject<HTMLElement | null>) {
   const stickRef = useRef(true)
   const lockRef = useRef(false)
+  // Re-running the initial scroll-to-bottom on every effect mount would
+  // overwrite the scroll position any time React tears the effect down and
+  // brings it back — e.g. when the host tab cycles through `<Activity
+  // mode="hidden">` and back. We want the jump only on a real first mount.
+  const didInitialScrollRef = useRef(false)
 
   useEffect(() => {
     const el = ref.current
@@ -53,8 +58,10 @@ export function useAutoScroll(ref: RefObject<HTMLElement | null>) {
 
     el.addEventListener("scroll", onScroll, { passive: true })
 
-    // Initial scroll to bottom
-    scrollToBottom()
+    if (!didInitialScrollRef.current) {
+      didInitialScrollRef.current = true
+      scrollToBottom()
+    }
 
     return () => {
       el.removeEventListener("scroll", onScroll)

packages/views/chat/components/chat-message-list.tsx

@@ -77,7 +77,12 @@ export function ChatMessageList({
   const showStatusPill = !!pendingTaskId && !pendingAlreadyPersisted && !!pendingTask;
 
   return (
-    <div ref={scrollRef} style={fadeStyle} className="flex-1 overflow-y-auto">
+    <div
+      ref={scrollRef}
+      data-tab-scroll-root
+      style={fadeStyle}
+      className="flex-1 overflow-y-auto"
+    >
       {/* Inner container matches issue / project detail width convention
        *  (max-w-4xl + mx-auto) so switching between chat and content
        *  views doesn't jolt the reading width. px-5 is a touch tighter

packages/views/issues/components/issue-detail.tsx

@@ -1674,6 +1674,7 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
 
         <div
           ref={setScrollContainerEl}
+          data-tab-scroll-root
           className="relative flex-1 overflow-y-auto"
         >
         <div className="mx-auto w-full max-w-4xl px-8 py-8">