feat(tui): independent scroll regions for conversation and tool output#2113
feat(tui): independent scroll regions for conversation and tool output#2113ljm3790865 wants to merge 2 commits into
Conversation
…utput Split the chat area into two independent scroll regions: - Upper region: conversation/transcript (user, assistant, thinking) - Lower region: tool output (exec, exploring, patch, etc.) Each region has its own scroll state and cache, allowing independent scrolling. Mouse wheel events are routed to the appropriate region based on cursor position. This fixes the scroll demon issue where running tasks would cause the entire interface to scroll, blocking UI elements. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | DeepSeek API Key | c6d25e5 | pack_claude_final.js | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
There was a problem hiding this comment.
Code Review
This pull request implements a split-pane layout in the TUI to separate conversation history from tool outputs and introduces a comprehensive set of diagnostic and deployment scripts. Reviewers identified critical security risks involving hardcoded API keys in configuration and script files, as well as portability issues with absolute file paths. Technical feedback also addresses a layout bug where chat regions may overlap with the sidebar, a regression in scroll-management logic, and suggestions for refactoring duplicated code and improving UI responsiveness to terminal resizing.
| # Active provider + DeepSeek defaults | ||
| # ───────────────────────────────────────────────────────────────────────────────── | ||
| provider = "deepseek" | ||
| api_key = "sk-f1bccc35f03d4e90be027a54ef02399a" |
| # DeepSeek Platform (DeepSeek平台配置) | ||
| # ───────────────────────────────────────────────────────────────────────────────── | ||
| [providers.deepseek] | ||
| api_key = "sk-f1bccc35f03d4e90be027a54ef02399a" |
| $workDir = "$env:USERPROFILE\DeepSeek Tui" | ||
|
|
||
| # 设置环境 | ||
| $env:DEEPSEEK_API_KEY = "sk-f1bccc35f03d4e90be027a54ef02399a" |
| const envContent=[ | ||
| '# Claude Code -> DeepSeek API', | ||
| 'ANTHROPIC_BASE_URL=https://api.deepseek.com/v1', | ||
| 'ANTHROPIC_API_KEY=sk-f1bccc35f03d4e90be027a54ef02399a', |
| 'Write-Host "[4/5] Setting up DeepSeek API..."', | ||
| 'Copy-Item "$s\\.env" $u -Force', | ||
| '[Environment]::SetEnvironmentVariable("ANTHROPIC_BASE_URL","https://api.deepseek.com/v1","User")', | ||
| '[Environment]::SetEnvironmentVariable("ANTHROPIC_API_KEY","sk-f1bccc35f03d4e90be027a54ef02399a","User")', |
| let conversation_area = chat_layout[0]; | ||
| let tool_output_area = chat_layout[1]; |
There was a problem hiding this comment.
The conversation_area and tool_output_area are calculated here based on the full width of chunks[1]. However, later in the function (line 5819), a sidebar might be added which reduces the available width for the chat area. This will cause the chat widgets to overlap with the sidebar because they are rendered using these stale, full-width areas. These regions should be calculated from the finalized chat_area after the sidebar logic has been applied.
| if update.delta_lines != 0 { | ||
| app.user_scrolled_during_stream = true; | ||
| app.needs_redraw = true; | ||
| } | ||
| } | ||
| MouseEventKind::ScrollDown => { | ||
| let update = app.viewport.mouse_scroll.on_scroll(ScrollDirection::Down); | ||
| app.viewport.pending_scroll_delta = app | ||
| .viewport | ||
| .pending_scroll_delta | ||
| .saturating_add(update.delta_lines); | ||
| let pending_delta = if mouse_hits_tool_output_area(app, mouse) { | ||
| &mut app.viewport.pending_tool_scroll_delta | ||
| } else { | ||
| &mut app.viewport.pending_scroll_delta | ||
| }; | ||
| *pending_delta = pending_delta.saturating_add(update.delta_lines); | ||
| if update.delta_lines != 0 { | ||
| app.user_scrolled_during_stream = true; | ||
| app.needs_redraw = true; |
There was a problem hiding this comment.
The removal of app.user_scrolled_during_stream = true; appears to be a regression. This flag is typically used to disable auto-scrolling to the bottom when new content arrives while the user is manually inspecting previous history. While the goal is to fix the 'scroll demon', removing this flag entirely will cause the UI to jump to the bottom whenever the assistant or a tool produces output, even if the user is actively scrolling. This logic should likely be made region-aware instead of being deleted.
| .split(size); | ||
|
|
||
| // Split chat area into conversation (upper) and tool output (lower) regions | ||
| const TOOL_OUTPUT_DEFAULT_HEIGHT: u16 = 10; |
There was a problem hiding this comment.
Using a fixed height of 10 for the tool output region (TOOL_OUTPUT_DEFAULT_HEIGHT) can lead to poor UX on small terminal windows. If the total available height is small, the conversation region might become invisible or unusable. Consider making this height dynamic (e.g., a percentage of the total height) or allowing the user to resize it.
| fn mouse_hits_tool_output_area(app: &App, mouse: MouseEvent) -> bool { | ||
| if let Some(area) = app.viewport.tool_output_area { | ||
| return mouse.row >= area.y && mouse.row < area.y.saturating_add(area.height) | ||
| && mouse.column >= area.x && mouse.column < area.x.saturating_add(area.width); | ||
| } | ||
| false | ||
| } | ||
|
|
||
| /// Check if mouse is over the conversation area (upper region). | ||
| fn mouse_hits_conversation_area(app: &App, mouse: MouseEvent) -> bool { | ||
| if let Some(area) = app.viewport.conversation_area { | ||
| return mouse.row >= area.y && mouse.row < area.y.saturating_add(area.height) | ||
| && mouse.column >= area.x && mouse.column < area.x.saturating_add(area.width); | ||
| } | ||
| false | ||
| } |
There was a problem hiding this comment.
The logic for checking if a mouse event hits a specific area is duplicated. Additionally, mouse_hits_conversation_area is defined but never used in the current changes. Consider refactoring the area check into a helper function.
| fn mouse_hits_tool_output_area(app: &App, mouse: MouseEvent) -> bool { | |
| if let Some(area) = app.viewport.tool_output_area { | |
| return mouse.row >= area.y && mouse.row < area.y.saturating_add(area.height) | |
| && mouse.column >= area.x && mouse.column < area.x.saturating_add(area.width); | |
| } | |
| false | |
| } | |
| /// Check if mouse is over the conversation area (upper region). | |
| fn mouse_hits_conversation_area(app: &App, mouse: MouseEvent) -> bool { | |
| if let Some(area) = app.viewport.conversation_area { | |
| return mouse.row >= area.y && mouse.row < area.y.saturating_add(area.height) | |
| && mouse.column >= area.x && mouse.column < area.x.saturating_add(area.width); | |
| } | |
| false | |
| } | |
| fn mouse_hits_area(area: Rect, mouse: MouseEvent) -> bool { | |
| mouse.row >= area.y && mouse.row < area.y.saturating_add(area.height) | |
| && mouse.column >= area.x && mouse.column < area.x.saturating_add(area.width) | |
| } | |
| fn mouse_hits_tool_output_area(app: &App, mouse: MouseEvent) -> bool { | |
| app.viewport.tool_output_area.map_or(false, |area| mouse_hits_area(area, mouse)) | |
| } |
| set MSYS_NO_PATHCONV=1 | ||
| set MSYS2_ARG_CONV_EXCL=* | ||
| set PATH=C:\msys64\mingw64\bin;%PATH% | ||
| cd /d "C:\Users\ljm37\DeepSeek Tui" |
|
The GitGuardian failure is likely detecting what looks like an API key pattern in the test fixtures. This is usually a false positive — GitGuardian scans for secret patterns in code. Could you check if there's a base64-encoded string or key-like pattern in the PR diff that's triggering it? If it's a test-only placeholder, we can add a exclusion or mark it as a false positive. |
Summary
Split the chat area into two independent scroll regions:
Each region has its own scroll state and cache, allowing independent scrolling. Mouse wheel events are routed to the appropriate region based on cursor position.
This fixes the scroll demon issue where running tasks would cause the entire interface to scroll, blocking UI elements.
Changes
crates/tui/src/tui/app.rscrates/tui/src/tui/history.rscrates/tui/src/tui/widgets/mod.rscrates/tui/src/tui/ui.rscrates/tui/src/tui/mouse_ui.rsTest plan
cargo check --workspace(compiles successfully)?? Generated with Claude Code