feat: Dynamic multi-terminal UI with split-pane support#6
Conversation
shreyas-lyzr
left a comment
shreyas-lyzr
left a comment
There was a problem hiding this comment.
Large feature PR — multi-terminal UI with split-pane support. The concept is solid and addresses a real gap for power users. A few things need attention before this is merge-ready:
Blocking — dependency regression
The package-lock.json downgrades monaco-editor from ^0.55.1 to ^0.53.0 and bumps vite from ^5.2.0 to ^8.0.1. The vite major bump is especially risky — vite 8 introduced breaking config changes. If this PR was developed on a different base branch, please rebase onto current main and re-lock. The lockfile changes will likely conflict once rebased.
New clawcontainer dependency
clawcontainer was added without explanation in the PR body. What does it provide and is it a trusted/internal package? A note on this would help reviewers.
Split-pane state on page reload
The new tab/pane state appears to live entirely in JS memory. If the user reloads, all open terminals and their layout disappear. Is that the intended behavior, or should open sessions persist in localStorage?
Accessibility
The new tab buttons use emoji characters (⚙, ＋, ⊞) as their visible label with no aria-label. Screen readers will get meaningless characters. Add aria-label attributes to the interactive buttons.
Terminal lifecycle
When a tab is closed, is the associated WebContainer process explicitly terminated? If not, processes will leak silently in the background.
The overall direction is good. Addressing the dependency regression and clarifying the clawcontainer addition are the minimum requirements before merge.
shreyas-lyzr
left a comment
shreyas-lyzr
left a comment
There was a problem hiding this comment.
Large, well-structured PR with a clear purpose. The multi-terminal tab system is a meaningful UX improvement and the code is generally solid. A few issues need attention before merge.
Blocking: onData listener accumulation in spawnShell
In container.ts, spawnShell registers terminal.onData(...) without disposing the previous one. Because TerminalManager.onData in this PR does dispose-before-register (via onDataDisposable), each new shell spawned to the same terminal will silently detach the previous shell's listener and attach the new one. This means switching between shells that share a TerminalManager instance will break input routing for all but the last-spawned shell. If each dynamic tab gets its own TerminalManager instance, this is fine — but that invariant is not enforced or documented, and the Map<string, TerminalManager> in UIManager suggests it might not always hold.
Blocking: window.addEventListener('resize') leak in spawnShell
Each call to spawnShell adds a new resize event listener on window with no corresponding removal. Spawning N shells registers N listeners, all of which survive even after the shell is closed. After a few sessions this adds up. Store the listener and remove it when the shell is killed, or use the ResizeObserver pattern already used in TerminalManager.
Concern: clawcontainer added as a production dependency
package.json adds clawcontainer: ^1.1.0 to dependencies. The diff does not show any direct import of this package in the source files being changed. If it is only used internally by spawnShell or if it is an internal package, that needs to be documented. If it is unused, remove it to avoid unnecessary bundle bloat.
Concern: vite 5 → 8 major bump without documented rationale
The package.json bump from vite ^5.2.0 to vite ^8.0.1 is a two-major-version jump. Vite 8 requires Node >=20.19.0 || >=22.12.0 and switches from esbuild to rolldown. This changes the minimum runtime requirement for contributors and may affect the WebContainer boot environment. This should be explicitly called out in the PR description and ideally confirmed against the CI node version.
Minor: monaco-editor downgraded from ^0.55.1 to ^0.53.0
This is a regression — going backwards on a dependency is unusual and may re-introduce bugs that were fixed between those versions. The reason is not mentioned in the PR description. Please justify or revert.
Minor: CSS split-pane specificity
The rule #terminal-panes.split-active .term-pane.active { display: flex; } and #terminal-panes.split-active #terminal-pane-split { display: flex !important; } mixes a specificity-won selector with !important. Prefer consistent specificity throughout; the !important here is a code smell that will make future layout overrides harder.
…e versions - Drop clawcontainer: ^1.1.0 — never imported in src/, only used as a window property name - Restore monaco-editor to ^0.55.1 (was downgraded to ^0.53.0 on branch) - Restore vite to ^5.2.0 (was bumped to ^8.0.1 on branch) - Regenerate package-lock.json cleanly
- TerminalManager.onData() now returns its IDisposable so callers can hold a reference and dispose it when the shell exits. - spawnShell stores the onData disposable and resize handler in per-id maps (shellOnDataDisposables, shellResizeHandlers). - A proc.exit handler cleans up both listeners when the shell exits naturally, via the new _cleanupShellListeners() helper. - killShell() disposes the onData listener and resize handler immediately, then calls proc.kill() whose exit resolution handles the rest — no double-decrement of activeProcessCount.
The tab close handler now calls container.killShell(id) before removing the pane, so the jsh process inside the WebContainer is explicitly killed instead of being abandoned. Also calls tm.dispose() to clean up the xterm instance.
Screen readers cannot infer meaning from the bare emoji characters (⚙, ＋, ⊞). Add aria-label to the two <button> elements and role=tab plus aria-label to the Agent tab <div> so assistive technology announces them correctly.
The previous rule used a high-specificity ID selector combined with !important to force display:flex on the split pane, which was both unnecessary (ID specificity already wins over .term-pane) and inconsistent with the other two rules in the same block. Replace all three split-active rules with uniform class-based selectors (.split-active .term-pane-split, .split-active .resize-split) and add the corresponding classes to the elements in index.html. No !important is needed — two-class specificity cleanly overrides the single-class .term-pane base rule.
VivanRajath
force-pushed
the
feature/ui-improvements
branch
from
f4cb061 to
1d5f3b8
Compare
…terminal killShell now deletes from shellProcesses before calling kill(), so the exit promise handler can use shellProcesses.has(id) as a sentinel to short-circuit and avoid double-decrement of activeProcessCount and double-disposal of listeners.
- Rename handler parameter from `code` to `exitCode` so it reads as
the plain number WebContainer resolves to, with no Number() wrapper.
- Add .catch(()=>{}) to proc.output.pipeTo() so the rejection that
fires when the process exits and closes the stream is not left
unhandled (could surface as a misleading error in devtools).
jsh crashes with an internal exitCode error when it tries to resolve the exit code of the clear(1) command. Erase the xterm display with ESC[2J ESC[H directly instead so the shell starts clean without jsh ever running clear as a child process.
All open terminal panes are now visible side-by-side simultaneously, like VS Code's terminal split view: - Each pane runs its own live shell and xterm instance. - Clicking a pane (or its tab) focuses it with a blue-outline highlight; keystrokes go to that pane's shell via xterm's native focus — the per-shell onData disposables in ContainerManager are not touched. - Draggable resize handles are inserted between adjacent panes on add. - Closing a pane removes it and its adjacent handle, then resets remaining panes to equal flex so they re-tile evenly. - The + and split (⊞) buttons both add a new tiled pane. - Removed the static #terminal-pane-split / #resize-terminal-split slots and .split-active CSS rules; everything is fully dynamic now.
|
Hi @shreyas-lyzr, sorry for the long delay here, I got pulled into some other projects and only just got back to this. Thanks for the detailed review, it genuinely helped tighten this up. I've rebased onto current Dependency regressions: After rebasing onto latest
Terminal lifecycle: Accessibility: Added CSS specificity: Dropped the Split view improvement: Following the reload/layout discussion, I reworked the split from a single swap slot into proper multi-pane tiling, closer to VS Code. Each open terminal can now render as its own live pane side by side, the focused pane is highlighted and receives keystrokes, panes are resizable via drag handles between them, and closing a pane kills its shell and re-tiles the rest. While testing I also fixed a runtime crash where On reload persistence: open sessions are still intentionally ephemeral and don't survive a reload. Happy to add Happy to make any other changes if something still looks off. |
2 participants
Summary
Adds a fully dynamic multi-terminal interface to the ClawLess terminal panel. Users can now open multiple independent shell sessions, rename them inline, close them individually, and view them in a split-pane layout alongside the Agent terminal — all without any page reload.
Motivation
The original UI had a single fixed terminal pane tied to the agent. Power users working with ClawLess often need a separate shell for running commands while the agent is active. This PR addresses that gap with a scalable, tab-based terminal system.
File Changes
shellProcess2/shellWriter2fields with aMap<string, WebContainerProcess>and a generic spawnShell(id, terminal) method supporting unlimited concurrent shells_terminal2andstartShell2calls⚙ Agenttab +＋button +⊞split button; cleaner pane structure＋button, inline rename viacontenteditable, scrollable overflow tab bar, split-pane layoutHow to Test
Environment: Windows 11, Node.js v20, Chrome 123
npm install && npm run devhttp://localhost:5173in ChromeTab creation:
＋→ "Terminal 1" tab appears; a livejshshell launches inside the WebContainer＋again → "Terminal 2" opens with another independent shellRename:
Close:
×button appears×→ tab and its shell pane are removed; view switches back to AgentSplit view:
⊞→ terminal panel splits: active shell on right, Agent on left⊞again → returns to single-pane viewNo regressions:
npm run buildpasses with zero TypeScript errorsChecklist
npm run buildpasses