feat: add auto-scroll toggle for AI output#390
Conversation
…sume Add configurable auto-scroll behavior for AI mode output that automatically scrolls to bottom when new content arrives, with smart pause when user scrolls up to read content and automatic resume when they scroll back to bottom. - Add autoScrollAiMode setting persisted via electron-store (default: off) - Add inline floating toggle button (bottom-left) with visual state feedback - Add keyboard shortcut (Alt+Cmd+S) registered as system utility shortcut - Add Settings Modal checkbox in General tab - Thread setting through App → MainPanel → TerminalOutput via props - Use instant scroll (behavior: 'auto') to prevent jitter during streaming - Pause auto-scroll when user scrolls up, resume on scroll-to-bottom - Toggle button reflects paused state (dim icon when paused) - New message indicator shown when paused (user scrolled away from bottom) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Pass autoScrollAiMode/setAutoScrollAiMode as props from App to SettingsModal so toggling takes immediate effect (was using independent useSettings() instance that didn't propagate to App's render tree) - Disable browser native scroll anchoring (overflow-anchor: none) on the AI output scroll container when auto-scroll is off or paused, preventing the browser from pinning viewport to bottom on new content - Add programmaticScrollRef guard and autoScrollPaused state to properly distinguish user scrolls from effect-driven scrolls Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pedramamini
left a comment
There was a problem hiding this comment.
Clean feature, well-scoped, follows existing patterns. A few issues worth fixing before merge.
Summary:
- Dead ref (
programmaticScrollRef) — remove - IIFE in JSX allocates every render — extract to a variable
- Resume-from-pause doesn't scroll to bottom — users will be stuck at old position until new content arrives
- Double dep trigger on auto-scroll effect —
filteredLogs.lengthandshellLogs.lengthboth change per message - No test updates despite 8 files changed with new state/props/shortcut/UI
Overall approach is solid — the tri-control (setting + shortcut + inline button), overflow-anchor: none, behavior: 'auto' for streaming, and default-OFF are all good calls.
|
|
||
| {/* Inline auto-scroll toggle - floating button (AI mode only) */} | ||
| {/* Shows active state only when auto-scroll is on AND not paused by user scrolling up */} | ||
| {session.inputMode === 'ai' && setAutoScrollAiMode && (() => { |
There was a problem hiding this comment.
This IIFE creates a new function every render just to hoist isActive into scope. Extract to a variable instead:
const isAutoScrollActive = autoScrollAiMode && !autoScrollPaused;
// ... in JSX:
{session.inputMode === 'ai' && setAutoScrollAiMode && (
<button
style={{ backgroundColor: isAutoScrollActive ? ... }}
...
/>
)}Simpler, no allocation, easier to read.
| return ( | ||
| <button | ||
| onClick={() => { | ||
| if (autoScrollPaused) { |
There was a problem hiding this comment.
When paused, clicking resume clears the pause flag but doesn't scroll to bottom. The user stays at their current scroll position until new content arrives and triggers the auto-scroll effect.
Should call scrollToBottom() here too:
if (autoScrollPaused) {
setAutoScrollPaused(false);
scrollToBottom(); // <-- jump to bottom immediately
} else {| }); | ||
| } | ||
| }, [session.inputMode, session.shellLogs.length]); | ||
| }, [session.inputMode, session.shellLogs.length, filteredLogs.length, autoScrollAiMode, autoScrollPaused]); |
There was a problem hiding this comment.
Both session.shellLogs.length and filteredLogs.length are in the dep array, but filteredLogs is derived from shellLogs. When a new log arrives, both change, potentially firing this effect twice per message.
Consider using only filteredLogs.length since it's the more correct dependency (accounts for search filtering). Or if you need the raw length for terminal mode, keep both but be aware of the double-fire.
Remove unused programmaticScrollRef declaration that was left over from an earlier iteration. It was declared but never read or written. Addresses PR RunMaestro#390 review feedback (Task 1). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace inline IIFE that computed isActive within JSX with a pre-computed isAutoScrollActive variable before the return statement. Eliminates unnecessary function creation per render and improves readability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…new content When auto-scroll is paused and the user clicks resume, immediately scroll to the bottom using behavior: 'auto' (instant snap) rather than leaving the viewport at the old scroll position until new content arrives. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ffect deps filteredLogs is derived from shellLogs via activeLogs → collapsedLogs → filteredLogs, so having both in the dependency array caused the effect to double-fire on every new log message. Keep only filteredLogs.length which correctly covers both AI and terminal modes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…button rendering, and props threading Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…and palette Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All review items addressed + additional fixEvery item from the review has been resolved. Here's the full breakdown: Review Item #1 — Dead
|
…ggle # Conflicts: # src/renderer/components/TerminalOutput.tsx # src/renderer/hooks/settings/useSettings.ts
Branch synced with latest main (e373e63)Merged
Lint and all 19,697 tests pass. |
pedramamini
left a comment
There was a problem hiding this comment.
All four review items addressed cleanly in dedicated commits. Good additions beyond the review:
- QuickActionsModal integration — auto-scroll toggle is now discoverable via Cmd+K command palette, consistent with other toggles
- settingsStore.ts migration — implemented in the Zustand store pattern rather than only in useSettings hook, following the codebase's migration direction
- 14 tests covering defaults, toggle persistence, shortcut registration, button rendering (AI vs terminal), click behavior, styling states, pause-on-scroll, resume-with-snap, props threading, and backward compatibility
The inline scrollTo on resume using behavior: 'auto' instead of reusing scrollToBottom() (which uses 'smooth') is the right call — snap > animate for resume.
LGTM.
…ide encoreFeatures
Auto-Scroll Toggle for AI Output
Feature Overview
Adds configurable auto-scroll behavior for AI mode output. When enabled, the terminal output automatically scrolls to the bottom as new AI content arrives — matching terminal mode behavior. The feature includes smart pause/resume: scrolling up pauses auto-scroll so you can read content, and scrolling back to bottom resumes it.
Controls
Alt+Cmd+S(works even with modals open)Behavior
Visual Feedback
The inline toggle button reflects three states:
Technical Details
autoScrollAiMode(boolean)behavior: auto(instant) instead ofsmoothto prevent jitter during rapid streamingoverflow-anchorCSS on the scroll container when auto-scroll is off/paused, preventing the browser from pinning the viewport to bottomautoScrollPausedis React state (not a ref) so the toggle button re-renders to reflect pause stateautoScrollAiMode/setAutoScrollAiModeas props from App (not from its ownuseSettings()instance) to ensure changes take immediate effectFiles Changed
useSettings.tsautoScrollAiModestate, persistence, loadingApp.tsxuseMainPanelProps.tsMainPanel.tsxTerminalOutput.tsxoverflow-anchorfix, inline toggle buttonSettingsModal.tsxshortcuts.tstoggleAutoScrollshortcut definition (Alt+Meta+S)useMainKeyboardHandler.tsTest plan
Alt+Cmd+Skeyboard shortcut🤖 Generated with Claude Code