mobile: styling updates and smoother animation#312441
Merged
Conversation
Remove 22 !important declarations from mobileChatShell.css where the .phone-layout class already wins by specificity. Kept !important only where it fights SplitView inline styles, Part.layoutContents inline sizing, or an equal-specificity desktop rule. Added a top-of-file policy comment documenting the three legitimate reasons to use !important here so future additions don't accrete out of habit. Verified against a 393x852 iPhone emulation with the mock agent host: welcome screen, sidebar overlay, session opening, and chat input layout are unchanged. Also updated the vscode-dev-workbench skill to call out that npm run dev must run in the vscode-dev folder (not vscode) and that the mock agent host is a separate process that must be started in addition to the dev server.
Replace the hamburger icon (Codicon.menu) in the mobile titlebar with the agents-app sidebar toggle icon (layoutSidebarLeftOff / layoutSidebarLeft), matching desktop/web. The icon flips based on SideBarVisibleContext, and the aria-label toggles between "Open sessions" and "Close sessions". Make the mobile sidebar cover the full viewport width below the top bar instead of an 85%/360px drawer with scrim. The toggle button in the top bar stays visible and is the dismiss affordance. Removes the no-longer-needed backdrop element and its CSS.
The previous code wrote inline position/top/left/width/height/zIndex on the sidebar Part container from layoutMobileSidebar(). Those styles were applied AFTER the grid had already made the wrapper visible and the browser had painted one frame, producing a visible snap on every toggle. The geometry was redundant with the existing layout anyway: the mobile top bar is a flex sibling above the grid, so the grid (and therefore the .split-view-view wrapper which fills the grid via CSS) is already correctly positioned below the top bar. The Part fills the wrapper. No inline positioning is needed. Drop the inline style writes; keep the mobile-overlay-sidebar class toggle (used for background) and the explicit sidebarPart.layout() call so the Part's internal sizing matches the drawer dimensions even on the first paint.
rebornix
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Removes post-layout inline geometry writes for the sessions mobile sidebar drawer to prevent a one-frame “snap” during open/close, shifting sizing/positioning responsibility back to CSS while ensuring the sidebar Part still lays out correctly on first paint.
Changes:
- Drops inline
position/top/left/width/height/zIndexupdates fromlayoutMobileSidebar()and relies on CSS-driven geometry. - Updates the mobile titlebar “hamburger” to reflect sidebar open/closed state via
SideBarVisibleContext(icon + aria-label). - Refines phone-layout CSS (including a documented
!importantpolicy) and updates sidebar drawer overlay styling.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/browser/workbench.ts | Removes inline sidebar drawer geometry writes and backdrop handling; keeps explicit sidebar layout() to match CSS sizing. |
| src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts | Makes the sidebar toggle icon/label react to SideBarVisibleContext. |
| src/vs/sessions/browser/parts/mobile/mobileChatShell.css | Documents !important usage policy and reduces !important usage in phone-layout rules. |
| src/vs/sessions/browser/parts/media/sidebarPart.css | Adjusts phone sidebar overlay drawer styling and removes JS-driven backdrop styling. |
| .github/skills/vscode-dev-workbench/SKILL.md | Clarifies local dev server/mock-agent-host startup instructions for Agents window testing. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 2
osortega
approved these changes
Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous code wrote inline position/top/left/width/height/zIndex on the sidebar Part container from layoutMobileSidebar(). Those styles were applied AFTER the grid had already made the wrapper visible and the browser had painted one frame, producing a visible snap on every toggle.
The geometry was redundant with the existing layout anyway: the mobile top bar is a flex sibling above the grid, so the grid (and therefore the .split-view-view wrapper which fills the grid via CSS) is already correctly positioned below the top bar. The Part fills the wrapper. No inline positioning is needed.
Drop the inline style writes; keep the mobile-overlay-sidebar class toggle (used for background) and the explicit sidebarPart.layout() call so the Part's internal sizing matches the drawer dimensions even on the first paint.