-
Notifications
You must be signed in to change notification settings - Fork 37.6k
Polishing mermaid rendering #290639
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Polishing mermaid rendering #290639
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Polishes Mermaid chat output rendering by improving webview integration (state/context) and adding pan/zoom + a reset action via the webview context menu.
Changes:
- Persist and restore chat-output webview state (
webview.state) across re-renders. - Provide webview context metadata (
providedViewType, scoped context keys) sowebview/contextmenu contributions work. - Add Mermaid webview pan/zoom support and a “Reset Pan and Zoom” command exposed in the webview context menu.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolOutputPart.ts | Passes persisted webviewState into renderer and stores state updates from the webview. |
| src/vs/workbench/contrib/chat/browser/chatOutputItemRenderer.ts | Sets providedViewType, scopes context keys to the webview container, and restores webview.state before mount. |
| src/vs/workbench/api/browser/mainThreadChatOutputRenderer.ts | Returns the ext-host render promise from the renderer callback (but currently misses unregister lifecycle wiring). |
| extensions/mermaid-chat-features/src/extension.ts | Adds reset command and webview context metadata; tracks a webview for reset messaging. |
| extensions/mermaid-chat-features/package.json | Contributes the reset command and webview context menu entry. |
| extensions/mermaid-chat-features/chat-webview-src/index.ts | Implements pan/zoom + persisted state in the Mermaid webview and listens for reset messages. |
Comments suppressed due to low confidence (1)
src/vs/workbench/api/browser/mainThreadChatOutputRenderer.ts:52
- The disposable returned by
_rendererService.registerRenderer(...)is currently ignored, soregisteredRenderersis never populated and$unregisterChatOutputRenderer/dispose()cannot actually unregister renderers. Store the returned disposable inregisteredRenderers(and delete it on unregister) so renderer registration lifecycle matches the ext host’s disposable.
return this._proxy.$renderChatOutput(viewType, mime, VSBuffer.wrap(data), webviewHandle, token);
},
}, {
extension: { id: extensionId, location: URI.revive(extensionLocation) }
});
| if (partState.webviewOrigin) { | ||
| partState.webviewOrigin = partState.webviewOrigin; | ||
| } |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if block is a no-op (partState.webviewOrigin is assigned to itself) and can be removed. If the intent was to persist a generated/actual origin across re-renders, store the real origin value instead (and consider keeping webviewOrigin readonly unless it is intentionally mutated).
| if (partState.webviewOrigin) { | |
| partState.webviewOrigin = partState.webviewOrigin; | |
| } |
| // Track this webview for the reset command | ||
| activeWebview = webview; | ||
|
|
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tracking a single global activeWebview can reset the wrong diagram when multiple Mermaid outputs are present (the context menu command is invoked for the clicked webview, not necessarily the last rendered one). Consider passing a per-webview instance id via data-vscode-context and using the command argument to look up the correct webview to message (or another mechanism that targets the invoking webview).
| "commands": [ | ||
| { | ||
| "command": "mermaid-chat.resetPanZoom", | ||
| "title": "Reset Pan and Zoom" |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This command title is user-facing but is not localized. Consider switching to a %...% placeholder and adding the string to package.nls.json (as is already done for displayName, description, and config descriptions).
| "title": "Reset Pan and Zoom" | |
| "title": "%mermaidChat.resetPanZoom.title%" |
| tryUpdate(); | ||
| }).observe(document.body, { attributes: true, attributeFilter: ['class'] }); | ||
|
|
||
| init(); |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init is async but its returned promise is ignored. If mermaid.run(...) rejects, this becomes an unhandled promise rejection in the webview. Consider prefixing with void and/or adding a .catch(...) handler.
| init(); | |
| void init().catch(error => { | |
| console.error('Failed to initialize mermaid webview', error); | |
| }); |
| interface VsCodeApi { | ||
| getState(): any; | ||
| setState(state: any): void; | ||
| postMessage(message: any): void; | ||
| } | ||
|
|
||
| declare function acquireVsCodeApi(): VsCodeApi; | ||
|
|
||
|
|
||
| interface PanZoomState { | ||
| scale: number; | ||
| translateX: number; | ||
| translateY: number; | ||
| } | ||
|
|
||
| class PanZoomHandler { | ||
| private scale = 1; | ||
| private translateX = 0; |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VsCodeApi interface uses any for state and messages, which loses type safety and can hide errors in this new pan/zoom + messaging code. Consider using unknown/generics (e.g. getState(): PanZoomState | undefined, typed message union) or at least constraining to Record<string, unknown> where appropriate.
| interface VsCodeApi { | |
| getState(): any; | |
| setState(state: any): void; | |
| postMessage(message: any): void; | |
| } | |
| declare function acquireVsCodeApi(): VsCodeApi; | |
| interface PanZoomState { | |
| scale: number; | |
| translateX: number; | |
| translateY: number; | |
| } | |
| class PanZoomHandler { | |
| private scale = 1; | |
| private translateX = 0; | |
| interface PanZoomState { | |
| scale: number; | |
| translateX: number; | |
| translateY: number; | |
| } | |
| interface VsCodeApi { | |
| getState(): PanZoomState | undefined; | |
| setState(state: PanZoomState): void; | |
| postMessage(message: Record<string, unknown>): void; | |
| } | |
| declare function acquireVsCodeApi(): VsCodeApi; | |
| class PanZoomHandler { | |
| private scale = 1; | |
| private translateX = 0; | |
| private translateX = 0; |
Fixes #271371