Skip to content

Commit c225a74

Browse files
authored
🤖 fix: fade sideways-scrolling rows, full skill descriptions, keep pop-out terminals out of the sidebar (#5732)
## Summary Three small layout and terminal fixes from the bug bash: - Rows that scroll sideways with a hidden scrollbar (workspace footer, Timeline filter chips) now fade the edge where more items sit past it. - The Skills popover shows each skill's full description instead of one line. - A terminal open in a pop-out window no longer comes back as a (often hidden) right-sidebar tab when the main window reloads. In the desktop app, closing the pop-out window ends its session. Fixes #5696 Fixes #5697 Fixes #5673 ## Implementation **#5696** One shared CSS class, `scroll-fade-x`, in `scrollbar-none.css` (shared with the VS Code webview, because both rows are webview-importable). It is scroll-driven CSS (`animation-timeline: scroll(self inline)` animating two registered `@property` lengths into a `mask-image`), so there is no JS and no per-render work. A row that fits has an inactive scroll timeline and shows no fade. Browsers without scroll timelines keep today's plain row. **#5697** The description drops `line-clamp-1` and wraps (`break-words`). The list already scrolls inside `max-h-[min(400px,60vh)]`. I chose full text over click-to-expand because expanding would be a new operation that needs its own keyboard shortcut. **#5673** (coordinator-approved design, no persisted state): 1. Pop-out terminal windows mount `TerminalRouterProvider popout`, so their `terminal.attach` calls send `popout: true` (new optional input field, no new subscription). 2. While such an attach stream is open, `TerminalService` counts the session as shown in a pop-out, and `terminal.listSessions` skips it. So the right sidebar's reload sync (and layout presets) do not adopt it. 3. The count drops when the stream ends: the pop-out closed, crashed, reloaded or lost its connection. A backend restart starts with no counts. 4. Desktop: `TerminalWindowManager` reports a closed pop-out window, and `TerminalService` closes that session. This matches closing a sidebar terminal tab, which also does not ask first. Reloading or crashing a pop-out does not close its window, so the session survives those. 5. Browser mode (documented decision): a pop-out page cannot tell its window closing from its own reload, so closing a browser pop-out does not end the session. Killing it would lose the shell on every pop-out reload. After the pop-out closes, the session returns as a sidebar tab on the next main-window load, which is today's recovery path. ## Validation Test-first for #5673. Before the fix: ``` TypeError: service.markPopoutAttached is not a function (fail) TerminalService > pop-out attachments (#5673) > a session with a live pop-out is not listed, so the sidebar does not adopt it (fail) TerminalService > pop-out attachments (#5673) > the session stays hidden until every pop-out attach for it has ended (fail) TerminalService > pop-out attachments (#5673) > closing the desktop pop-out window ends its session ``` `routerSubscriptions.test.ts` adds the stream-level test (reload-adoption and crash path): a pop-out attach hides the session while it is live, and aborting it lists the session again. If `attachTerminal` ignores the flag, the test fails. `vscode/src/webview/webviewCss.test.ts` passes (it first caught the class living only in desktop CSS, so the class moved to the shared file). #5696 and #5697 are CSS-only. Asserting class names would be a tautological test, so the evidence is measured in the browser (agent-browser against `tests/bugbash/startApp.ts` servers built from main and from this branch): | Check | main | this branch | |---|---|---| | Footer row at 375px (`scrollWidth` 430 > `clientWidth` 375) | cut at the edge, no hint | right edge fades; after scrolling to the end the left edge fades | | Timeline chip row in the default sidebar (438 > 373) | cut chip, no hint | edge fades | | Skills popover: first 10 description spans clipped | 9 of 10 | 0 of 10 (desktop and 375px) | | Main-window reload with a browser pop-out terminal open | session added as a sidebar Terminal tab | no tab added | | Main-window reload after that browser pop-out is closed | tab | tab (session comes back, as designed) | Footer at 375px: main (top), branch at the start (middle), branch scrolled to the end (bottom): ![Footer row at 375px](https://github.com/user-attachments/assets/c5ea28a1-837d-4641-b5af-2d8eccd24d09) Timeline chips (row narrowed to 240px, as in a narrow sidebar): main (top), branch (bottom): ![Timeline chip row](https://github.com/user-attachments/assets/e3322735-f84c-48e4-97df-ac999a539a7f) Skills popover on main (desktop) and on this branch (375px): ![Skills popover on main](https://github.com/user-attachments/assets/1ca23732-34ef-4949-9c47-ac6e72f98678) ![Skills popover on this branch at 375px](https://github.com/user-attachments/assets/4a4195c4-3bba-4874-8902-f1ec8ac500e4) ## Risks Low. The fade only paints a mask on two rows and needs scroll-timeline support (Chromium, so Electron and the VS Code webview). Long skill descriptions make the popover list taller, and it scrolls. `listSessions` now hides sessions that a pop-out shows, so the layout-preset terminal mapping also skips them, which matches the intent. Closing a desktop pop-out now kills its shell. Before, the shell kept running unseen until the sidebar adopted it. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$3.19`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high -->
1 parent d084763 commit c225a74

14 files changed

Lines changed: 236 additions & 14 deletions

File tree

‎src/browser/components/ChatPane/WorkspaceFooterBar.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ export const WorkspaceFooterBar: React.FC<WorkspaceFooterBarProps> = (props) =>
400400
>
401401
{/* min-h rather than a fixed height: mobile raises these buttons to 44px touch targets, and a
402402
capped row would clip them along the same axis overflow-x-auto makes scrollable. */}
403-
<div className="scrollbar-none flex min-h-7 items-center gap-2 overflow-x-auto px-2 text-xs whitespace-nowrap">
403+
<div className="scrollbar-none scroll-fade-x flex min-h-7 items-center gap-2 overflow-x-auto px-2 text-xs whitespace-nowrap">
404404
<RuntimeBadge
405405
runtimeConfig={props.runtimeConfig}
406406
isWorking={isWorking}

‎src/browser/components/SkillIndicator/SkillIndicator.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ const SkillsPopoverContent: React.FC<SkillsPopoverContentProps> = (props) => {
9191
)}
9292
{isLoaded && <Check className="text-success ml-1 inline h-3 w-3" />}
9393
</span>
94-
<span className="text-muted-foreground line-clamp-1 text-[11px] leading-snug">
94+
{/* Full text (#5697): the list already scrolls, and a clamp left no way to read
95+
the rest. */}
96+
<span className="text-muted-foreground text-[11px] leading-snug break-words">
9597
{skill.description}
9698
</span>
9799
</div>

