mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
Avoid scroll reset on file viewer query changes
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
bce60bce8e
commit
8f6f8ce205
2 files changed
+41
-2
No files matched your search
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<typeof useNavigationType>;
|
||||
}): 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<HTMLInputElement | null>(null);
|
||||
const lastMarkedReadIssueIdRef = useRef<string | null>(null);
|
||||
const lastScrollIssueIdRef = useRef<string | undefined>(undefined);
|
||||
const commentComposerRef = useRef<IssueChatComposerHandle | null>(null);
|
||||
const cancelledQueuedOptimisticCommentIdsRef = useRef(new Set<string>());
|
||||
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;
|
||||
|
||||
Reference in new issue
Block a user