-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Make high contrast detection configurable #17394
Make high contrast detection configurable #17394
Conversation
Hi @geirsagberg, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
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.
@geirsagberg very cool, only minor feedback!
'type': 'boolean', | ||
'default': true, | ||
'description': nls.localize('autoDetectHighContrast', "If enabled, will automatically change to high contrast theme if Windows is using a high contrast theme.") | ||
} |
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.
Missing trailing comma.
this.themeService.setColorTheme(VS_HC_THEME, false); | ||
}); | ||
const windowConfig = this.configurationService.getConfiguration<IWindowSettings>('window'); | ||
if (windowConfig.autoDetectHighContrast) { |
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.
windowConfig
can be null
this.partService.joinCreation().then(() => { | ||
this.themeService.setColorTheme(VS_DARK_THEME, false); | ||
}); | ||
const windowConfig = this.configurationService.getConfiguration<IWindowSettings>('window'); |
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.
I think we can ignore the setting in this case because leaving HC on Windows you want to switch back to dark theme. Or otherwise the setting should be clearer that it prevents switching in both ways, not just from normal to HC theme.
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.
I have expanded the setting description. I feel if this settings is disabled, nothing should happen automatically when changing to/from windows HC theme.
I have signed the CLA; do I need to do anything to make the bot accept it? |
@geirsagberg I think it can take some minutes to be picked up 👍 |
Hi, I am closing and re-opening this PR to bump the CLA bot. Sorry for the inconvenience! |
Hi @geirsagberg, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
LGTM, thanks for the prompt fix. |
Add a new option 'window.autoDetectHighContrast', defaults to true. If false, VS Code theme will ignore Windows High Contrast theme.
Fixes #17279