Restore chat scroll affordances and add timeline minimap#3587
Restore chat scroll affordances and add timeline minimap#3587juliusmarminge wants to merge 2 commits into
Conversation
- Prevent the scroll-to-end pill from flickering during thread switches - Preserve anchor scroll position during user-driven scrolling - Add timeline minimap navigation and live-edge detection
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Inverted anchor scroll restore
- Removed the inverted <= 2px pixel-threshold guard that caused scroll restoration to only fire when drift was negligible (a no-op) and skip when drift was large (when it was actually needed); the userScrollGeneration check already correctly handles user-initiated scroll suppression.
Or push these changes by commenting:
@cursor push cb3843c97d
Preview (cb3843c97d)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -3273,14 +3273,7 @@
settledTimelineAnchorRef.current === pending.messageId &&
pending.userScrollGeneration === anchorUserScrollGenerationRef.current
) {
- const list = legendListRef.current;
- const currentScrollOffset = list?.getState().scroll;
- if (
- typeof currentScrollOffset === "number" &&
- Math.abs(currentScrollOffset - pending.offset) <= 2
- ) {
- void list?.scrollToOffset({ offset: pending.offset, animated: false });
- }
+ void legendListRef.current?.scrollToOffset({ offset: pending.offset, animated: false });
}
});
}, []);You can send follow-ups to the cloud agent here.
| Math.abs(currentScrollOffset - pending.offset) <= 2 | ||
| ) { | ||
| void list?.scrollToOffset({ offset: pending.offset, animated: false }); | ||
| } |
There was a problem hiding this comment.
Inverted anchor scroll restore
Medium Severity
In onTimelineAnchorSizeChanged, scroll restoration only applies if the current scroll offset is within 2px of the saved position. This causes the viewport to jump instead of staying pinned to the anchored message when layout drift, common during streaming, shifts the scroll position by more than 2px.
Reviewed by Cursor Bugbot for commit da040f6. Configure here.
ApprovabilityVerdict: Needs human review This PR introduces a new timeline minimap UI component and modifies scroll behavior logic, which constitutes new user-facing functionality requiring human review. Additionally, two unresolved medium-severity findings identify potential UX issues with scroll restoration and minimap overflow. You can customize Macroscope's approvability policy. Learn more. |
- Switch the minimap from a fixed-height rail to item-spaced positioning - Preserve hover/focus width cues while keeping the scroll-jump targets visible
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Minimap grows past viewport
- Added TIMELINE_MINIMAP_MAX_HEIGHT (180px) cap so the minimap height never exceeds a bounded value, and when items exceed that cap the spacing compresses proportionally to keep all strips visible and clickable.
Or push these changes by commenting:
@cursor push db100acdc9
Preview (db100acdc9)
diff --git a/apps/web/src/components/chat/MessagesTimeline.logic.ts b/apps/web/src/components/chat/MessagesTimeline.logic.ts
--- a/apps/web/src/components/chat/MessagesTimeline.logic.ts
+++ b/apps/web/src/components/chat/MessagesTimeline.logic.ts
@@ -11,6 +11,7 @@
export const MAX_VISIBLE_WORK_LOG_ENTRIES = 1;
export const TIMELINE_MINIMAP_ITEM_SPACING = 8;
+export const TIMELINE_MINIMAP_MAX_HEIGHT = 180;
export const TIMELINE_MINIMAP_MIN_ITEMS = 2;
export interface TimelineEndState {
diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx
--- a/apps/web/src/components/chat/MessagesTimeline.tsx
+++ b/apps/web/src/components/chat/MessagesTimeline.tsx
@@ -73,6 +73,7 @@
type StableMessagesTimelineRowsState,
type MessagesTimelineRow,
TIMELINE_MINIMAP_ITEM_SPACING,
+ TIMELINE_MINIMAP_MAX_HEIGHT,
TIMELINE_MINIMAP_MIN_ITEMS,
type TimelineLatestTurn,
} from "./MessagesTimeline.logic";
@@ -562,7 +563,10 @@
return null;
}
- const minimapHeight = Math.max(1, (items.length - 1) * TIMELINE_MINIMAP_ITEM_SPACING);
+ const naturalHeight = Math.max(1, (items.length - 1) * TIMELINE_MINIMAP_ITEM_SPACING);
+ const minimapHeight = Math.min(naturalHeight, TIMELINE_MINIMAP_MAX_HEIGHT);
+ const effectiveSpacing =
+ items.length > 1 ? minimapHeight / (items.length - 1) : 0;
return (
<div
@@ -592,7 +596,7 @@
<div className="relative w-10 select-none" style={{ height: minimapHeight }}>
<div className="absolute top-0 left-3 h-full w-px bg-border/15" />
{items.map((item, index) => {
- const top = index * TIMELINE_MINIMAP_ITEM_SPACING;
+ const top = index * effectiveSpacing;
return (
<button
aria-label={`Jump to message: ${item.userText ?? "User message"}`}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 8fdd7de. Configure here.
| } | ||
| `} | ||
| </style> | ||
| <div className="relative w-10 select-none" style={{ height: minimapHeight }}> |
There was a problem hiding this comment.
Minimap grows past viewport
Medium Severity
Replacing the fixed minimap height with minimapHeight = (items.length - 1) * TIMELINE_MINIMAP_ITEM_SPACING lets the rail grow without a cap. The minimap stays vertically centered in the chat column, which lives under overflow-hidden, so strips for early and late user turns can be clipped and their jump buttons become unreachable on long threads.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8fdd7de. Configure here.
1 participant
Summary
Testing
apps/web/src/components/chat/MessagesTimeline.test.tsx: added coverage forresolveTimelineIsAtEndbehavior.vp check.vp run typecheck.Note
Medium Risk
Touches chat scroll anchoring and programmatic restore logic, where regressions could fight user scrolling or mis-show the live-edge pill; changes are localized to web chat UI with a small unit test for end-state resolution.
Overview
Adds a timeline minimap on medium+ viewports: one strip per user turn, in-view highlighting on scroll, hover previews, and animated jump to the row.
Live-edge detection now prefers LegendList
isNearEndviaresolveTimelineIsAtEnd, so the scroll-to-end pill matches “near the bottom” instead of only strictisAtEnd. The pill is renamed to Scroll to end with accessibility labels;scrollToEndimmediately clears the pill and marks at-end.ChatView scroll behavior is tightened: composer overlay height ignores zero reads; anchor offset restore is skipped after user wheel/touch/pointer scroll (generation counter) and only when scroll is still within ~2px of the saved offset.
Reviewed by Cursor Bugbot for commit 8fdd7de. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Restore chat scroll affordances and add a timeline minimap for user messages
scrollToEndto immediately hide the scroll-to-end pill and mark the list as at-end before scrolling, and renames the button label from 'Scroll to bottom' to 'Scroll to end'.resolveTimelineIsAtEndin MessagesTimeline.logic.ts so the is-at-end state reported to parents prefersisNearEndoverisAtEndwhen available.Macroscope summarized 8fdd7de.