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.

Sign up for GitHub

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

Jump to bottom

vscode: add hcLight support #11589

Merged
merged 1 commit into from
Dec 8, 2022
Merged

vscode: add hcLight support #11589

merged 1 commit into from
Dec 8, 2022

Conversation

vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Aug 19, 2022 •
edited
Loading

What it does

Fixes: #11314.
The pull-request adds support for high contrast light themes and includes the following updates:

  • adds support for hcLight themes
  • updates our color registrations to include hcLight based on values present in vscode
  • updates our change color theme command to support hcLight and adds localizations

Screen Shot 2022-08-19 at 9 17 35 AM

How to test

  1. include the following test theme theia-hclight-0.0.2.zip
  2. start the application
  3. execute the command Preferences: Color Theme - the high contrast light themes should be under high contrast, selecting Test High Contrast Light Theme should work
  4. the new High Contrast Light (Theme) should also work

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto vince-fugnitto added theming issues related to theming vscode issues related to VSCode compatibility labels Aug 19, 2022
msujew
msujew reviewed Aug 19, 2022
View reviewed changes
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

The changes looks quite good. I noticed just two minor issues (which might have been partially introduced by me, but weren't noticeable using just the dark high contrast theme). Do you mind taking care of those with your PR?

Tabbar colors:
vscode
grafik
theia
grafik

Focused notifications:
vscode
grafik
theia
grafik

@vince-fugnitto
Copy link
Member Author

@msujew sure, do you know what the color names are, I wasn't able to find them or identify what might be wrong.

@vince-fugnitto
Copy link
Member Author

I noticed issues when setting Github Dark High Contrast as a theme so more work is needed.

@msujew
Copy link
Member

msujew commented Aug 19, 2022

@vince-fugnitto For the notification it should be list.focusBackground and for the tab bar its editorGroupHeader.tabsBorder. Note that there seems to be some conditional logic attached for both (1 and 2).

@vince-fugnitto
Copy link
Member Author

I've opened the issue #11616 regarding the top border, it seems to be a bug also present on master.

@msujew
Copy link
Member

msujew commented Aug 26, 2022

@vince-fugnitto Ah, I actually didn't talk about the top border of the tab bar, but rather the slim bar that separates the tabs from the widget area (blue in Theia currently, should be black, see screenshots above).

msujew
msujew previously approved these changes Oct 13, 2022
View reviewed changes
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

Coming back to this: I'm actually in favor of merging it now and fixing any possible issues (i.e. missing/incorrectly used colors, etc.) in follow up PRs.

@vince-fugnitto vince-fugnitto marked this pull request as draft October 17, 2022 13:21
@vince-fugnitto
Copy link
Member Author

I noticed some issues with the support so I'll address them.

@vince-fugnitto
Copy link
Member Author

The changes depend on #11589 to be able to use the hc-light monaco.editor.BuiltinTheme.

@vince-fugnitto vince-fugnitto marked this pull request as ready for review November 11, 2022 17:41
@vince-fugnitto
Copy link
Member Author

@msujew I've come back to the pull-request after the monaco uplift and we benefited from proper support for hc-light. I've included a plugin to test against that makes use of hcLight.

@msujew msujew dismissed their stale review November 11, 2022 22:06

Cool, I'll re-review this on Monday :)

@vince-fugnitto
vscode: add hcLight support
419a3bb 
The commit adds support for `hcLight` themes in the framework:
- adds support for a builtin `hc-light` theme.
- adds plugin support for `hcLight`.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto
Copy link
Member Author

I've rebased the changes on the latest master 👍

msujew
msujew approved these changes Dec 8, 2022
View reviewed changes
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@vince-fugnitto vince-fugnitto merged commit 4440abd into master Dec 8, 2022
@vince-fugnitto vince-fugnitto deleted the vf/hcLight branch December 8, 2022 19:32
@github-actions github-actions bot added this to the 1.33.0 milestone Dec 8, 2022
This was referenced Feb 13, 2023
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msujew msujew msujew approved these changes

Assignees
No one assigned
Labels
theming issues related to theming vscode issues related to VSCode compatibility
Projects
None yet
Milestone
1.33.0
Development

Successfully merging this pull request may close these issues.

Support high contrast light theme
2 participants
@vince-fugnitto @msujew