Skip to content

Commit 2139b8a

Browse files
committed
fix(webview): show modes skeleton until view state loads
- ExtensionStateContext: viewStateLoaded plumbing (context field, initial state, setViewStateLoaded; flipped to true on the first state message) - ModesView: renders a loading skeleton while view-local state is initializing, so stale mode/profile values never flash - App.tsx: remove the bare webviewDidLaunch postMessage useEffect (F1c-owned hunk from #928, folded in here because F1c landed without it); the payload-carrying post now lives only in ExtensionStateContext - Tests: skeleton regression test (ModesView), viewStateLoaded lifecycle tests (ExtensionStateContext), getViewStateId in the App vscode mock Upstream: #915 / PR #928 (vps2 F7)
1 parent 4187c2c commit 2139b8a

6 files changed

Lines changed: 96 additions & 5 deletions

File tree

webview-ui/src/App.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,9 +203,6 @@ const App = () => {
203203
}
204204
}, [telemetrySetting, telemetryKey, machineId, vscodeTelemetryEnabled, didHydrateState])
205205

206-
// Tell the extension that we are ready to receive messages.
207-
useEffect(() => vscode.postMessage({ type: "webviewDidLaunch" }), [])
208-
209206
// Initialize source map support for better error reporting
210207
useEffect(() => {
211208
// Initialize source maps for better error reporting in production

webview-ui/src/__tests__/App.spec.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import AppWithProviders from "../App"
88
vi.mock("@src/utils/vscode", () => ({
99
vscode: {
1010
postMessage: vi.fn(),
11+
getViewStateId: vi.fn(() => "test-view-state-id"),
1112
},
1213
}))
1314

webview-ui/src/components/modes/ModesView.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ const ModesView = () => {
7777
setCustomInstructions,
7878
customModes,
7979
mcpServers,
80+
viewStateLoaded,
8081
} = useExtensionState()
8182

8283
// Use a local state to track the visually active mode
@@ -613,6 +614,17 @@ const ModesView = () => {
613614
})
614615
}
615616

