Conversation
Co-authored-by: Copilot <copilot@github.com>
| icon: app.icon, | ||
| }, | ||
| this.client.getAxiosInstance().defaults.baseURL, |
There was a problem hiding this comment.
I think we should pass the resolved icon instead of the baseUrl here (esp. since it should always be defined). That way we do not leak abstraction into the AppStatusTreeItem which shouldn't be aware of those details
| const trimmed = icon.trim(); | ||
| if (/^[a-zA-Z][a-zA-Z\d+\-.]*:/.test(trimmed)) { | ||
| return vscode.Uri.parse(trimmed); | ||
| } | ||
|
|
||
| if (!baseUrl) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
This is not needed, the URL implements WHATWG resolution semantics, so:
new URL("https://x/y.svg", "https://base/").toString() // "https://x/y.svg"
new URL("/icon.svg", "https://base/").toString() // "https://base/icon.svg"
new URL("data:image/png;base64,...", "https://base/").toString() // unchanged
| } { | ||
| return { | ||
| light: vscode.Uri.file( | ||
| path.join(__dirname, "..", "..", "media", "logo-black.svg"), |
There was a problem hiding this comment.
This seems wrong now, it used to be __filename then traversing up by two levels but now it's __dirname but by two levels as well
| super(app.name, vscode.TreeItemCollapsibleState.None); | ||
| this.id = app.id; | ||
| this.description = app.name; | ||
| this.description = app.workspace_name; |
There was a problem hiding this comment.
Honestly name here is confusing, this is status.message which is why it was rendered in the description (to make it less prominent). How would this currently look like actually?
If we want the status to be more prominent here then maybe we just show app.display_name ?? app.slug in the description? Since the workspace name is already the parent (or grandparent) so no need to repeat it
Updated workspace tree items to resolve app/workspace icon strings into VS Code URIs and restored the default logo fallback when the icon is missing or invalid.
logo-black.svg/logo-white.svgfallback.