fix(ui): make overflow hidden to fix fitAddon when search bar #13324
Conversation
📝 WalkthroughWalkthroughThe update modifies the Svelte component template in two ways: it changes the prop passing for the Estimated code review effort1 (~2 minutes) Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/renderer/src/lib/ui/TerminalWindow.svelteOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'svelte-eslint-parser' imported from /eslint.config.mjs 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: Simon Rey <sfbrey+eqqe@gmail.com>
|
@cdrage can you test it please? I am a bit surprised by the solution I provided but it seems to work for me. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/renderer/src/lib/ui/TerminalWindow.svelte(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: typecheck
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: build website
- GitHub Check: macOS
- GitHub Check: smoke e2e tests (development)
- GitHub Check: unit tests / macos-15
- GitHub Check: smoke e2e tests (production)
- GitHub Check: unit tests / windows-2025
- GitHub Check: linter, formatters
- GitHub Check: Linux
- GitHub Check: Windows
🔇 Additional comments (1)
packages/renderer/src/lib/ui/TerminalWindow.svelte (1)
88-90: Prop-shorthand looks goodSwitching to Svelte’s
{terminal}shorthand is idiomatic and keeps the markup succinct; no behavioural change introduced.
| {/if} | ||
| <div class="{className} p-[5px] pr-0 bg-[var(--pd-terminal-background)]" role="term" bind:this={logsXtermDiv}></div> | ||
|
|
||
| <div class="{className} overflow-hidden p-[5px] pr-0 bg-[var(--pd-terminal-background)]" role="term" bind:this={logsXtermDiv}></div> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
overflow-hidden may unintentionally kill horizontal scroll / selection
overflow-hidden clips both axes. While it fixes the vertical overflow caused by the search bar, it also disables horizontal scroll and can truncate long log lines or break mouse-drag text selection past the viewport.
Consider restricting the clipping to the vertical axis only:
-<div class="{className} overflow-hidden p-[5px] pr-0 bg-[var(--pd-terminal-background)]" role="term" bind:this={logsXtermDiv}></div>
+<!-- Preserve horizontal scroll while hiding vertical overflow -->
+<div class="{className} overflow-y-hidden p-[5px] pr-0 bg-[var(--pd-terminal-background)]" role="term" bind:this={logsXtermDiv}></div>Please verify that horizontal scrolling / selection still works after this change, especially with very wide log lines.
I’ll inspect how the terminal is instantiated and whether horizontal overflow/wrapping is configured before adjusting the overflow utility. Gathering the snippet…
🏁 Script executed:
#!/bin/bash
# Locate and print the TerminalWindow.svelte around the template and script
FILE=$(fd --hidden --exclude node_modules TerminalWindow.svelte)
echo "File: $FILE"
# Print first 200 lines to capture template + script
sed -n '1,200p' "$FILE"
# Show where and how Xterm is constructed
grep -n "new Terminal" -n "$FILE"Length of output: 2710
Restrict terminal container overflow to vertical axis
The terminal’s outer <div> currently uses overflow-hidden, which prevents any horizontal overflow or extended text selection if wrapping is ever disabled or very long lines appear. Switching to overflow-y-hidden confines clipping to vertical overflow only while preserving horizontal overflow/selection.
Please update and verify:
-<div class="{className} overflow-hidden p-[5px] pr-0 bg-[var(--pd-terminal-background)]" role="term" bind:this={logsXtermDiv}></div>
+<div class="{className} overflow-y-hidden p-[5px] pr-0 bg-[var(--pd-terminal-background)]" role="term" bind:this={logsXtermDiv}></div>• Confirm that text selection and horizontal overflow still work correctly, especially if lineWrap is ever disabled or with very wide log lines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="{className} overflow-hidden p-[5px] pr-0 bg-[var(--pd-terminal-background)]" role="term" bind:this={logsXtermDiv}></div> | |
| <div class="{className} overflow-y-hidden p-[5px] pr-0 bg-[var(--pd-terminal-background)]" role="term" bind:this={logsXtermDiv}></div> |
🤖 Prompt for AI Agents
In packages/renderer/src/lib/ui/TerminalWindow.svelte at line 92, replace the
CSS class 'overflow-hidden' on the terminal container div with
'overflow-y-hidden' to restrict overflow clipping to the vertical axis only.
This change will allow horizontal overflow and text selection to function
properly, especially when line wrapping is disabled or with very long lines.
After updating, test the terminal to ensure horizontal scrolling and text
selection behave as expected.
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
I don't think so, maybe a e2e |
What does this PR do?
Fixes the logs display when there is a search bar.
When resizing the window, there was an issue where the height of the search controls was not taken into account overflowing to the status bar and making logs not visible.
Screenshot / video of UI
Screen.Recording.2025-07-23.at.17.37.15.mov
What issues does this PR fix or reference?
Fixes #13231
How to test this PR?