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

Add missing StatusBarItem fields of VS Code API #11026

Conversation

lucas-koehler
Copy link
Contributor

@lucas-koehler lucas-koehler commented Apr 14, 2022

What it does

Fixes #11019

  • Add missing properties id, name, backgroundColor to StatusBarItem API
  • Handle properties in implementation and forward to frontend
  • Add backgroundColor handling to StatusBar
  • In the plugin context, if no status bar item id is given, do NOT use the extension id as specified in the VS Code API.
    Instead, keep the current logic of generating a unique id.

Contributed on behalf of STMicroelectronics

Signed-off-by: Lucas Koehler [email protected]

status-bar-item-properties.mp4

How to test

  • Download the test extension from here. The source is available here for reference.
  • Select Theia Light theme
  • Open a text file and select a few lines. For line count zero, the error colors should be used, for one or two the text should have color blue (#0000FF), for 3 or more lines the warning colors should be used
  • Click on the status bar entry.

Review checklist

Reminder for reviewers

* Add missing properties `id`, `name`, `backgroundColor` to `StatusBarItem` API
* Handle properties in implementation and forward to frontend
* Add backgroundColor handling to `StatusBar`
* In the plugin context, if no status bar item id is given, do NOT  use the extension id as specified in the VS Code API.
  Instead, keep the current logic of generating a unique id.

Contributed on behalf of STMicroelectronics

Signed-off-by: Lucas Koehler <[email protected]>
@lucas-koehler lucas-koehler force-pushed the gh-11019-status-bar-item-fields branch from b478610 to 8de4ba3 Compare April 14, 2022 13:32
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

Adapt Theia's light theme in your repository to have warning and error colors because the theme's in Theia do not have them, yet: Add to packages/monaco/data/monaco-themes/vscode/light_theia.json to colors property:

"statusBarItem.errorBackground": "#c72e0f",
"statusBarItem.warningBackground": "#c7b50f",
"statusBarItem.errorForeground": "#ffffff",
"statusBarItem.warningForeground": "#000"

@lucas-koehler given that the changes rely on the following theming colors, part of the changes should be to register the defaults for these theming colors (so even if a theme does not set them they still work):

@JonasHelming JonasHelming added the vscode issues related to VSCode compatibility label Apr 15, 2022
The commit registers the following theming colors for statusbar items:
- `statusBarItem.errorBackground`
- `statusBarItem.errorForeground`
- `statusBarItem.warningBackground`
- `statusBarItem.warningForeground`

Signed-off-by: vince-fugnitto <[email protected]>
@lucas-koehler
Copy link
Contributor Author

lucas-koehler commented Apr 19, 2022

@vince-fugnitto Thanks for the feedback! I agree that it makes sense to add default colors. I cherry-picked and tested your commit. Is that alright with you?

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 look good to me. I can confirm that the extension performs as expected 👍

@vince-fugnitto anything to add?

@vince-fugnitto
Copy link
Member

@vince-fugnitto Thanks for the feedback! I agree that it makes sense to add default colors. I cherry-picked and tested your commit. Is that alright with you?

Of course, I tried to make it easy for you to consume :)

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I also confirm that the changes work well using the example plugin 👍

@JonasHelming JonasHelming merged commit c78d302 into eclipse-theia:master Apr 20, 2022
@JonasHelming JonasHelming added this to the 1.25.0 milestone Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add missing StatusBarItem fields of VS Code API
4 participants