sessions: drop the composer card, give each control its own surface - #334655
Conversation
5ff45d6 to
da1044f
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The screenshot fixture obscures its wallpaper, and the new control borders lack a reliable high-contrast treatment.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
src/vs/sessions/contrib/chat/browser/media/chatWidget.css — This newly added control frame is not reliable in high-contrast themes.… |
What changed in this PR
Aligns the new-session composer with started-session control surfaces.
Changes:
- Removes the shared frosted composer card.
- Adds opaque, individually framed controls with preserved hover tint.
- Adds CSS assertions and a background screenshot fixture.
| File | Description |
|---|---|
agentHostSessionConfigPicker.css |
Spaces independent configuration controls. |
newChatWidget.fixture.ts |
Adds the chat-background visual fixture. |
chatView.test.ts |
Tests composer surface styling. |
chatWidget.css |
Styles individual controls and hover states. |
chatView.css |
Removes the shared composer card. |
Suppressed comments (1)
src/vs/sessions/contrib/chat/test/browser/newChatWidget.fixture.ts:345
- This background fixture still assigns an opaque inline
background-colorto the.session-viewbelow. Inline style wins overchatBackground.css's.has-chat-background .session-view { background-color: transparent; }, so the codicon layer is covered and the new screenshot does not exercise the wallpaper/composer contrast described here. Leave the inline fill unset for this variant so the production transparency rule can apply.
});
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Screenshot ChangesBase: Changed (34)2 insignificant change(s) omitted (≤20 px, Δ≤2). See CI logs for details. Added (2) |
|
Also picked up the suppressed comment about the fixture's Right: the inline Wrong: that never hid the wallpaper. Fixed anyway in b837ad5, since the fixture was reproducing production's appearance through a different layer stack than production's. The opaque base now sits on |
|
Applied the regenerated manifest in 9c97f6e. Most of the entries are this PR repainting the composer, and the
Worth noting the report was generated from |
|
Correction on that stray entry — my read of it was wrong, and CI called it. I said Fixed in 7b6e566 by restoring the The first half of the old comment still holds: that fixture does not use any class this PR touches, and the removed Why it moves at all: it is the only |
Over a chat background the new-session composer sat on a single frosted card, while a started session gives every control its own surface. This makes the composer match the started session. - Delete `.new-chat-widget-content`'s card styling: the padding, blur, border, radius and shadow. - Give the bottom-row action labels and the workspace pills an opaque `--session-view-background` fill, and put a 2px gap between the agent host config controls now that they no longer share one surface. - Keep the hover tint opaque. `--vscode-toolbar-hoverBackground` is translucent, so it goes on `background-image` as a flat gradient and composites over the opaque fill instead of replacing it. - Frame the bottom-row chips when a chat background is set, the same move the in-session secondary toolbar makes in `chatView.css`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The chip frame over a chat background used commandCenter.inactiveBorder, which — unlike its sibling border tokens — registers no hcDark/hcLight value, so it stayed a 25%-alpha wash in high contrast. Put contrastBorder first, the way the rest of the codebase does: high-contrast themes define it, others fall through. The chat-background fixture painted its opaque base on .session-view, which production makes transparent so the wallpaper can show. Move that base onto the part instead. Both screenshots are byte-identical, since the wallpaper is positioned and always painted above a static background either way — but the fixture now stacks its layers the way the real window does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Regenerated by CI. Most entries are this PR repainting the composer, and the NewSessionChatBackground pair is the fixture it adds. One entry is not from here: chatInput/ResponsiveModelResizeCycleMinimal/Dark. #334021 landed after the manifest was last regenerated and changed chatView.css without refreshing it, so this PR is the first regeneration to pick that up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ResponsiveModelResizeCycleMinimal/Dark is not a fixture this branch touches, but the regenerated manifest I pasted carried a one-off hash for it. Put back the value main has held across ~20 regenerations, which is also what the latest run produced. That fixture is the only blocksCi one that sets virtualTime.enabled to false, and it drives 19 sequential resize widths on wall-clock time, so its final frame can land mid-settle on a loaded runner. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
7b6e566 to
78b28d2
Compare

Over a chat background the new-session composer sat on a single frosted card, while a started session gives every control its own surface. This makes the composer match the started session.
How it looks
The new
NewSessionChatBackgroundfixture, over the codicon wallpaper. The composer sits straight on the background with no card, the workspace pills and the input and each bottom-row chip carry their own opaque surface and a thin frame, and the wallpaper reads through the gaps between them.There is no CI-hosted "before" to pair these with, because the fixture is new here and no earlier one rendered the composer over a background. The card it replaces is the rule deleted from
chatView.cssin the diff: acolor-mixfill underbackdrop-filter: blur(...) saturate(1.08), plus a border, a large radius and a shadow, wrapped around the composer as a whole. The other fixtures in the CI screenshot report change only by a few pixels, since off a background the new chip fill matches what was already behind it.What changed
.new-chat-widget-content's card styling — the padding, blur, border, radius and shadow.--session-view-backgroundfill, and put a 2px gap between the agent host config controls now that they no longer share one surface.--vscode-toolbar-hoverBackgroundis translucent, so it goes onbackground-imageas a flat gradient and composites over the opaque fill instead of replacing it. The bottom-row hover rule also needs the.monaco-workbench.agent-sessions-workbenchprefix to outrank the shared action-bar hover in workbenchstyle.css.chatView.css. The frame leads with--vscode-contrastBorderand falls back to--vscode-commandCenter-inactiveBorder, so high contrast gets a solid stroke while every other theme matches the workspace pill directly above. Using thebutton-secondarypalette the in-session rule uses would have replaced the--session-view-backgroundfill.Verification
./scripts/test.sh --grep "Sessions - Chat"— 80 passing.chatView.test.tsnow covers the opaque fill and the conditional border on both the background and no-background paths.NewSessionChatBackgroundcomponent-explorer fixture, screenshotted and approved in light and dark.1px solid rgba(139, 148, 158, 0.25)/4pxradius /rgb(255, 255, 255)fill on every chip in light, andrgba(157, 157, 157, 0.25)/rgb(24, 24, 24)in dark. Without one:0px none, unchanged.🤖 Generated with Claude Code