-
Notifications
You must be signed in to change notification settings - Fork 28.9k
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
Register theme participant refactoring #165576
Register theme participant refactoring #165576
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.
Thanks a lot!
I think the default color will be transparent and the typescript check is just an optimization. We'll see if it works in tomorrows insider ;) |
@hediet Awesome! Are there any other tasks from this issue I could pick up? |
|
Issue - #165169
src/vs/workbench/contrib/codeEditor/browser/accessibility/accessibility.ts
src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts
src/vs/workbench/contrib/codeEditor/browser/inspectEditorTokens/inspectEditorTokens.ts
src/vs/workbench/contrib/codeEditor/browser/untitledTextEditorHint.ts
[1]src/vs/workbench/contrib/codeEditor/browser/suggestEnabledInput/suggestEnabledInput.ts
[2][1] - Needed to add a CSS file and move the .ts and .css file into a separate folder
[2] - Seems like this file uses some conditional logic, I'm not sure if we can move it completely to CSS variables. If there's anything I can do though in this one, let me know.
Also, I had a query. Most of the ones we are replacing have a condition in the TypeScript file that checks if that variable has a value, then it adds the CSS rule. I don't think this condition would be checked while using CSS Variables right.
So, do the variables always have some default value, or will there be a case when the variable might be undefined and cause an issue?