fix: break Linux resize/redraw scroll loop (#46)#47
Draft
mollyretter wants to merge 6 commits into
Draft
Conversation
* feat: add primary/secondary sidebar architecture and pre-plugin cleanup Introduces dual-sidebar layout and extracts App.svelte into focused service modules in preparation for plugin infrastructure. Sidebar architecture: - Rename Sidebar to PrimarySidebar (vertically scrolling sections) - Add SecondarySidebar (tab-controlled with control row slot) - Both collapsible (toggles in TitleBar) and drag-resizable (max 33%) - Remove Split Pane button from sidebar, remove active pane blue border Pre-plugin cleanup: - Delete dead src/config.ts, unused imports, half-implemented zoom feature - Type PendingAction as discriminated union - Extract drag-resize into shared Svelte action - Extract services from App.svelte (721 -> 280 lines): workspace-service, pane-service, surface-service - Add lefthook for pre-commit and pre-push hooks Documentation: - Add docs/sidebar-architecture.md and docs/glossary.md - Update CLAUDE.md with architecture docs and service structure Closes #40 Refs #43 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove dead isActivePane prop from PaneView Leftover from removing the active-pane blue border. Silences the vite-plugin-svelte unused-export warning and keeps the prop chain honest. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: rename plugin → extension in project-owned strings The project's "plugin" concept is now called "extension" everywhere. Updated docs (glossary, sidebar-architecture, README), the secondary sidebar control-row comment, the preview system's self-register comment, and the theme-accessor doc comment. Third-party Tauri/Vite package names (tauri-plugin-*, vite-plugin-*) are left untouched — those are external symbols. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Nicholas Maloney <nick@gnar.dog>
) On Linux (WebKitGTK), PTY output triggers spurious ResizeObserver callbacks that create a fit()→resize_pty→SIGWINCH→redraw loop, yanking the viewport to the top. Also makes it impossible to scroll up during active output. Three guards: - createResizeHandler: dedup resize_pty when cols/rows unchanged - shouldFit: skip fitAddon.fit() when container pixels unchanged - isScrolledToBottom: preserve scroll position when user is scrolled up Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The per-flush isScrolledToBottom check was unreliable during active output because xterm.js auto-scrolls during write() before our restore callback fires. New approach uses wheel events (user-initiated only) to set a persistent anchor that survives across rapid flush cycles. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous approach (restoring scroll position in the write callback) was still jumpy because xterm.js auto-scrolls during write() before the callback fires — producing one wrong frame per flush cycle. New approach: terminal.onScroll fires synchronously during write() processing, before any render. A writeInFlight flag distinguishes auto-scrolls (counteracted immediately) from user scrolls (which update the anchor). The viewport is corrected before the browser paints. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
#46) xterm.js already has built-in scroll lock via BufferService.isUserScrolling. When the user scrolls up, it sets the flag and subsequent write() calls don't auto-scroll. Our onScroll interception was actually fighting this native behavior — scrollToLine() calls were clearing isUserScrolling via the scrollLines() path. The real fix is just the two resize guards (createResizeHandler, shouldFit) which prevent the spurious fit→resize→viewport sync cycle on Linux that was clearing isUserScrolling. With those guards, xterm's native scroll lock works correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the Linux-only bug where the terminal viewport jumps during active output and users can't scroll up.
Closes #46
Root cause discovery
xterm.js already has built-in scroll lock via `BufferService.isUserScrolling` — when the user scrolls up, subsequent `write()` calls don't auto-scroll. On Linux, WebKitGTK's spurious `ResizeObserver` callbacks trigger a `fit()` → `resize` → `Viewport._sync()` → `scrollLines()` chain that clears this flag. On macOS, WKWebView doesn't fire spurious ResizeObservers, so the native scroll lock is never broken.
Changes
Two resize guards in `src/lib/resize-guard.ts` that prevent the spurious cycle:
Earlier commits (scroll anchor attempts) removed
Three prior approaches (per-flush snapshot, wheel-event anchor, onScroll interception) were fighting xterm.js's native behavior. They've been removed — the resize guards alone are the correct fix.
macOS safety
Both guards are no-ops on macOS — WKWebView only fires ResizeObserver on real container size changes, so the dedup checks always pass through.
Test plan
🤖 Generated with Claude Code