617+
if (viewStateLoaded === false) {
618+
return (
619+
<div data-testid="modes-view-loading-skeleton" className="p-4" aria-busy="true">
620+
<div className="mb-4 h-7 w-24 animate-pulse rounded bg-vscode-input-background" />
621+
<div className="mb-3 h-8 w-full animate-pulse rounded bg-vscode-input-background" />
622+
<div className="mb-4 h-16 w-full animate-pulse rounded bg-vscode-input-background" />
623+
<div className="mb-4 h-16 w-full animate-pulse rounded bg-vscode-input-background" />
624+
</div>
625+
)
626+
}
627+
616628
return (
617629
<div>
618630
<Section>

webview-ui/src/components/modes/__tests__/ModesView.spec.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ const mockExtensionState = {
2626
currentApiConfigName: "",
2727
customInstructions: "Initial instructions",
2828
setCustomInstructions: vitest.fn(),
29+
viewStateLoaded: true,
30+
setViewStateLoaded: vitest.fn(),
2931
}
3032

3133
const renderPromptsView = (props = {}) => {
@@ -43,6 +45,19 @@ describe("PromptsView", () => {
4345
vitest.clearAllMocks()
4446
})
4547

48+
it("shows a loading skeleton while view-local state is initializing", () => {
49+
renderPromptsView({
50+
viewStateLoaded: false,
51+
mode: "debug",
52+
currentApiConfigName: "stale-global-profile",
53+
})
54+
55+
expect(screen.getByTestId("modes-view-loading-skeleton")).toBeInTheDocument()
56+
expect(screen.queryByTestId("mode-select-trigger")).not.toBeInTheDocument()
57+
expect(screen.queryByText("Debug")).not.toBeInTheDocument()
58+
expect(screen.queryByText("stale-global-profile")).not.toBeInTheDocument()
59+
})
60+
4661
it("displays the current mode name in the select trigger", () => {
4762
renderPromptsView({ mode: "code" })
4863
const selectTrigger = screen.getByTestId("mode-select-trigger")

webview-ui/src/context/ExtensionStateContext.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ import { convertTextMateToHljs } from "@src/utils/textMateToHljs"
3838
export interface ExtensionStateContextType extends ExtensionState {
3939
historyPreviewCollapsed?: boolean // Add the new state property
4040
didHydrateState: boolean
41+
viewStateLoaded: boolean
42+
setViewStateLoaded: (value: boolean) => void
4143
showWelcome: boolean
4244
theme: any
4345
mcpServers: McpServer[]
@@ -289,6 +291,7 @@ export const ExtensionStateContextProvider: React.FC<{
289291
)
290292

291293
const [didHydrateState, setDidHydrateState] = useState(false)
294+
const [viewStateLoaded, setViewStateLoaded] = useState(false)
292295
const [showWelcome, setShowWelcome] = useState(false)
293296
const [theme, setTheme] = useState<any>(undefined)
294297
const [filePaths, setFilePaths] = useState<string[]>([])
@@ -345,6 +348,7 @@ export const ExtensionStateContextProvider: React.FC<{
345348
setState((prevState) => mergeExtensionState(prevState, newState))
346349
setShowWelcome(!checkExistKey(newState.apiConfiguration, newState.zooCodeIsAuthenticated))
347350
setDidHydrateState(true)
351+
setViewStateLoaded(true)
348352
// Update alwaysAllowFollowupQuestions if present in state message
349353
if ((newState as any).alwaysAllowFollowupQuestions !== undefined) {
350354
setAlwaysAllowFollowupQuestions((newState as any).alwaysAllowFollowupQuestions)
@@ -539,6 +543,8 @@ export const ExtensionStateContextProvider: React.FC<{
539543
chatFontSize: state.chatFontSize ?? undefined,
540544
reasoningBlockCollapsed: state.reasoningBlockCollapsed ?? true,
541545
didHydrateState,
546+
viewStateLoaded,
547+
setViewStateLoaded,
542548
showWelcome,
543549
theme,
544550
mcpServers,

webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,31 @@ vi.mock("@src/utils/vscode", () => ({
2525
}))
2626

2727
const TestComponent = () => {
28-
const { allowedCommands, setAllowedCommands, soundEnabled, showRooIgnoredFiles, setShowRooIgnoredFiles } =
29-
useExtensionState()
28+
const {
29+
allowedCommands,
30+
setAllowedCommands,
31+
soundEnabled,
32+
showRooIgnoredFiles,
33+
setShowRooIgnoredFiles,
34+
viewStateLoaded,
35+
setViewStateLoaded,
36+
} = useExtensionState()
3037

3138
return (
3239
<div>
3340
<div data-testid="allowed-commands">{JSON.stringify(allowedCommands)}</div>
3441
<div data-testid="sound-enabled">{JSON.stringify(soundEnabled)}</div>
3542
<div data-testid="show-rooignored-files">{JSON.stringify(showRooIgnoredFiles)}</div>
43+
<div data-testid="view-state-loaded">{JSON.stringify(viewStateLoaded)}</div>
3644
<button data-testid="update-button" onClick={() => setAllowedCommands(["npm install", "git status"])}>
3745
Update Commands
3846
</button>
3947
<button data-testid="toggle-rooignore-button" onClick={() => setShowRooIgnoredFiles(!showRooIgnoredFiles)}>
4048
Update Commands
4149
</button>
50+
<button data-testid="set-view-state-loaded-button" onClick={() => setViewStateLoaded(true)}>
51+
Set View State Loaded
52+
</button>
4253
</div>
4354
)
4455
}
@@ -223,6 +234,55 @@ describe("ExtensionStateContext", () => {
223234
expect(JSON.parse(screen.getByTestId("rules").textContent!)).toEqual([])
224235
})
225236

237+
it("initializes with viewStateLoaded set to false", () => {
238+
render(
239+
<ExtensionStateContextProvider>
240+
<TestComponent />
241+
</ExtensionStateContextProvider>,
242+
)
243+
244+
expect(JSON.parse(screen.getByTestId("view-state-loaded").textContent!)).toBe(false)
245+
})
246+
247+
it("marks viewStateLoaded true after receiving initial state", () => {
248+
render(
249+
<ExtensionStateContextProvider>
250+
<TestComponent />
251+
</ExtensionStateContextProvider>,
252+
)
253+
254+
act(() => {
255+
window.dispatchEvent(
256+
new MessageEvent("message", {
257+
data: {
258+
type: "state",
259+
state: {
260+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic },
261+
mode: "ask",
262+
currentApiConfigName: "view-local-profile",
263+
},
264+
},
265+
}),
266+
)
267+
})
268+
269+
expect(JSON.parse(screen.getByTestId("view-state-loaded").textContent!)).toBe(true)
270+
})
271+
272+
it("updates viewStateLoaded through setViewStateLoaded", () => {
273+
render(
274+
<ExtensionStateContextProvider>
275+
<TestComponent />
276+
</ExtensionStateContextProvider>,
277+
)
278+
279+
act(() => {
280+
screen.getByTestId("set-view-state-loaded-button").click()
281+
})
282+
283+
expect(JSON.parse(screen.getByTestId("view-state-loaded").textContent!)).toBe(true)
284+
})
285+
226286
it("updates rules from incoming rules message", () => {
227287
render(
228288
<ExtensionStateContextProvider>

0 commit comments

Comments
 (0)