Allow webview panels to provide a custom icon

[mjbvz]

Allow webview panels to set an icon. This should probably be done declaratively in the package.json, something like:

"contributes": {
    "webviews": [
      {
        "viewType": "markdown.preview",
        "icon": {
          "light": "./media/Preview.svg",
          "dark": "./media/Preview_inverse.svg"
        }
      }
    ]
  
}

/cc @jrieken

[eamodio]

@mjbvz Since the title is provided dynamically in code, why not provide the icon there as well?

[jrieken]

Yeah, it's little weird to now start mixing declarative and symbolic information... @mjbvz why isn't this a property like the title?

[mjbvz]

The declarative approach has a few advantages:

1. Makes it clearer that you need per theme icons
2. We always know where to get the icon resource file from
3. We don't have to save the old icon or old icon path when restoring a webview after vscode restarts (if an icons path changes, such as during an extension update, we also would handle the change properly)

[jrieken]

Discussion ended in agreeing to disagree. There is the declarative vs in-code approach, arguments for the declarative are listed above, concerns against it are: (1) it uses package.json/contributes in a new way, not describing parts of the UI before loading code, but augmenting parts of the UI after loading & executing code. (2) it won't be possible to use a ThemeIcon or dynamic icons, (3) it feels asymmetric to provide properties like title et al in code but icon not. Last, the icon challenge (caching, fetching, transmitting) is something we already have today (activity bar icons, decorations with icons, tree items) and something we need to solve anyways.

[eamodio]

Probably not the best idea (because of the extra work, etc), but maybe support both? Support the declarative as spec'd above, which will be the default unless overridden in-code, where more features (ThemeIcon, dynamically changing) can be used. Maybe even add title to the declarative syntax for the same, so they are symmetric?

[mjbvz]

I still think a static contribution is the correct design.

1. The icon is used before code is loaded, for example after restarting vscode and a webview is first restored.

2. I don't think we want dynamic icons. I'm also not sure ThemeIcon is a good fit for webviews, at least how ThemeIcon is being used currently. Rather, to support themable icons, we could add a top level "webviews" entry to the file theme json, something like:

"webviews": {
      "markdown.preview": "_markdown_preview"
}

In fact, I believe that this would be required even if we did use ThemeIcon for the webviews icons.

3. The proposed package.json contribution is for a webview type: markdown preview, azure explorer, gitlens, and so on. The title and webview content on the other hand are instance properties on the panel itself. I believe the icon should be the same for all instances of a webview type, as this is consistent with how we display other "editor" types.

---

Two follow up ideas:

• Change the contribution point to webviewIcons to make it very clear what the intended use case of it is

• Move the contribution into themeIcons:

{
  "contributes": {
      "themeIcons": [
          {
              // no label or id, just always apply these icons but allow other icons themes to override them
               "path": "./icons/my-webview-icon-theme.json"
          }
      ]  
  }
}

with my-webview-icon-theme.json being something like:

{
  "iconDefinitions": {
     "_markdown_preview": { "iconPath": "..." }
  },
   "webviewPanels": {
        "markdown.preview": "_markdown.preview"
    }
}

/cc @sandy081

[sandy081]

I agree that web-view icon contribution is not a good fit into views contribution point. Moving it out makes sense to me. I would wait for @jrieken 's opinion about static vs dynamic.

[eamodio]

Any update on this?