diff --git a/src/view/scroll-position-manager.ts b/src/view/scroll-position-manager.ts deleted file mode 100644 index dc4a442..0000000 --- a/src/view/scroll-position-manager.ts +++ /dev/null @@ -1,47 +0,0 @@ -import type {Terminal} from "@xterm/xterm"; - -/** - * Mitigates Ink TUI scroll-to-top bug (Claude Code issue #826). - * When Ink redraws, it can cause the terminal to jump to the top. - * Detects suspicious jumps and restores the previous scroll position. - */ -export class ScrollPositionManager { - private lastStableScrollPos = 0; - private scrollRestoreTimeout: ReturnType | null = null; - private scrollLockUntil = 0; - - private static readonly THRESHOLD = 10; - private static readonly LOCK_DURATION = 200; - private static readonly RESTORE_DELAY = 50; - - attach(terminal: Terminal): void { - terminal.onScroll((newPos) => this.handleScroll(terminal, newPos)); - } - - private handleScroll(terminal: Terminal, newPos: number): void { - const now = Date.now(); - - // If we jumped to top from a significant scroll position, restore - if (newPos === 0 && this.lastStableScrollPos > ScrollPositionManager.THRESHOLD) { - if (this.scrollRestoreTimeout) { - clearTimeout(this.scrollRestoreTimeout); - } - this.scrollLockUntil = now + ScrollPositionManager.LOCK_DURATION; - const restorePos = this.lastStableScrollPos; - this.scrollRestoreTimeout = setTimeout(() => { - terminal.scrollToLine(restorePos); - this.scrollRestoreTimeout = null; - }, ScrollPositionManager.RESTORE_DELAY); - } else if (newPos > 0 && now > this.scrollLockUntil) { - // Update stable position only when not locked (user scrolling normally) - this.lastStableScrollPos = newPos; - } - } - - dispose(): void { - if (this.scrollRestoreTimeout) { - clearTimeout(this.scrollRestoreTimeout); - this.scrollRestoreTimeout = null; - } - } -} diff --git a/src/view/terminal-view.ts b/src/view/terminal-view.ts index 55d751f..165877f 100644 --- a/src/view/terminal-view.ts +++ b/src/view/terminal-view.ts @@ -4,7 +4,6 @@ import { FitAddon } from "@xterm/addon-fit"; import { getThemeColors } from "../theme/xterm-theme"; import { injectXtermCSS } from "../theme/xterm-css"; import { TerminalProcess, connectTerminalToProcess, ClaudeCommandOptions } from "../terminal/terminal-process"; -import { ScrollPositionManager } from "./scroll-position-manager"; import type VaultCodePlugin from "../main"; export const VIEW_TYPE = "vault-terminal"; @@ -19,7 +18,6 @@ export class TerminalView extends ItemView { private escapeScope: Scope | null = null; private fitTimeout: ReturnType | null = null; private plugin: VaultCodePlugin; - private scrollManager = new ScrollPositionManager(); constructor(leaf: WorkspaceLeaf, plugin: VaultCodePlugin) { super(leaf); @@ -140,9 +138,6 @@ export class TerminalView extends ItemView { // Watch for Obsidian layout changes (sidebar resize, etc.) this.registerEvent(this.app.workspace.on("layout-change", () => this.debouncedFit())); - - // Mitigate Ink TUI scroll-to-top bug (Claude Code issue #826) - this.scrollManager.attach(this.term); } private updateTheme(): void { @@ -250,8 +245,6 @@ export class TerminalView extends ItemView { this.fitTimeout = null; } - this.scrollManager.dispose(); - if (this.escapeScope) { this.app.keymap.popScope(this.escapeScope); this.escapeScope = null; diff --git a/tests/view/scroll-position-manager.test.ts b/tests/view/scroll-position-manager.test.ts deleted file mode 100644 index a74bf80..0000000 --- a/tests/view/scroll-position-manager.test.ts +++ /dev/null @@ -1,210 +0,0 @@ -import {describe, it, expect, vi, beforeEach, afterEach} from "vitest"; -import {ScrollPositionManager} from "../../src/view/scroll-position-manager"; -import type {Terminal} from "@xterm/xterm"; - -describe("ScrollPositionManager", () => { - let manager: ScrollPositionManager; - let mockTerminal: Partial; - let scrollHandler: ((newPos: number) => void) | null = null; - - beforeEach(() => { - vi.useFakeTimers(); - manager = new ScrollPositionManager(); - - mockTerminal = { - onScroll: vi.fn((handler) => { - scrollHandler = handler; - return {dispose: vi.fn()}; - }), - scrollToLine: vi.fn(), - }; - }); - - afterEach(() => { - vi.useRealTimers(); - manager.dispose(); - scrollHandler = null; - }); - - describe("attach", () => { - it("should register scroll handler on terminal", () => { - manager.attach(mockTerminal as Terminal); - - expect(mockTerminal.onScroll).toHaveBeenCalled(); - expect(scrollHandler).toBeInstanceOf(Function); - }); - }); - - describe("scroll behavior", () => { - beforeEach(() => { - manager.attach(mockTerminal as Terminal); - }); - - it("should track stable scroll position", () => { - // Scroll to position 50 - scrollHandler!(50); - - // Scroll to position 100 - scrollHandler!(100); - - // No restore should happen - just normal scrolling - expect(mockTerminal.scrollToLine).not.toHaveBeenCalled(); - }); - - it("should restore position when jumping from significant scroll to top", () => { - // Build up stable position beyond threshold (10) - scrollHandler!(50); - - // Simulate Ink bug - jump to top - scrollHandler!(0); - - // Should schedule restore after 50ms - vi.advanceTimersByTime(50); - - expect(mockTerminal.scrollToLine).toHaveBeenCalledWith(50); - }); - - it("should not restore if scroll position was below threshold", () => { - // Position below threshold (10) - scrollHandler!(5); - - // Jump to top - scrollHandler!(0); - - vi.advanceTimersByTime(50); - - expect(mockTerminal.scrollToLine).not.toHaveBeenCalled(); - }); - - it("should not restore if scroll position equals threshold", () => { - // Position at threshold exactly - scrollHandler!(10); - - // Jump to top - scrollHandler!(0); - - vi.advanceTimersByTime(50); - - expect(mockTerminal.scrollToLine).not.toHaveBeenCalled(); - }); - - it("should restore if scroll position exceeds threshold", () => { - // Position just above threshold - scrollHandler!(11); - - // Jump to top - scrollHandler!(0); - - vi.advanceTimersByTime(50); - - expect(mockTerminal.scrollToLine).toHaveBeenCalledWith(11); - }); - - it("should debounce multiple rapid jumps to top", () => { - scrollHandler!(100); - - // Multiple rapid jumps - scrollHandler!(0); - scrollHandler!(0); - scrollHandler!(0); - - vi.advanceTimersByTime(50); - - // Should only restore once - expect(mockTerminal.scrollToLine).toHaveBeenCalledTimes(1); - }); - - it("should lock scroll position during restore window", () => { - scrollHandler!(100); - scrollHandler!(0); - - // Advance partially through lock duration (200ms) - vi.advanceTimersByTime(100); - - // Try to update stable position during lock - scrollHandler!(200); - - // Trigger another jump to top - scrollHandler!(0); - - // Complete both restore delays - vi.advanceTimersByTime(50); - - // Should restore to original position (100), not 200 - // because scrolling during lock doesn't update stable position - expect(mockTerminal.scrollToLine).toHaveBeenLastCalledWith(100); - }); - - it("should resume tracking after lock duration expires", () => { - scrollHandler!(100); - scrollHandler!(0); - - // Wait for lock to expire (200ms) plus restore delay - vi.advanceTimersByTime(250); - - // Now scroll to new position - scrollHandler!(150); - - // Jump to top again - scrollHandler!(0); - vi.advanceTimersByTime(50); - - // Should restore to new stable position - expect(mockTerminal.scrollToLine).toHaveBeenLastCalledWith(150); - }); - - it("should not update stable position when at 0", () => { - // Scroll to position above threshold - scrollHandler!(100); - - // Jump to 0 (triggers restore) - scrollHandler!(0); - vi.advanceTimersByTime(50); - expect(mockTerminal.scrollToLine).toHaveBeenCalledWith(100); - - // Wait for lock to expire - vi.advanceTimersByTime(200); - - // Scroll to new position (should update stable position) - scrollHandler!(50); - - // Jump to 0 again - scrollHandler!(0); - vi.advanceTimersByTime(50); - - // Should restore to new stable position (50) - expect(mockTerminal.scrollToLine).toHaveBeenLastCalledWith(50); - }); - }); - - describe("dispose", () => { - it("should clear pending timeout", () => { - manager.attach(mockTerminal as Terminal); - - scrollHandler!(100); - scrollHandler!(0); - - // Dispose before timeout fires - manager.dispose(); - vi.advanceTimersByTime(100); - - // Should not have restored - expect(mockTerminal.scrollToLine).not.toHaveBeenCalled(); - }); - - it("should be safe to call multiple times", () => { - manager.attach(mockTerminal as Terminal); - - expect(() => { - manager.dispose(); - manager.dispose(); - }).not.toThrow(); - }); - - it("should be safe to call before attach", () => { - expect(() => { - manager.dispose(); - }).not.toThrow(); - }); - }); -});