From 86601591bdc2a90216b122cb4e8891d2a19eb6fd Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Fri, 24 Apr 2026 14:31:45 -0700 Subject: [PATCH] mobile: drop unneeded !important from phone-layout CSS 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. --- .github/skills/vscode-dev-workbench/SKILL.md | 20 ++++-- .../browser/parts/mobile/mobileChatShell.css | 66 +++++++++++-------- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/.github/skills/vscode-dev-workbench/SKILL.md b/.github/skills/vscode-dev-workbench/SKILL.md index 01589c1716c..93716893ca0 100644 --- a/.github/skills/vscode-dev-workbench/SKILL.md +++ b/.github/skills/vscode-dev-workbench/SKILL.md @@ -21,12 +21,20 @@ If your paths differ, check `server/` in `vscode-dev` for the source root resolu ## Start the dev server +**Critical:** Run `npm run dev` from the **`vscode-dev`** folder, NOT from `vscode`. The `vscode` repo has no `dev` script and will fail with `npm error Missing script: "dev"`. Terminal tools that simplify/strip leading `cd` into separate commands will silently keep the cwd of a previous terminal — always use an absolute `pushd` or verify with `pwd` before `npm run dev`. + ```bash -cd vscode-dev -npm run dev # runs watch + nodemon; serves https://127.0.0.1:3000 +cd /path/to/vscode-dev # NOT /path/to/vscode +npm run dev # runs watch + nodemon; serves https://127.0.0.1:3000 ``` -On first start you may see one crash like `Cannot find module './indexes'` — it's the watcher racing the first build. nodemon restarts automatically once `out/` finishes compiling. +If you're driving this through an agent/terminal tool, prefer: + +```bash +pushd /absolute/path/to/vscode-dev >/dev/null && pwd && npm run dev +``` + +On first start you may see one crash like `Cannot find module './indexes'` — it's the watcher racing the first build. nodemon restarts automatically once `out/` finishes compiling. The server is ready when `curl -sk -o /dev/null -w "%{http_code}" https://127.0.0.1:3000/` returns `200`. ## URLs @@ -97,15 +105,19 @@ For a true mobile viewport, drive a standalone Playwright script with `devices[' ## Testing the Agents window against a local mock agent host +If the scenario touches the Agents window (`/agents` route), you almost always need the mock agent host running. Without it, the Agents window will sit on the sign-in / tunnel-discovery screen and block any real interaction. Start it **in addition to** the dev server — it's a second terminal, not a replacement. + `vscode-dev` supports a `?mock-agent-host=ws://…` URL parameter that short-circuits tunnel discovery and wires the Agents window to a raw WebSocket. Pair it with the mock agent host binary from `microsoft/vscode`: ```bash -cd vscode +cd /path/to/vscode node out/vs/platform/agentHost/node/agentHostServerMain.js \ --enable-mock-agent --quiet --without-connection-token --port 8765 # Listens on ws://localhost:8765 ``` +Prerequisite: `out/` in the `vscode` repo must be populated by the `VS Code - Build` task (or `npm run watch`). If `out/vs/platform/agentHost/node/agentHostServerMain.js` is missing, start that task first. + `--enable-mock-agent` registers the `ScriptedMockAgent` from `src/vs/platform/agentHost/test/node/mockAgent.ts` with one pre-existing session. Seed additional sessions via the `VSCODE_AGENT_HOST_MOCK_SEED_SESSIONS` env var, using a comma-separated list of session URIs (for example, `VSCODE_AGENT_HOST_MOCK_SEED_SESSIONS=mock://pre-1,mock://pre-2`). Scripted prompts include `hello`, `use-tool`, `error`, `permission`, `write-file`, `run-safe-command`, `slow`, `client-tool`, `subagent`, etc. (see `mockAgent.ts` for the full list). Then open: diff --git a/src/vs/sessions/browser/parts/mobile/mobileChatShell.css b/src/vs/sessions/browser/parts/mobile/mobileChatShell.css index 72eddfcfe05..6bc47480f18 100644 --- a/src/vs/sessions/browser/parts/mobile/mobileChatShell.css +++ b/src/vs/sessions/browser/parts/mobile/mobileChatShell.css @@ -3,6 +3,21 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +/* + * `!important` policy for this file: + * + * Only use `!important` when fighting one of: + * 1. SplitView.View.layoutContainer — sets inline position/top/left/width/height + * on every `.split-view-view` (src/vs/base/browser/ui/splitview/splitview.ts). + * 2. Part.layoutContents — inlines width/height on `.part > .content` via size(). + * 3. An equal-specificity desktop rule in the main workbench stylesheet. + * + * Rules that only face lower-specificity desktop CSS (e.g. a single `.part.X` + * selector) do NOT need `!important` — the `.phone-layout` class on the + * workbench root already wins. When adding a new rule, omit `!important` + * first and only add it if DevTools shows an actual override. + */ + /* ---- Mobile Top Bar ---- */ .mobile-top-bar { @@ -114,9 +129,9 @@ /* On phone, stack the mobile top bar and grid vertically */ .agent-sessions-workbench.phone-layout { - display: flex !important; - flex-direction: column !important; - overflow: hidden !important; + display: flex; + flex-direction: column; + overflow: hidden; } /* On phone, split-view-views that directly contain a Part fill the full @@ -175,34 +190,33 @@ height: 100% !important; } -/* Remove card appearance from ALL parts on phone */ +/* Remove card appearance from ALL parts on phone. + Specificity wins over the desktop card rule in style.css without !important; + width/height match what the mobile Part.layout() already inlines. */ .agent-sessions-workbench.phone-layout .part.chatbar, .agent-sessions-workbench.phone-layout .part.sidebar, .agent-sessions-workbench.phone-layout .part.auxiliarybar, .agent-sessions-workbench.phone-layout .part.panel { - margin: 0 !important; - border: none !important; - border-radius: 0 !important; - box-shadow: none !important; - --part-border-color: transparent !important; - width: 100% !important; - height: 100% !important; + margin: 0; + border: none; + border-radius: 0; + box-shadow: none; + --part-border-color: transparent; + width: 100%; + height: 100%; } -/* Force content div inside parts to fill the part on phone. - Part.layoutContents() sets inline width/height via size(), which - may use the grid-allocated dimensions rather than the CSS-overridden - 100% dimensions. Override with !important. */ +/* Content div matches the inline size set by Part.layoutContents(). */ .agent-sessions-workbench.phone-layout .part.chatbar > .content, .agent-sessions-workbench.phone-layout .part.sidebar > .content, .agent-sessions-workbench.phone-layout .part.auxiliarybar > .content, .agent-sessions-workbench.phone-layout .part.panel > .content { - width: 100% !important; + width: 100%; } /* Hide the session composite bar (Copilot CLI / Approvals / Branch) on phone */ .agent-sessions-workbench.phone-layout .session-composite-bar { - display: none !important; + display: none; } /* Ensure the grid view element doesn't overflow — flex child must shrink */ @@ -224,8 +238,8 @@ } .agent-sessions-workbench.phone-layout .interactive-session .interactive-input-part { - max-width: none !important; - padding-bottom: calc(10px + env(safe-area-inset-bottom)) !important; + max-width: none !important; /* fights equal-specificity rule in style.css */ + padding-bottom: calc(10px + env(safe-area-inset-bottom)); } /* Chat input minimum font size to prevent iOS auto-zoom */ @@ -247,7 +261,7 @@ } .agent-sessions-workbench.phone-layout .part.sidebar > .composite.title { - display: none !important; + display: none; } .agent-sessions-workbench.phone-layout .part.sidebar > .content { @@ -260,7 +274,7 @@ /* Customization toolbar: hidden on phone (opens editors, not mobile-compatible) */ .agent-sessions-workbench.phone-layout .part.sidebar .ai-customization-toolbar { - display: none !important; + display: none; } /* Make sidebar footer touch-friendly */ @@ -271,7 +285,7 @@ /* Hide the "+ Session" button in the sidebar on phone — replaced by top bar + button */ .agent-sessions-workbench.phone-layout .agent-sessions-new-button-container { - display: none !important; + display: none; } /* Hide sashes on phone */ @@ -291,7 +305,7 @@ /* On phone, push the chat input to the bottom of the chat area */ .agent-sessions-workbench.phone-layout .interactive-session .interactive-input-and-execute-toolbar { - margin-top: auto !important; + margin-top: auto; } /* ---- Phone Layout: Chat Welcome Page ---- */ @@ -351,7 +365,7 @@ } .agent-sessions-workbench.phone-layout .session-workspace-picker-label { - font-size: 18px !important; + font-size: 18px; opacity: 0.6; } @@ -370,12 +384,12 @@ /* Hide the local mode bar (Copilot CLI / Default Approvals / Branch) on phone */ .agent-sessions-workbench.phone-layout .new-chat-bottom-container { - display: none !important; + display: none; } /* Also hide the sessions-chat-widget's DnD overlay on phone */ .agent-sessions-workbench.phone-layout .sessions-chat-dnd-overlay { - display: none !important; + display: none; } /* Chat widget fills full width on phone */