-
Notifications
You must be signed in to change notification settings - Fork 36.2k
Don't inline CSS for comment widget #276479
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
Conversation
Co-authored-by: alexr00 <[email protected]>
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
This PR refactors the comment widget theme styling by replacing dynamically generated inline CSS with static CSS rules that use CSS custom properties (variables). This eliminates runtime CSS generation and reduces code complexity.
Key changes:
- Removed dynamic CSS generation code from
applyThememethod - Moved all theme-dependent styles to static CSS file using CSS variables
- Retained only font-related CSS custom property assignments in TypeScript
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts |
Removed _styleElement field, dynamic CSS content generation, and unused theme color imports; simplified applyTheme to only set font custom properties |
src/vs/workbench/contrib/comments/browser/media/review.css |
Added static CSS rules using CSS variables for theme colors (links, focus, blockquotes, borders, validation errors, thread state, code fonts) |
| const fontSizeVar = '--comment-thread-editor-font-size'; | ||
| const fontWeightVar = '--comment-thread-editor-font-weight'; | ||
| this.container?.style.setProperty(fontFamilyVar, fontInfo.fontFamily); | ||
| this.container?.style.setProperty(fontSizeVar, `${fontInfo.fontSize}px`); |
Copilot
AI
Nov 11, 2025
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 --comment-thread-editor-font-size CSS custom property is being set but is never used in the CSS file. Consider either using it in review.css (e.g., in the .review-widget .body code rule) or removing the fontSizeVar assignment to avoid setting unused CSS properties.
Fixes https://github.com/microsoft/vscode/issues/[issue_number]
The
applyThememethod was dynamically generating CSS rules and injecting them via a<style>element. This moves all theme-dependent styles to the static CSS file using CSS variables.Changes
commentThreadWidget.ts:_styleElementfield and initializationapplyThemeto only set font custom properties (fontFamily, fontSize, fontWeight)domStylesheets, theme color constantsreview.css:--vscode-textLink-foreground,--vscode-textLink-activeForeground--vscode-focusBorder--vscode-textBlockQuote-background--vscode-panel-border,--vscode-contrastBorder--vscode-inputValidation-error*--comment-thread-state-color,--comment-thread-state-background-color--comment-thread-editor-font-*Net: -25 lines, no runtime CSS generation.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.