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

VS Code like theming (colors and icons) #6475

Merged
merged 65 commits into from
Dec 27, 2019
Merged

VS Code like theming (colors and icons) #6475

merged 65 commits into from
Dec 27, 2019

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Oct 31, 2019 •
edited
Loading

What it does

fix #4831, fix #1486, fix #6300, fix #6747, fix #6776

  • (core) allow to register and use colors
    • Theia extensions can register new colors.
    • All colors are translated into css variables with . replaced by - and --theia prefix
    • Theia extensions can use registered colors via css or programatically from ColorRegistrsy.getCurrentColor
  • (plugin) hook themes contribution point @akosyakov
    • json files
    • tmTheme files
    • Theia extensions should use MonacoThemingService.register to contribute new themes
  • (plugin) hook colors contribution point @akosyakov
  • (monaco) preserve themes between user sessions and reconnections @akosyakov
  • register missing VS Code colors from corresponding Theia extension, see [WIP] VS Code missing colors #6722
    • Base colors are reused from Monaco, one has to check what is missing by inspecting them.
    • restyle Theia extensions
    • remove existing css variables
  • define color theming coding guidelines
    • make sure that colors are not defined in css anymore, i.e. --theia-ui-font-color and --theia-layout-color are not used
  • update CHANGELOG with breaking changes
  • (core) allow to register and select icon themes
    • register existing theme
    • register None theme to disable icon themes
    • deprecate name, description and icon for tree nodes, use LabelProvider instead to avoid storing them in the local storage and compute synchronously only demand only for visible UI nodes
  • (plugin) hook icon theme contribution point
    • icon definitions from paths
    • preserve current icon theme between reloads
    • icon definitions based on font definitions
    • icon associations
    • overriding icon associations for different color themes
  • (core) align theming commands and preferences with VS Code
  • a CQ for copied code: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=21223
  • fix tests
  • review places where FOLDER_ICON and FILE_ICON are used
  • local storage runs out of memory sometimes -> theme data are stored in indexeddb instead
  • debug console tab does not have icon
  • in offline mode for One Dark Pro theme status bar elements are bogusly highlighted while hovering over them
  • status bar coloring while debugging a program
  • custom default dark/light Theia themes, in light theme activity bar should be light as well
  • outline show duplicate elements

How to test

Review checklist

Reminder for reviewers

@akosyakov akosyakov changed the title [theming] #4831: color contribution point color theming Oct 31, 2019
@akosyakov akosyakov self-assigned this Oct 31, 2019
@akosyakov akosyakov changed the title color theming WIP color theming Oct 31, 2019
@akosyakov akosyakov added shell issues related to the core shell theming issues related to theming vscode issues related to VSCode compatibility labels Oct 31, 2019
@akosyakov
Copy link
Member Author

It's a temporary PR against the webview PR, don't merge it!

@jbicker please see What it does, it explains what is your part. We can discuss it next Tuesday face to face.

I'm going to hook VS Code contribution points that you can also test against VS Code extensions and then go back to the webview PR.

@akosyakov
Copy link
Member Author

akosyakov commented Oct 31, 2019 •
edited
Loading

I've hooked up the theme contribution point from VS Code extensions for JSON files.

@jbicker You can download following themes and put them under plugins folder to install for testing:

Right now new css color variables are only applied to editors and webviews:
theming

@akosyakov akosyakov force-pushed the color_theming branch 3 times, most recently from b982038 to e7ee215 Compare November 1, 2019 13:11
@akosyakov
Copy link
Member Author

@jbicker I've finished my part, all VS Code extension contribution points are respected, please see How to test section.

@akosyakov akosyakov force-pushed the ak/vscode_webviews branch 2 times, most recently from 2f86eed to f2660f8 Compare November 1, 2019 14:41
@akosyakov akosyakov force-pushed the ak/vscode_webviews branch 12 times, most recently from b961ceb to 10879d4 Compare November 13, 2019 10:44
@akosyakov
update default Theia themes
55227b4 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[theming] update CHANGELOG
19d29fa 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] remove patching of VS Code extension configurations
785757f 
Extensions relying on null values, we should not stub them with empty objects.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[keybinding] fix #6781: highlight search input on light themes
58a1d7d 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[theming] fix #6783: don't render an icon element if an icon is not set
269909c 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[theming] fix #6785: align monaco list and scrollbar colors
ea270ba 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
Copy link
Member Author