‎src/browser/features/RightSidebar/Timeline/TimelinePanel.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,7 @@ export function TimelinePanelView(props: TimelinePanelViewProps) {
813813
}}
814814
>
815815
<div className="border-border shrink-0 border-b px-3 py-2.5">
816-
<div className="scrollbar-none flex min-w-0 gap-1.5 overflow-x-auto">
816+
<div className="scrollbar-none scroll-fade-x flex min-w-0 gap-1.5 overflow-x-auto">
817817
{FILTERS.map((item) => (
818818
<button
819819
key={item.value}

‎src/browser/styles/scrollbar-none.css‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,49 @@
1212
scrollbar-width: none;
1313
scrollbar-color: auto;
1414
}
15+
16+
/*
17+
* Edge fade for rows that scroll sideways with a hidden scrollbar (#5696): an edge fades out
18+
* while more items sit past it, so cut-off items do not look missing. Scroll-driven CSS, no JS:
19+
* a row that fits has an inactive scroll timeline and shows no fade, and browsers without scroll
20+
* timelines keep the plain row.
21+
*/
22+
@property --scroll-fade-start {
23+
syntax: "<length>";
24+
inherits: false;
25+
initial-value: 0px;
26+
}
27+
@property --scroll-fade-end {
28+
syntax: "<length>";
29+
inherits: false;
30+
initial-value: 0px;
31+
}
32+
@keyframes scroll-fade-x {
33+
0% {
34+
--scroll-fade-start: 0px;
35+
--scroll-fade-end: 24px;
36+
}
37+
10%,
38+
90% {
39+
--scroll-fade-start: 24px;
40+
--scroll-fade-end: 24px;
41+
}
42+
100% {
43+
--scroll-fade-start: 24px;
44+
--scroll-fade-end: 0px;
45+
}
46+
}
47+
@supports (animation-timeline: scroll()) {
48+
.scroll-fade-x {
49+
mask-image: linear-gradient(
50+
to right,
51+
transparent,
52+
#000 var(--scroll-fade-start),
53+
#000 calc(100% - var(--scroll-fade-end)),
54+
transparent
55+
);
56+
animation: scroll-fade-x linear both;
57+
/* After the shorthand, which resets the timeline. */
58+
animation-timeline: scroll(self inline);
59+
}
60+
}

‎src/browser/terminal-window.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ if (!workspaceId || !sessionId) {
6363
// race conditions with WebSocket connections and terminal lifecycle
6464
ReactDOM.createRoot(document.getElementById("root")!).render(
6565
<APIProvider>
66-
<TerminalRouterProvider>
66+
<TerminalRouterProvider popout>
6767
<TerminalWindowContent
6868
workspaceId={workspaceId}
6969
sessionId={sessionId}

‎src/browser/terminal/TerminalRouterContext.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ const TerminalRouterContext = createContext<TerminalSessionRouter | null>(null);
1313

1414
interface TerminalRouterProviderProps {
1515
children: React.ReactNode;
16+
/** Set in pop-out terminal windows (terminal-window.tsx). */
17+
popout?: boolean;
1618
}
1719

1820
/**
@@ -35,12 +37,12 @@ export function TerminalRouterProvider(props: TerminalRouterProviderProps) {
3537
}
3638

3739
// Create/cleanup after commit to avoid render-time disposal in concurrent mode.
38-
const nextRouter = new TerminalSessionRouter(api);
40+
const nextRouter = new TerminalSessionRouter(api, { popout: props.popout === true });
3941
setRouter(nextRouter);
4042
return () => {
4143
nextRouter.dispose();
4244
};
43-
}, [api]);
45+
}, [api, props.popout]);
4446

4547
const routerForContext = api && router?.getApi() === api ? router : null;
4648

‎src/browser/terminal/TerminalSessionRouter.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,12 @@ export class TerminalSessionRouter {
5151
private readonly api: APIClient;
5252
private sessions = new Map<string, SessionState>();
5353

54-
constructor(api: APIClient) {
54+
/** True in a pop-out terminal window: its attaches keep the session out of the sidebar. */
55+
private readonly popout: boolean;
56+
57+
constructor(api: APIClient, options?: { popout?: boolean }) {
5558
this.api = api;
59+
this.popout = options?.popout === true;
5660
}
5761

5862
/** Get the API client (for identity comparison when recreating router) */
@@ -287,7 +291,10 @@ export class TerminalSessionRouter {
287291
// Start attach stream (fire-and-forget, but managed by abort controller)
288292
void (async () => {
289293
try {
290-
const iterator = await this.api.terminal.attach({ sessionId }, { signal });
294+
const iterator = await this.api.terminal.attach(
295+
this.popout ? { sessionId, popout: true } : { sessionId },
296+
{ signal }
297+
);
291298
for await (const msg of iterator) {
292299
// Check if session was removed (unsubscribed)
293300
const currentSession = this.sessions.get(sessionId);

‎src/common/orpc/schemas/api.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2907,7 +2907,11 @@ export const terminal = {
29072907
* Guarantees no missed output between state snapshot and live stream.
29082908
*/
29092909
attach: {
2910-
input: z.object({ sessionId: z.string() }),
2910+
input: z.object({
2911+
sessionId: z.string(),
2912+
/** Set by pop-out terminal windows; their sessions stay out of listSessions meanwhile. */
2913+
popout: z.boolean().nullish(),
2914+
}),
29112915
output: eventIterator(
29122916
z.discriminatedUnion("type", [
29132917
z.object({ type: z.literal("screenState"), data: z.string() }),

‎src/desktop/terminalWindowManager.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ export class TerminalWindowManager {
2424
private windows = new Map<string, Set<BrowserWindow>>(); // workspaceId -> Set of windows
2525
private windowCount = 0; // Counter for unique window IDs
2626
private readonly config: Config;
27+
private onSessionWindowClosed: ((sessionId: string, closedBy: "user" | "app") => void) | null =
28+
null;
29+
/** Windows closed by closeTerminalWindow rather than by the user. */
30+
private readonly appClosedWindows = new WeakSet<BrowserWindow>();
2731

2832
constructor(
2933
config: Config,
@@ -34,6 +38,13 @@ export class TerminalWindowManager {
3438
this.config = config;
3539
}
3640

41+
/** Called with the session ID when a pop-out window that showed a session has closed. */
42+
setSessionWindowClosedHandler(
43+
handler: (sessionId: string, closedBy: "user" | "app") => void
44+
): void {
45+
this.onSessionWindowClosed = handler;
46+
}
47+
3748
/**
3849
* Open a new terminal window for a workspace
3950
* Multiple windows can be open for the same workspace
@@ -117,6 +128,18 @@ export class TerminalWindowManager {
117128
}
118129
}
119130
log.info(`Terminal window ${windowId} closed for workspace: ${workspaceId}`);
131+
// 'closed' fires only when the window really goes away: its reload and a renderer crash
132+
// keep the window, so the session survives those.
133+
if (sessionId) {
134+
try {
135+
this.onSessionWindowClosed?.(
136+
sessionId,
137+
this.appClosedWindows.has(terminalWindow) ? "app" : "user"
138+
);
139+
} catch (err) {
140+
log.error(`Failed to end terminal session ${sessionId} after its window closed:`, err);
141+
}
142+
}
120143
});
121144

122145
// Load the terminal page
@@ -158,6 +181,7 @@ export class TerminalWindowManager {
158181
if (windowSet) {
159182
for (const window of windowSet) {
160183
if (!window.isDestroyed()) {
184+
this.appClosedWindows.add(window);
161185
window.close();
162186
}
163187
}

‎src/node/orpc/router.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2492,7 +2492,9 @@ export const router = (authToken?: string) => {
24922492
attach: t
24932493
.input(schemas.terminal.attach.input)
24942494
.output(schemas.terminal.attach.output)
2495-
.handler(({ context, input, signal }) => attachTerminal(context, input.sessionId, signal)),
2495+
.handler(({ context, input, signal }) =>
2496+
attachTerminal(context, input.sessionId, signal, input.popout === true)
2497+
),
24962498
onExit: t
24972499
.input(schemas.terminal.onExit.input)
24982500
.output(schemas.terminal.onExit.output)

0 commit comments

Comments
 (0)