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

Support dark and light high contrast theme types #144193

Closed
daviddossett opened this issue Mar 1, 2022 · 6 comments · Fixed by #144759
Closed

Support dark and light high contrast theme types #144193

daviddossett opened this issue Mar 1, 2022 · 6 comments · Fixed by #144759
Assignees
Labels
insiders-released Patch has been released in VS Code Insiders themes Color theme issues
Milestone

Comments

@daviddossett
Copy link
Contributor

daviddossett commented Mar 1, 2022

Adding the new light high contrast theme (#143398) uncovered challenges across the codebase when attempting to render unique styles or assets based on the theme settings. Here's a simple example where the letterpress SVG used in an empty editor view was hardcoded for the original dark HC theme.

Proposal

Currently we only expose a general high contrast theme type. A couple of things I think would be worth supporting:

  • Split hc into something like hc-light and hc-dark
  • Support workbench.preferredLightHighContrastColorTheme and workbench.preferredDarkHighContrastColorTheme if we're able to pick up the OS mode as an additional input like we do for regular dark/light themes.

Ideally the outcome here is that we can continue to use the theme type as an input in both logic (e.g. this.theme.type === ColorScheme.LIGHT_HIGH_CONTRAST and in stylesheets (e.g. .hc-light or .hc-black).

It would be great to get any additonal thoughts/opinions. cc @isidorn @aeschli @misolori

@miguelsolorio
Copy link
Contributor

I think this makes sense and we'll likely start seeing requests for this (to create a new Light HC based off the new version) so 👍

@isidorn
Copy link
Contributor

isidorn commented Mar 2, 2022

I agree with the proposal, it makes sense. I know @aeschli is buried in issues so maybe somebody else can drive this debt work? Martin do you have suggestions?

fyi @bpasero

@bpasero
Copy link
Member

bpasero commented Mar 2, 2022

Splitting hc up into hc-light and hc-dark resonates with me.

@bpasero bpasero added the themes Color theme issues label Mar 2, 2022
@aeschli
Copy link
Contributor

aeschli commented Mar 2, 2022

Yes, makes sense.

@aeschli
Copy link
Contributor

aeschli commented Mar 2, 2022

I'm happy to do the workbench theme service changes if you look at the css fixes.
Currently all colors have default colors for light, dark and hc. That means we would need new defaults for hc-light as well.

So it's quite some work...

@daviddossett
Copy link
Contributor Author

I'm happy to do the workbench theme service changes if you look at the css fixes. Currently all colors have default colors for light, dark and hc. That means we would need new defaults for hc-light as well.

That works for me! 👍

@daviddossett daviddossett self-assigned this Mar 2, 2022
@aeschli aeschli added this to the March 2022 milestone Mar 8, 2022
@miguelsolorio miguelsolorio mentioned this issue Apr 5, 2022
52 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Apr 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
insiders-released Patch has been released in VS Code Insiders themes Color theme issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@bpasero @isidorn @aeschli @daviddossett @miguelsolorio and others