Make high contrast detection configurable #17394

Merged
merged 2 commits into from Dec 16, 2016

Projects

None yet

4 participants

@geirsagberg
Contributor

Add a new option 'window.autoDetectHighContrast', defaults to true. If false, VS Code theme will ignore Windows High Contrast theme.

Fixes #17279

@geirsagberg geirsagberg Make high contrast detection configurable
a655ded
@msftclas

Hi @geirsagberg, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@bpasero

@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.")
+ }
@bpasero
bpasero Dec 16, 2016 Member

Missing trailing comma.

- this.themeService.setColorTheme(VS_HC_THEME, false);
- });
+ const windowConfig = this.configurationService.getConfiguration<IWindowSettings>('window');
+ if (windowConfig.autoDetectHighContrast) {
@bpasero
bpasero Dec 16, 2016 Member

windowConfig can be null

- this.partService.joinCreation().then(() => {
- this.themeService.setColorTheme(VS_DARK_THEME, false);
- });
+ const windowConfig = this.configurationService.getConfiguration<IWindowSettings>('window');
@bpasero
bpasero Dec 16, 2016 Member

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.

@geirsagberg
geirsagberg Dec 16, 2016 Contributor

I have expanded the setting description. I feel if this settings is disabled, nothing should happen automatically when changing to/from windows HC theme.

@bpasero bpasero self-assigned this Dec 16, 2016
@bpasero bpasero added this to the January 2017 milestone Dec 16, 2016
@geirsagberg geirsagberg Fix trailing comma, better description and null check
80d3c9b
@geirsagberg
Contributor

I have signed the CLA; do I need to do anything to make the bot accept it?

@bpasero
Member
bpasero commented Dec 16, 2016

@geirsagberg I think it can take some minutes to be picked up 👍

@msftgits

Hi, I am closing and re-opening this PR to bump the CLA bot. Sorry for the inconvenience!

@msftgits msftgits closed this Dec 16, 2016
@msftgits msftgits reopened this Dec 16, 2016
@msftclas

Hi @geirsagberg, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

@msftclas msftclas added the cla-signed label Dec 16, 2016
@msftgits msftgits removed the cla-required label Dec 16, 2016
@bpasero
Member
bpasero commented Dec 16, 2016

LGTM, thanks for the prompt fix.

@bpasero bpasero merged commit a0f4983 into Microsoft:master Dec 16, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@geirsagberg geirsagberg deleted the geirsagberg:17279_disable_highContrast branch Dec 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment