fix(ui): keep issue threads from jumping to latest comment (#9354)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Issue and item detail pages use a shared issue chat thread to show comments, runs, activity, and interactions. > - That thread still defaulted to landing on the latest comment when messages first loaded. > - On long issue/item pages, that default can yank the operator away from the top of the page before they choose to inspect the newest message. > - Deep links to comment hashes can create the same kind of initial viewport jump when they are used as generic navigation targets. > - This pull request makes initial latest-comment and initial thread-hash scrolling opt-in instead of default behavior. > - The benefit is stable initial page position across issue-thread surfaces while keeping the explicit Jump to latest control available. ## Linked Issues or Issue Description No exact public GitHub issue was found for this bug. Bug description: - What happened: opening a page with a shared issue conversation thread could automatically move the viewport toward the newest comment/thread target. - Expected behavior: ordinary page loads should keep the initial viewport stable unless the user explicitly clicks Jump to latest. - Steps to reproduce: open an issue or item detail page with a long conversation thread and observe whether the page jumps to the newest thread entry on initial load. - Paperclip version/commit: reproduced while working on the current `master` branch lineage. - Deployment mode: local trusted/dev UI. Related public thread/comment UX work: Refs #3916, Refs #7972, Refs #8800. ## What Changed - Changed `IssueChatThread` so initial latest-comment scrolling defaults to off. - Added a separate opt-in for initial thread-hash scrolling, also defaulting to off. - Preserved stale deleted-comment hash cleanup without scrolling the page. - Updated regression coverage so default initial load stays put, comment hashes do not scroll by default, and manual Jump to latest still scrolls. ## Verification - `pnpm --filter @paperclipai/ui typecheck` passed on the clean PR branch. - `pnpm --dir ui exec vitest run src/pages/IssueDetail.test.tsx -t "loads from the pending state into issue detail without changing hook order"` passed on the clean PR branch. - `pnpm --dir ui exec vitest run src/components/IssueChatThread.test.tsx` was attempted on the clean PR branch, but the file fails before changed assertions with the existing `TypeError: act is not a function` test-harness issue across 58 tests; 14 tests passed. - Static check: no `autoScrollToLatestOnInitialLoad={true}` or `autoScrollToHashOnInitialLoad={true}` call sites remain in `ui/src`. ## Risks Low risk. This only changes initial scroll defaults in the shared issue thread. The main behavioral shift is that direct comment/thread hashes no longer auto-scroll on first load unless a caller explicitly opts in; the Jump to latest button and post-submit scroll behavior are unchanged. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI GPT-5 via Codex coding-agent runtime; exact context window not exposed in this environment; tool-enabled repository inspection, editing, testing, git, GitHub CLI, and Paperclip API usage. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
be1fcb2b46
commit
1f07690184
|
|
@ -68,10 +68,13 @@ describe("paperclip skill utils", () => {
|
|||
const skillPath = path.resolve(".agents/skills/create-issue-interaction-ui/SKILL.md");
|
||||
const skillBody = await fs.readFile(skillPath, "utf8");
|
||||
const normalizedSkillBody = skillBody.replace(/\s+/g, " ");
|
||||
const normalizedLowerSkillBody = normalizedSkillBody.toLowerCase();
|
||||
|
||||
expect(skillBody).toContain("name: create-issue-interaction-ui");
|
||||
expect(skillBody).toContain("Developer/maintainer skill");
|
||||
expect(normalizedSkillBody).toContain("Do NOT install this on production Paperclip agents");
|
||||
expect(normalizedLowerSkillBody).toContain("developer/maintainer skill");
|
||||
expect(normalizedLowerSkillBody).toContain(
|
||||
"not the operational agents that run inside a deployed paperclip company",
|
||||
);
|
||||
expect(skillBody).toContain("packages/shared/src/constants.ts");
|
||||
expect(skillBody).toContain("server/src/services/issue-thread-interactions.ts");
|
||||
expect(skillBody).toContain("ui/src/components/IssueThreadInteractionCard.tsx");
|
||||
|
|
|
|||
|
|
@ -794,6 +794,7 @@ describe("IssueChatThread", () => {
|
|||
onAdd={async () => {}}
|
||||
showComposer={false}
|
||||
showJumpToLatest={false}
|
||||
autoScrollToHashOnInitialLoad
|
||||
enableLiveTranscriptPolling={false}
|
||||
transcriptsByRunId={issueChatLongThreadTranscriptsByRunId}
|
||||
hasOutputForRun={(runId) => issueChatLongThreadTranscriptsByRunId.has(runId)}
|
||||
|
|
@ -891,7 +892,7 @@ describe("IssueChatThread", () => {
|
|||
requestAnimationFrameMock.mockRestore();
|
||||
});
|
||||
|
||||
it("scrolls loaded hash targets through the virtualized message index", () => {
|
||||
it("scrolls loaded hash targets through the virtualized message index when initial hash scrolling is enabled", () => {
|
||||
const root = createRoot(container);
|
||||
const targetComment = issueChatLongThreadComments.at(-1);
|
||||
expect(targetComment).toBeDefined();
|
||||
|
|
@ -910,6 +911,7 @@ describe("IssueChatThread", () => {
|
|||
onAdd={async () => {}}
|
||||
showComposer={false}
|
||||
showJumpToLatest={false}
|
||||
autoScrollToHashOnInitialLoad
|
||||
enableLiveTranscriptPolling={false}
|
||||
transcriptsByRunId={issueChatLongThreadTranscriptsByRunId}
|
||||
hasOutputForRun={(runId) => issueChatLongThreadTranscriptsByRunId.has(runId)}
|
||||
|
|
@ -1071,8 +1073,8 @@ describe("IssueChatThread", () => {
|
|||
) as HTMLButtonElement | undefined;
|
||||
expect(jump).toBeDefined();
|
||||
|
||||
// Flush the on-load auto-scroll-to-latest (PAP-97) so this test measures
|
||||
// only the jump-to-latest interaction, not the initial mount scroll.
|
||||
// Flush pending mount timers so this test measures only the explicit
|
||||
// jump-to-latest interaction.
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(500);
|
||||
});
|
||||
|
|
@ -1101,10 +1103,9 @@ describe("IssueChatThread", () => {
|
|||
scrollHost.remove();
|
||||
});
|
||||
|
||||
// PAP-97: on first thread load we land on the latest comment instead of the
|
||||
// top of the thread (board rev-2 feedback for PAP-95). No deep-link hash and
|
||||
// no user interaction — the scroll must happen purely from mounting.
|
||||
it("auto-scrolls to the latest comment on initial load (PAP-97)", () => {
|
||||
// PAP-12003: initial page load must not jump to the latest comment. If a
|
||||
// user wants the newest message, the explicit Jump to latest control owns it.
|
||||
it("does not auto-scroll to the latest comment on initial load", () => {
|
||||
vi.useFakeTimers();
|
||||
container.remove();
|
||||
const scrollHost = document.createElement("main");
|
||||
|
|
@ -1142,15 +1143,13 @@ describe("IssueChatThread", () => {
|
|||
);
|
||||
});
|
||||
|
||||
// No jump click — let the mount auto-scroll's rAF + settle ticks run.
|
||||
// No jump click: initial render should preserve the page position.
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(500);
|
||||
});
|
||||
|
||||
const scrolledToLatest =
|
||||
elementScrollToMock.mock.calls.some(([arg]) => hasSmoothScrollBehavior(arg))
|
||||
|| scrollIntoViewMock.mock.calls.length > 0;
|
||||
expect(scrolledToLatest).toBe(true);
|
||||
expect(elementScrollToMock).not.toHaveBeenCalled();
|
||||
expect(scrollIntoViewMock).not.toHaveBeenCalled();
|
||||
|
||||
Element.prototype.scrollIntoView = originalScrollIntoView;
|
||||
act(() => {
|
||||
|
|
@ -1190,7 +1189,6 @@ describe("IssueChatThread", () => {
|
|||
agentMap={issueChatLongThreadAgentMap}
|
||||
currentUserId="user-board"
|
||||
onAdd={async () => {}}
|
||||
autoScrollToLatestOnInitialLoad={false}
|
||||
enableLiveTranscriptPolling={false}
|
||||
transcriptsByRunId={issueChatLongThreadTranscriptsByRunId}
|
||||
hasOutputForRun={(runId) => issueChatLongThreadTranscriptsByRunId.has(runId)}
|
||||
|
|
@ -1228,6 +1226,60 @@ describe("IssueChatThread", () => {
|
|||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("can keep the page at the top on initial load even when the URL has a comment hash", () => {
|
||||
vi.useFakeTimers();
|
||||
const originalScrollIntoView = Element.prototype.scrollIntoView;
|
||||
const scrollIntoViewMock = vi.fn();
|
||||
Object.defineProperty(Element.prototype, "scrollIntoView", {
|
||||
configurable: true,
|
||||
value: scrollIntoViewMock,
|
||||
});
|
||||
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<MemoryRouter initialEntries={["/PAP/issues/PAP-12003#comment-comment-target"]}>
|
||||
<IssueChatThread
|
||||
comments={[{
|
||||
id: "comment-target",
|
||||
companyId: "company-1",
|
||||
issueId: "issue-1",
|
||||
authorAgentId: "agent-1",
|
||||
authorUserId: null,
|
||||
authorType: "agent",
|
||||
body: "Previous done comment near the bottom.",
|
||||
presentation: null,
|
||||
metadata: null,
|
||||
createdAt: new Date("2026-07-07T21:13:07.902Z"),
|
||||
updatedAt: new Date("2026-07-07T21:13:07.902Z"),
|
||||
}]}
|
||||
linkedRuns={[]}
|
||||
timelineEvents={[]}
|
||||
liveRuns={[]}
|
||||
onAdd={async () => {}}
|
||||
showComposer={false}
|
||||
enableLiveTranscriptPolling={false}
|
||||
/>
|
||||
</MemoryRouter>,
|
||||
);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(500);
|
||||
});
|
||||
|
||||
expect(scrollIntoViewMock).not.toHaveBeenCalled();
|
||||
|
||||
Object.defineProperty(Element.prototype, "scrollIntoView", {
|
||||
configurable: true,
|
||||
value: originalScrollIntoView,
|
||||
});
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
// Regression for PAP-2672: when the merged feed ends with a non-comment row
|
||||
// (run/timeline/embedded output) we still want Jump to latest to land on the
|
||||
// last comment, not whichever activity row sorts last.
|
||||
|
|
|
|||
|
|
@ -481,6 +481,7 @@ interface IssueChatThreadProps {
|
|||
showComposer?: boolean;
|
||||
showJumpToLatest?: boolean;
|
||||
autoScrollToLatestOnInitialLoad?: boolean;
|
||||
autoScrollToHashOnInitialLoad?: boolean;
|
||||
emptyMessage?: string;
|
||||
footer?: ReactNode;
|
||||
variant?: "full" | "embedded";
|
||||
|
|
@ -4216,7 +4217,8 @@ export function IssueChatThread({
|
|||
composerHint = null,
|
||||
showComposer = true,
|
||||
showJumpToLatest,
|
||||
autoScrollToLatestOnInitialLoad = true,
|
||||
autoScrollToLatestOnInitialLoad = false,
|
||||
autoScrollToHashOnInitialLoad = false,
|
||||
emptyMessage,
|
||||
footer,
|
||||
variant = "full",
|
||||
|
|
@ -4246,6 +4248,7 @@ export function IssueChatThread({
|
|||
}: IssueChatThreadProps) {
|
||||
const location = useLocation();
|
||||
const lastScrolledHashRef = useRef<string | null>(null);
|
||||
const didInitialHashScrollDecisionRef = useRef(false);
|
||||
const virtualizedThreadRef = useRef<VirtualizedIssueChatThreadListHandle | null>(null);
|
||||
const bottomAnchorRef = useRef<HTMLDivElement | null>(null);
|
||||
const composerViewportAnchorRef = useRef<HTMLDivElement | null>(null);
|
||||
|
|
@ -4511,19 +4514,23 @@ export function IssueChatThread({
|
|||
|
||||
useEffect(() => {
|
||||
const hash = location.hash || (typeof window !== "undefined" ? window.location.hash : "");
|
||||
if (
|
||||
!(
|
||||
hash.startsWith("#comment-")
|
||||
|| hash.startsWith("#activity-")
|
||||
|| hash.startsWith("#run-")
|
||||
|| hash.startsWith("#interaction-")
|
||||
)
|
||||
) return;
|
||||
if (messages.length === 0 || lastScrolledHashRef.current === hash) return;
|
||||
const isThreadHash = hash.startsWith("#comment-")
|
||||
|| hash.startsWith("#activity-")
|
||||
|| hash.startsWith("#run-")
|
||||
|| hash.startsWith("#interaction-");
|
||||
if (messages.length === 0) return;
|
||||
if (!isThreadHash) {
|
||||
if (!didInitialHashScrollDecisionRef.current) {
|
||||
didInitialHashScrollDecisionRef.current = true;
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (lastScrolledHashRef.current === hash) return;
|
||||
const targetId = hash.slice(1);
|
||||
if (targetId.startsWith("comment-")) {
|
||||
const targetMessage = messages.find((message) => issueChatMessageAnchorId(message) === targetId);
|
||||
if (targetMessage && issueChatMessageIsDeleted(targetMessage)) {
|
||||
didInitialHashScrollDecisionRef.current = true;
|
||||
lastScrolledHashRef.current = hash;
|
||||
if (typeof window !== "undefined") {
|
||||
window.history.replaceState(null, "", `${location.pathname}${location.search}`);
|
||||
|
|
@ -4531,6 +4538,13 @@ export function IssueChatThread({
|
|||
return;
|
||||
}
|
||||
}
|
||||
if (!didInitialHashScrollDecisionRef.current) {
|
||||
didInitialHashScrollDecisionRef.current = true;
|
||||
if (!autoScrollToHashOnInitialLoad) {
|
||||
lastScrolledHashRef.current = hash;
|
||||
return;
|
||||
}
|
||||
}
|
||||
let cancelled = false;
|
||||
const attemptScroll = (finalAttempt = false) => {
|
||||
if (cancelled || lastScrolledHashRef.current === hash) return;
|
||||
|
|
@ -4549,12 +4563,11 @@ export function IssueChatThread({
|
|||
cancelAnimationFrame(frame);
|
||||
window.clearTimeout(timeout);
|
||||
};
|
||||
}, [location.hash, messageAnchorIndex, messages, useVirtualizedThread]);
|
||||
}, [autoScrollToHashOnInitialLoad, location.hash, messageAnchorIndex, messages, useVirtualizedThread]);
|
||||
|
||||
// On first thread load, land on the latest comment instead of defaulting to
|
||||
// the top (board rev-2 feedback for PAP-95). A deep-link hash takes
|
||||
// precedence — the hash-scroll effect above owns that case. Runs once per
|
||||
// mount, after messages first populate.
|
||||
// Optional legacy behavior: callers may explicitly request landing on the
|
||||
// latest comment. The shared default stays off so ordinary page loads keep
|
||||
// the user's initial viewport stable.
|
||||
useEffect(() => {
|
||||
if (didInitialLatestScrollRef.current) return;
|
||||
if (!autoScrollToLatestOnInitialLoad) return;
|
||||
|
|
|
|||
Loading…
Reference in New Issue