Skip to content
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

Expose status bar debugging and no-folder foreground colors. #27052

Closed

Conversation

smoogipoo
Copy link

Just being able to change the foreground color may not be enough in cases where the background color is radically different.

@msftclas
Copy link

@smoogipooo,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@msftclas
Copy link

@smoogipooo, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@bpasero bpasero self-assigned this May 22, 2017
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, some feedback provided.

@@ -118,6 +118,12 @@ export const STATUS_BAR_FOREGROUND = registerColor('statusBar.foreground', {
hc: '#FFFFFF'
}, 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',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@@ -20,6 +20,12 @@ export const STATUS_BAR_DEBUGGING_BACKGROUND = registerColor('statusBar.debuggin
hc: '#CC6633'
}, 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',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, use STATUS_BAR_FOREGROUND as default

return STATUS_BAR_DEBUGGING_BACKGROUND;
}

private getForegroundColorKey(): string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)?

@bpasero
Copy link
Member

bpasero commented May 22, 2017

@smoogipooo thanks, there is a bigger issue though which I am not sure how to solve: If you search for STATUS_BAR_FOREGROUND you will notice that we use this color in more places (tasks, feedback) where we use SVG icons and set the foreground color too. E.g. with your changes you will notice that the color is not being picked up here for the icons:

image

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.

@smoogipoo
Copy link
Author

Closing temporarily pending changes since I don't know how much time I have to work on this over the next few days.

@smoogipoo smoogipoo closed this May 22, 2017
@bpasero bpasero added this to the May 2017 milestone May 24, 2017
@bpasero
Copy link
Member

bpasero commented May 24, 2017

@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.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants