Skip to content

fix: preserve Shell tab scroll position across periodic refresh#633

Open
CoderLuii wants to merge 1 commit into
siteboon:mainfrom
CoderLuii:fix/shell-tab-scroll-reset
Open

fix: preserve Shell tab scroll position across periodic refresh#633
CoderLuii wants to merge 1 commit into
siteboon:mainfrom
CoderLuii:fix/shell-tab-scroll-reset

Conversation

@CoderLuii

@CoderLuii CoderLuii commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Preserve the Shell terminal viewport when the existing focus callback runs.
  • Keep focus scheduling, connection handling, refresh behavior, and auto-connect unchanged.
  • Move the downstream HolyClaude feat: Add thinking mode selector to chat interface #35 vendored-bundle patch into source.

Context

CoderLuii/HolyClaude#35 reported the Shell tab losing scroll position during the periodic browser refresh in the HolyClaude full image on Linux. The downstream reproduction was: open HolyClaude, go to the Shell tab, scroll inside a long shell session, wait for the periodic refresh, and the viewport jumps away from the line being read.

HolyClaude v1.2.1 fixed that downstream by patching the vendored CloudCLI bundle to preserve buffer.active.viewportY around the Shell focus() call. Current CloudCLI source still focuses the terminal without preserving the viewport, so this PR applies the same behavior in src/components/shell/view/Shell.tsx.

Testing

  • git diff --check
  • npm ci completed; npm audit reported 39 existing advisories
  • npm run typecheck
  • npm run build passed with existing Browserslist/CSS/chunk-size warnings
  • npm run lint passed with 0 errors and existing repo-wide warnings
  • Built app smoke: login completed with a disposable local account, a test project opened, and the Shell tab mounted with xterm focused

@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Updated the focusTerminal logic to preserve the terminal's viewport position when focusing. The implementation now saves the current viewport position before focusing and restores it afterward, maintaining view continuity while ensuring the terminal receives focus.

Changes

Cohort / File(s) Summary
Terminal Focus Viewport Preservation
src/components/shell/view/Shell.tsx
Modified the focusTerminal effect to capture the current viewport position before focusing, then restore it after. Focus scheduling via requestAnimationFrame and setTimeout remains unchanged.

Poem

🐰 The terminal now holds its view so tight,
When focus arrives, the viewport stays in sight,
No more jumping scrolls or vanished lines,
Just smooth refocus—a rabbit-approved design! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: preserve Shell tab scroll position across periodic refresh' directly and accurately describes the main change: preserving scroll position in the Shell tab during periodic refreshes.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@CoderLuii

CoderLuii commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by the refreshed PR branch and the newer maintainer reply below. The branch has now been rebased onto current main, the PR body was cleaned up, and the latest verification is recorded below.

@blackmammoth blackmammoth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @CoderLuii, thanks for the PR. Can you send me a detailed reproduction step for this issue? I tried this both on Windows and Android (using Chrome) and I didn't notice a scroll issue with the shell.

@blackmammoth blackmammoth marked this pull request as draft June 4, 2026 10:57
@CoderLuii CoderLuii force-pushed the fix/shell-tab-scroll-reset branch from fecacea to 583bb46 Compare June 15, 2026 18:15
@CoderLuii CoderLuii marked this pull request as ready for review June 15, 2026 18:16
@CoderLuii

Copy link
Copy Markdown
Contributor Author

@blackmammoth thanks for checking this. I traced the report back to CoderLuii/HolyClaude#35. That downstream report was for the HolyClaude full image on Linux: open the Shell tab, scroll inside a long shell session, wait for the periodic browser refresh, and the terminal viewport jumps away from the line being read.

HolyClaude v1.2.1 fixed it downstream by patching the vendored CloudCLI bundle to preserve buffer.active.viewportY around the Shell focus() call. This PR applies the same fix at source in Shell.tsx, so the terminal can still regain focus without forcing the viewport away from the line the user was reading.

I refreshed the branch against current main, cleaned up the PR body, and re-ran git diff --check, npm ci, npm run typecheck, npm run build, and npm run lint. I also opened the built app locally and confirmed the Shell tab mounts with the xterm input focused. The change is intentionally small: it does not change connection handling, refresh behavior, or auto-connect logic. It only preserves the terminal viewport around the existing focus call.

Ready for another look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants