fix: reset settings scroll position when switching sections - #2082
Merged
Merged
Conversation
Settings sections (General/History/Models/...) render inside one persistent scrollable container that never unmounts between switches. As a result, scroll position carried over from one section to the next (e.g. scroll down in History, click Models, Models opens already scrolled). Add a ref to the scroll container and a useLayoutEffect keyed on currentSection that resets scrollTop to 0 before paint on every section change. Co-Authored-By: Claude Code (Anthropic) <agent@nousresearch.com>
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues were identified.
Pull request overview
Resets the Settings scroll position when switching sections.
Changes:
- Added a scroll-container ref.
- Reset scroll position on section changes with
useLayoutEffect. - Attached the ref to the shared settings container.
File summaries
| File | Summary |
|---|---|
src/App.tsx |
Resets scroll position when changing Settings sections. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Owner
|
This is a really good catch. We will definitely pull this in ASAV as soon as I |
NairoDorian
added a commit
to NairoDorian/S2B2S
that referenced
this pull request
Sep 25, 2026
…cjpais#2033, cjpais#2040, cjpais#2082, cjpais#2106) Integrates changes from cjpais/Handy:main adapted for ZER0's Solid 2 frontend, Tauri 3 backend, and Multi-STT pipeline: - compound shortcuts (upstream cjpais#1862): emit parseable compact names for compound keys (e.g. `scrolllock`, `capslock`, `numlock`) while preserving friendly display labels; add backend Hotkey / Shortcut parser regression test and test:keyboard script - history clipboard (upstream cjpais#2011): extract copyToClipboard helper with error handling and unit tests; display error toast if clipboard write fails; add copyError translation across all 26 locales - reset bindings (upstream cjpais#2033): reject unknown shortcut binding IDs via settings::get_stored_binding with unit tests - settings scroll reset (upstream cjpais#2082): reset settings container scroll position to top on section change in App.tsx using Solid 2 createEffect - empty audio model unload (upstream cjpais#2106): ensure FinishGuard calls maybe_unload_immediately on drop for both standard and multi-STT recording completion or early termination - docs (upstream cjpais#2040): document recording overlay workaround for Hyprland / Omarchy with Wayland in README.md Cost: no new runtime threads, polls, or external dependencies.
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
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.
Before Submitting This PR
Human Written Description
I noticed that in Settings, if you scroll down while viewing History and then switch to Models (or any other section), the new section opens already scrolled down instead of at the top. This is confusing since each section has different, unrelated content, and it can hide the top of a section behind the previous scroll offset. It's a small but noticeable UX papercut that's easy and low-risk to fix.
Related Issues/Discussions
Fixes #2081
Root Cause
All settings sections (General/History/Models/...) render inside one persistent scrollable container in
src/App.tsx(<div className="flex-1 overflow-y-auto">, around line 373). BecauserenderSettingsContent()swaps only the child content and the scroll container itself never unmounts/remounts between section switches, the container'sscrollTopcarries over from whichever section was scrolled last.Fix
settingsScrollRefref on the scroll containeruseLayoutEffectkeyed oncurrentSectionthat callssettingsScrollRef.current?.scrollTo({ top: 0 })— this runs synchronously before paint, so there's no visible flash of the old scroll positionThis is a single, self-contained, regression-testable UI fix — 3 edits total in
src/App.tsx(ref declaration, effect, ref attachment).Testing
bunx tsc --noEmitpasses cleanly with no new errorsScreenshots/Videos (if applicable)
N/A — behavioral fix, visually the difference is scroll position on section switch.
AI Assistance
If AI was used: