Expose status bar debugging and no-folder foreground colors.#27052
Expose status bar debugging and no-folder foreground colors.#27052smoogipoo wants to merge 4 commits intomicrosoft:masterfrom
Conversation
|
@smoogipooo, It will cover your contributions to all Microsoft-managed open source projects. |
|
@smoogipooo, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request. |
bpasero
left a comment
bpasero
left a comment
There was a problem hiding this comment.
Thanks, some feedback provided.
src/vs/workbench/common/theme.ts
Outdated
| }, nls.localize('statusBarForeground', "Status bar foreground color. The status bar is shown in the bottom of the window.")); | ||
|
|
||
| export const STATUS_BAR_NO_FOLDER_FOREGROUND = registerColor('statusBar.noFolderForeground', { | ||
| dark: '#FFFFFF', |
There was a problem hiding this comment.
I suggest to use STATUS_BAR_FOREGROUND here as default color for all themes instead of specifying the color again as FFFFFF. This makes sure that all existing themes are not broken that specify STATUS_BAR_FOREGROUND any different from FFFFFF.
| }, localize('statusBarDebuggingBackground', "Status bar background color when a program is being debugged. The status bar is shown in the bottom of the window")); | ||
|
|
||
| export const STATUS_BAR_DEBUGGING_FOREGROUND = registerColor('statusBar.debuggingForeground', { | ||
| dark: '#FFFFFF', |
There was a problem hiding this comment.
Same here, use STATUS_BAR_FOREGROUND as default
| return STATUS_BAR_DEBUGGING_BACKGROUND; | ||
| } | ||
|
|
||
| private getForegroundColorKey(): string { |
There was a problem hiding this comment.
This looks almost identical to getBackgroundColorKey. Can we just have one method where all color keys are passed in? Something like getColorKey(noFolderColor, debuggingColor, normalColor)?
…d-colors # Conflicts: # src/vs/workbench/browser/parts/statusbar/statusbarPart.ts
|
@smoogipooo thanks, there is a bigger issue though which I am not sure how to solve: If you search for
We need a way to carry over the colors to those places but without introducing a dependency to debug. I think this needs a different solution. |
|
Closing temporarily pending changes since I don't know how much time I have to work on this over the next few days. |
|
@smoogipooo I took your PR as starting point and did the necessary changes on top to make this work with debugging: 1281eab I added you to the release notes for our upcoming release to attribute your PR. |
3 participants
Just being able to change the foreground color may not be enough in cases where the background color is radically different.