From 8f6f8ce2059603c40cbffc0fb653a20a2b8ac1db Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 8 Jun 2026 16:18:44 +0000 Subject: [PATCH] Avoid scroll reset on file viewer query changes Co-Authored-By: Paperclip --- ui/src/pages/IssueDetail.test.tsx | 29 ++++++++++++++++++++++++++++- ui/src/pages/IssueDetail.tsx | 14 +++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index b035cc50e4a..1ed1726a8b1 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -3,10 +3,11 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { Agent, Issue, IssueAttachment, IssueTreeControlPreview, IssueTreeHold, IssueWorkProduct } from "@paperclipai/shared"; import type { AnchorHTMLAttributes, ButtonHTMLAttributes, ReactNode } from "react"; +import { NavigationType } from "react-router-dom"; import { flushSync } from "react-dom"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { canBoardResolveRecoveryAction, IssueDetail } from "./IssueDetail"; +import { canBoardResolveRecoveryAction, IssueDetail, shouldScrollIssueDetailToTopOnNavigation } from "./IssueDetail"; const mockIssuesApi = vi.hoisted(() => ({ get: vi.fn(), @@ -1961,3 +1962,29 @@ describe("canBoardResolveRecoveryAction", () => { ).toBe(false); }); }); + +describe("shouldScrollIssueDetailToTopOnNavigation", () => { + it("does not scroll when only URL search params changed for the same issue", () => { + expect(shouldScrollIssueDetailToTopOnNavigation({ + previousIssueId: "PAP-10306", + nextIssueId: "PAP-10306", + navigationType: NavigationType.Push, + })).toBe(false); + }); + + it("scrolls on forward navigation to a different issue", () => { + expect(shouldScrollIssueDetailToTopOnNavigation({ + previousIssueId: "PAP-1", + nextIssueId: "PAP-2", + navigationType: NavigationType.Push, + })).toBe(true); + }); + + it("does not scroll on browser back or forward restoration", () => { + expect(shouldScrollIssueDetailToTopOnNavigation({ + previousIssueId: "PAP-1", + nextIssueId: "PAP-2", + navigationType: NavigationType.Pop, + })).toBe(false); + }); +}); diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index bdd56fe84ff..3686fbf36d3 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -267,6 +267,15 @@ export function canBoardResolveRecoveryAction( return membership.membershipRole !== "viewer" && membership.membershipRole !== null; } +export function shouldScrollIssueDetailToTopOnNavigation(input: { + previousIssueId: string | undefined; + nextIssueId: string | undefined; + navigationType: ReturnType; +}): boolean { + if (input.navigationType === "POP") return false; + return input.previousIssueId !== input.nextIssueId; +} + function resolveRunningIssueRun( activeRun: ActiveRunForIssue | null | undefined, liveRuns: readonly LiveRunForIssue[] | undefined, @@ -1312,6 +1321,7 @@ export function IssueDetail() { const [pendingCommentComposerFocusKey, setPendingCommentComposerFocusKey] = useState(0); const fileInputRef = useRef(null); const lastMarkedReadIssueIdRef = useRef(null); + const lastScrollIssueIdRef = useRef(undefined); const commentComposerRef = useRef(null); const cancelledQueuedOptimisticCommentIdsRef = useRef(new Set()); const resolvedIssueDetailState = useMemo( @@ -2775,7 +2785,9 @@ export function IssueDetail() { // Scroll to top on forward navigation (PUSH/REPLACE) so issue doesn't // inherit the inbox/issues-list scroll position on mobile. useEffect(() => { - if (navigationType === "POP") return; + const previousIssueId = lastScrollIssueIdRef.current; + lastScrollIssueIdRef.current = issueId; + if (!shouldScrollIssueDetailToTopOnNavigation({ previousIssueId, nextIssueId: issueId, navigationType })) return; window.scrollTo({ top: 0, left: 0, behavior: "auto" }); const main = document.getElementById("main-content"); if (main) main.scrollTop = 0;