microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.59k stars 28.66k forks source link

[product icon themes] Finalize icon contribution API #119101

Closed aeschli closed 2 years ago

aeschli commented 3 years ago

The icon contribution point allow extensions to define a new icon by id, along with a default icon. That icon id can then be used by the extension (or any other extensions that depend on the extension) at the places where ThemeIcon can be used new ThemeIcon("iconId"), markdown strings ($(iconId)) and as icons in certain contribution points.

Product icon themes can redefine the icon (if they know about the icon id)

"contributes": {
    "icons": [
        {
            "id": "distro-ubuntu",
            "description": "Ubuntu icon",
            "default": {
                "fontId": "distro-icon-font",
                "fontCharacter": "\\E001"
            }
        },
        {
            "id": "distro-fedora",
            "description": "Fedora icon",
            "default": {
                "fontId": "distro-icon-font",
                "fontCharacter": "\\E002"
            }
        }
    ],
    "iconFonts": [
        {
            "id": "distro-icon-font",
            "src": [
                {
                    "path": "./distroicons.woff",
                    "format": "woff"
                }
            ]
        }
    ]
}

In this example, an extension defines two new icon ids distro-ubuntu and distro-fedora along with default icon definitions. The icons are defined in an icon font at the given font character. The icon font is defined in a new contribution point iconFonts.

Update

See https://github.com/microsoft/vscode/issues/119101#issuecomment-1032860411 for the latest proposal that is implemented in the latest 1.65 insiders builds

    "icons": {
        "distro-ubuntu": {
            "description": "Ubuntu icon",
            "default": {
                "fontPath": "./distroicons.woff",
                "fontCharacter": "\\E001"
            }
        },
        "distro-fedora": {
            "description": "Ubuntu icon",
            "default": {
                "fontPath": "./distroicons.woff",
                "fontCharacter": "\\E002"
            }
        }
    }
aeschli commented 3 years ago

Planed for April. I still want to measure if having many fonts can become a performance issue.

matthewjamesadam commented 2 years ago

@aeschli I am very interested in this proposal. One use case that is compelling to me is being able to use an extended set of icons in the status bar, specifically so that an extension could use icons in StatusBarItems that are domain-specific, that would likely never be added as octicons.

Has there been any progress or discussion about adopting this proposal? I would be willing to put work towards this feature, if there's any way I can help.

aeschli commented 2 years ago

I've looked at the performance when there are many icon fonts. In my test setup I loaded 26 medium sized icon fonts (~36kB)

aeschli commented 2 years ago

In the API call we discussed that it's not nice to have a separate "iconFonts" section which is internal to the extension. Looking at this I think we can condense it. The icon path is enough, and we can require it to end with woff or tiff. There are currently no other font properties supported. If icon size becomes a requirement, we can add it to each icon.

    "icons": [
        {
            "id": "distro-ubuntu",
            "description": "Ubuntu icon",
            "default": {
                "fontPath": "./distroicons.woff",
                "fontCharacter": "\\E001"
            }
        },
        {
            "id": "distro-fedora",
            "description": "Fedora icon",
            "default": {
                "fontPath": "./distroicons.woff",
                "fontCharacter": "\\E002"
            }
        }
    ]

@jrieken @mjbvz ?

jrieken commented 2 years ago

Potential, small refinement which guarantees unique ids by JSON

    "icons": {
        "distro-ubuntu": {
            "description": "Ubuntu icon",
            "default": {
                "fontPath": "./distroicons.woff",
                "fontCharacter": "\\E001"
            }
        },
        "distro-fedora": {
            "description": "Ubuntu icon",
            "default": {
                "fontPath": "./distroicons.woff",
                "fontCharacter": "\\E002"
            }
        }
}
miguelsolorio commented 2 years ago

This broke the GitHub Copilot extension for a couple of days (see https://github.com/github/feedback/discussions/11684, not sure if we have a better way to notify extensions when the api changes) so suggested they update their icons contribution to:

        "icons": {
            "copilot-logo": {
                "description": "GitHub Copilot Icon",
                "default": {
                    "fontPath": "assets/copilot.woff",
                    "fontCharacter": "\\0041"
                }
            },
            "copilot-warning": {
                "description": "GitHub Copilot Warning",
                "default": {
                    "fontPath": "assets/copilot.woff",
                    "fontCharacter": "\\0042"
                }
            },
            "copilot-notconnected": {
                "description": "GitHub Copilot Not Connected",
                "default": {
                    "fontPath": "assets/copilot.woff",
                    "fontCharacter": "\\0043"
                }
            }
        },

@aeschli is there no longer a shorthand way to use iconFonts when defined? Feels a bit silly to have to specify the font path for every contributed icon (if you've got a lot this can be tedious)

aeschli commented 2 years ago

@misolori I filed an issue for copilot a week back (https://github.com/github/copilot/issues/1759)

The outcome of the API call was to remove the iconFonts contribution point as the contributed icon fonts were internal to the extension and more like a helper. No other contribution point is like that. Given that all that it describes was the path to the font and the format of the font can be deducted from the file name, we went with the font path. Maybe don't use the most complicated font path. It's not much different as with icon paths.