Use product icons instead of 'hardcoded' icons

[DeltaRazero]

This changes themability

The following changes are proposed/purpose of the changes:

• Add a product icon font instead of 'hardcoded' icons. This has the following benefits:
  • Product icon themes can now override the icons if desired
  • Color themes can now properly set the colors of the icons, so no need to maintain light and dark icons that are not following the theme's colors.
• Add a script using FontForge scripting API for generating product icon font for future use
  • I feel like FontForge is a good cross-platform open source platform to use as font generator. It is robust and it doesn't require a separate installation of Python to invoke the scripting API (i.e. FontForge bundles its own Python runtime).
  • The inclusion of the script is purely for future use if more icons are needed
  • Filenames of icons now include the glyph codepoint so it's easier to see what the codepoint is for each icon, as well as the script uses this for mapping the icons to the desired codepoints.
• Remove redundant icons that are already part of codicons; change the package.json to use the already present codicons

Furthermore, I centered the build icon as it was off-center.

[DeltaRazero]

@microsoft-github-policy-service agree

[DeltaRazero]

@gcampbell-msft Could you kindly provide me with feedback or a comment please? Thanks!

[gcampbell-msft]

@DeltaRazero My apologies for the much too long delay in interacting and reviewing this PR, we have been very busy, but again, my apologies.

I think that this is a great addition to help us have an easier time with these icons, and it also makes us match the vscode docs here better: https://code.visualstudio.com/api/references/contribution-points#contributes.icons.

However, I don't like the idea of having to install fontforge. Therefore, I think we should instead follow the pattern of the https://github.com/microsoft/vscode-extension-samples/tree/main/product-icon-theme-sample example and use the webfont package, and create a new "tool" in typescript that does the same thing as your python script.

Could you please follow this example and change the FontForge use to be typescript/javascript?

[gcampbell-msft]

See other comments, but we will need this to use the typescript implementation of the creation of the .woff file, rather than having to install fontforge. We're excited at the idea of having less maintenance for the dark/light, but we'll need you to change the approach from FontForge to the typescript. Unfortunately, we likely won't merge the PR with the current state due to not wanting a dependency on an executable in order to update our code.

Thanks!

[DeltaRazero]

Hey @gcampbell-msft Totally no problem. Also now that I think about it, it's indeed better to the script be JS/TS based.

To avoid confusion, regarding the following:

However, I don't like the idea of having to install fontforge. Therefore, I think we should instead follow the pattern of the https://github.com/microsoft/vscode-extension-samples/tree/main/product-icon-theme-sample example and use the webfont package, and create a new "tool" in typescript that does the same thing as your python script.

Do you mean to add a tool just like the pr-creator tool in this repo? If so, I'll follow the structure of that.

[gcampbell-msft]

Hey @gcampbell-msft Totally no problem. Also now that I think about it, it's indeed better to the script be JS/TS based.

To avoid confusion, regarding the following:

However, I don't like the idea of having to install fontforge. Therefore, I think we should instead follow the pattern of the https://github.com/microsoft/vscode-extension-samples/tree/main/product-icon-theme-sample example and use the webfont package, and create a new "tool" in typescript that does the same thing as your python script.

Do you mean to add a tool just like the pr-creator tool in this repo? If so, I'll follow the structure of that.

Yes! That is exactly what I mean, that's a great idea.

[DeltaRazero]

@gcampbell-msft I've rebased the branch on a new commit that now adds a TypeScript/NodeJS-based tool for generating product icon fonts. The webfont package unfortunately did not allow for manually specifying glyph codepoints. I tested a few ones but felt that fontagon was the best suited.

I've added a command as well to package.json for easy reproducible use of the tool within the repo.

It's possible that the ttf2woff asm.js sub-dependency generates a warning based on certain newer NodeJS runtime versions, but there's no actual breakage. Besides, woff2 is a better target than the original woff spec, which uses a different sub-dependency for conversion.

Aside from usual feedback I can be expecting, I'm interested in your opinion on the following matter: I'm in the process of making a product icon theme and want to submit PRs to multiple repositories for including such a tool so those too can switch over to using the product icon API. Among these are a few Microsoft repos. Would it be better to request a PR to every repo copying the code/tool, or would it be better to upload this as a separate NPM package so there's one place to maintain it (if it needs any)?

[gcampbell-msft]

@DeltaRazero As for your question about putting this new tool in multiple repos or an NPM package. I think it's likely easier to create an npm package, IF, the code will be the same and has support for customizability (such as where the images live, ability to pass arguments that give the tool relevant information, etc).

For a first step though, I don't think it's an awful thing to put it in multiple places, but if it's going to be the same code, it definitely would be nice to have a package so it's maintained in one spot.

[gcampbell-msft]

@DeltaRazero Left a couple of comments, I think the only things left are the comments I made. Other than that, I think the typescript tool looks very good, and I appreciate you changing it from the Python!

Looking forward to the final nit changes before being able to merge. THanks.

[DeltaRazero]

@DeltaRazero As for your question about putting this new tool in multiple repos or an NPM package. I think it's likely easier to create an npm package, IF, the code will be the same and has support for customizability (such as where the images live, ability to pass arguments that give the tool relevant information, etc).

For a first step though, I don't think it's an awful thing to put it in multiple places, but if it's going to be the same code, it definitely would be nice to have a package so it's maintained in one spot.

Thanks for sharing your thoughts. Yeah in that case I'll go for the route of having the code in multiple repos, given that some more customization options may be necessary for other projects. Whenever I think it's tried and tested enough I'll submit another PR and make a package for it.

[gcampbell-msft]

@DeltaRazero We've noticed a bug in pre-release after this PR merged where the icons are very small, is there context we should understand about why this is? I'm investigating now as well. Thanks

[DeltaRazero]

@gcampbell-msft I see I disabled the normalize option, this should definitely be enabled.

diff --git a/tools/product-icon-font-generator/src/index.ts b/tools/product-icon-font-generator/src/index.ts
index 8622a86f..80d94dd1 100644
--- a/tools/product-icon-font-generator/src/index.ts
+++ b/tools/product-icon-font-generator/src/index.ts
@@ -141,7 +141,7 @@ class WebfontGenerator {
                 // The SVG intermediate format has all the transformative options we can set.
                 svg: {
                     fontHeight: 1000,
-                    normalize: false
+                    normalize: true
                 }
             }
         });

After enabling this, all icons will be scaled up. Icons that have different sizes are kept at their respective sizes, and this option basically all sizes them to a normalized scale. Not sure when and why I disabled it, probably some testing that I forgot about.

Anyways the diff above should fix it. Do you want me to submit a new merge request, or is this something that can be done on your end? Maybe it'd be good to have this be a commandline option to enable/disable (by default enabled)?

[gcampbell-msft]

@DeltaRazero I've got a PR here: #4139, thanks!