feat: adds floating terminal layout#2344
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
c78fad9 to
df27669
Compare
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a new feature (floating terminal layout) with new user-facing behavior and UI components. Additionally, unresolved review comments identify substantive bugs: the restore defaults functionality doesn't reset the new terminal layout setting, and height clamping may not work correctly when switching between layouts. You can customize Macroscope's approvability policy. Learn more. |
df27669 to
2343d3d
Compare
2343d3d to
4796964
Compare
- add terminal layout setting and contract schema - render the thread terminal as a floating dialog when enabled - switch package scripts to run under Bun
f5b32fe to
7954880
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7954880. Configure here.
| settings.automaticGitFetchInterval, | ||
| settings.enableAssistantStreaming, | ||
| settings.sidebarThreadPreviewCount, | ||
| settings.terminalLayout, |
There was a problem hiding this comment.
Restore defaults omits terminalLayout from reset
Medium Severity
The changedSettingLabels memo correctly detects when terminalLayout differs from its default and includes "Terminal layout" in the list shown in the confirmation dialog. However, the restoreDefaults callback's updateSettings call does not include terminalLayout, so the setting is never actually reset. The user sees it promised as restored but it stays unchanged.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7954880. Configure here.
There was a problem hiding this comment.
🟠 High
The diff adds terminalLayout detection to changedSettingLabels (lines 414-416), so users see "Terminal layout" in the reset confirmation dialog, but restoreDefaults omits terminalLayout from the updateSettings call. The setting appears resettable but silently persists when confirmed.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/settings/SettingsPanels.tsx around line 467:
The diff adds `terminalLayout` detection to `changedSettingLabels` (lines 414-416), so users see "Terminal layout" in the reset confirmation dialog, but `restoreDefaults` omits `terminalLayout` from the `updateSettings` call. The setting appears resettable but silently persists when confirmed.
Evidence trail:
apps/web/src/components/settings/SettingsPanels.tsx lines 414-416 (terminalLayout in changedSettingLabels) and lines 467-480 (updateSettings call omitting terminalLayout) at REVIEWED_COMMIT.
| onHeightChangeRef.current(clampedHeight); | ||
| }, []); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
🟡 Medium components/ThreadTerminalDrawer.tsx:1034
When the layout prop changes from "floating" to "docked", the drawer height is not re-clamped to the new maximum. The effect at line 1034 only depends on [height, threadId], so it won't run when layout changes, leaving a height of 800px (valid for floating's 920px max) exceeding docked's 750px max.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/ThreadTerminalDrawer.tsx around line 1034:
When the `layout` prop changes from `"floating"` to `"docked"`, the drawer height is not re-clamped to the new maximum. The effect at line 1034 only depends on `[height, threadId]`, so it won't run when `layout` changes, leaving a height of 800px (valid for floating's 920px max) exceeding docked's 750px max.
Evidence trail:
apps/web/src/components/ThreadTerminalDrawer.tsx lines 55-56 (ratio constants: 0.75 and 0.92), line 64-68 (clampDrawerHeight function), lines 884-887 (maxHeightRatio derived from layout, ref updated but no effect triggered), lines 1034-1039 (effect depends on [height, threadId] only), lines 1086-1107 (window resize handler only fires on window resize, not layout change). No other effect in the file depends on `layout`.


Summary
Adds an option to have the terminal in a floating window
Settings page
floating terminal
Note
Medium Risk
Adds a new persisted client setting and expands per-thread terminal persisted state to include
terminalWidth, which could impact settings/state decoding and UI behavior across sessions. Main changes are UI/layout-related, but touch shared contracts and state persistence paths.Overview
Adds a new
terminalLayoutclient setting (docked|floating) and wires it throughChatView/ChatHeaderso the terminal can render either as the existing docked drawer or as a modal-like floating window.When
floatingis selected, the terminal is shown in a backdrop overlay with outside-click/close-button dismissal and resizable width; terminal state persistence is extended to storeterminalWidth, andThreadTerminalDraweradjusts its max height/border styling based on layout.Updates the Settings panel to let users choose/reset terminal layout, and updates tests/initial browser terminal state to include the new fields/defaults.
Reviewed by Cursor Bugbot for commit 7954880. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add floating terminal layout option as an alternative to the docked drawer
TerminalLayouttype ('docked'|'floating') to client settings in settings.ts, defaulting to'docked'.'floating', the terminal renders as a centered modal dialog with a backdrop, close button, and left/right resize handles instead of the bottom drawer.📊 Macroscope summarized 7954880. 8 files reviewed, 3 issues evaluated, 0 issues filtered, 2 comments posted
🗂️ Filtered Issues