@vince-fugnitto I've tackled all issues except #6786. I don't think it is very critical. Please let me know what else should be done for the approval.

@vince-fugnitto
Copy link
Member

@vince-fugnitto I've tackled all issues except #6786. I don't think it is very critical. Please let me know what else should be done for the approval.

I verified my existing issues and they look to be resolved 👍
I did notice a couple of issues however:

  1. it looks like we might add git decorations twice?

Screen Shot 2019-12-26 at 1 28 25 PM

  1. certain light themes have incorrect colors for the sidebar (activity bar)

@theia/vscode-builtin-quietlight

Screen Shot 2019-12-26 at 1 40 31 PM

VS Code Horizon Bright Theme

Screen Shot 2019-12-26 at 1 40 42 PM

  1. the LICENSE icon still seems to be incorrect when using minimal icon theme (it seems to be treated as a folder)

Screen Shot 2019-12-26 at 1 44 43 PM

@akosyakov
Copy link
Member Author

akosyakov commented Dec 27, 2019 •
edited
Loading

it looks like we might add git decorations twice?

@vince-fugnitto Are you sure that you did not have git support installed from Theia and VS Code extensions at the same time? Or maybe you forgot to rebuild the example app after changing package.json?

@akosyakov
[theming] fix #6788: introduce URIIconReference
7b42a8e 
To distinguis between folder and file URIs.
By default file icons are computed for URIs,
but clients can provide `folder` as an id of `URIIconReference`
to compute folder icons.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[theming] fix activity bar icon colors
872e810 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
Copy link
Member Author

@vince-fugnitto I'have addressed everything in #6475 (comment). Except double git decorators, I suspect you have git support from Theia and VS Code extensions at the same time. Maybe you have not rebuilt the browser example after changing package.json. Please have a look again.

vince-fugnitto
vince-fugnitto approved these changes Dec 27, 2019
View reviewed changes
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.

It looks and works very nicely 👍

You are right, it must have been that the example-browser was not rebuilt however I was sure I did so 🙈 Everything works correctly now and is fine with me. Any further enhancements can always be done during the month including bug fixes but I really like the changes!

Sorry if I am a bit late reviewing the changes at each iteration, I'm not officially back to work until the 2nd of January but did not want to hold you back 😄

@akosyakov
Copy link
Member Author

Sorry if I am a bit late reviewing the changes at each iteration, I'm not officially back to work until the 2nd of January but did not want to hold you back 😄

@vince-fugnitto that's alright, have nice holidays!

@akosyakov
Copy link
Member Author

I'm going to merge it. Please open new issues if something is wrong or to ask for help on how to migrate.

@akosyakov akosyakov merged commit c99cc1d into master Dec 27, 2019
This was referenced Dec 30, 2019
Closed
Closed
@akosyakov akosyakov deleted the color_theming branch January 3, 2020 06:56
@spoenemann spoenemann mentioned this pull request Jan 6, 2020
Closed
@vince-fugnitto vince-fugnitto mentioned this pull request Jan 6, 2020
Closed
This was referenced Jan 16, 2020
Closed
Closed
@azatsarynnyy azatsarynnyy mentioned this pull request Jan 21, 2020
Closed
@tortmayr tortmayr mentioned this pull request Jan 27, 2020
Closed
@azatsarynnyy azatsarynnyy mentioned this pull request Feb 19, 2020
Closed
@azatsarynnyy azatsarynnyy mentioned this pull request Apr 30, 2020
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcdumais-work marcdumais-work marcdumais-work left review comments

@svenefftinge svenefftinge svenefftinge left review comments

@westbury westbury westbury left review comments

@vince-fugnitto vince-fugnitto vince-fugnitto approved these changes

Assignees

@akosyakov akosyakov

@jbicker jbicker

Labels
shell issues related to the core shell theming issues related to theming vscode issues related to VSCode compatibility
Projects
None yet
Milestone
No milestone
6 participants
@akosyakov @vince-fugnitto @westbury @svenefftinge @marcdumais-work @jbicker