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.

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

Remove ICommandAction#iconClassName #85673

Closed
jrieken opened this issue Nov 27, 2019 · 4 comments
Closed

Remove ICommandAction#iconClassName #85673

jrieken opened this issue Nov 27, 2019 · 4 comments
Assignees
Labels
debt Code quality issues

Comments

@jrieken
Copy link
Member

jrieken commented Nov 27, 2019

This got added via c9f9081 and is a no, no because it a) weirdly competes with iconLocation give consumer headaches what to use and b) won't work for the Mac Touch Bar (which is the reason this originally has been added). In essence this must go again and we should use uri until it comes to rendering

@jrieken
Copy link
Member Author

jrieken commented Nov 27, 2019

fyi - with #85635 we are adding support to have uris like this vscode-icon://codicon/XYZ That means codicon can then be used everywhere, just as uris.

@jrieken
Copy link
Member Author

jrieken commented Nov 27, 2019

Adding @eamodio since the removal could be a side effect of #85635

@jrieken jrieken self-assigned this Nov 29, 2019
@jrieken
Copy link
Member Author

jrieken commented Nov 29, 2019

I am preparing a change for #84695 and I am cleaning this up along the way. A command icon is now defined like so: icon?: { dark?: URI; light?: URI; } | ThemeIcon;, a sample for codicons is icon: { id: "codicon.go-to-file" }

@miguelsolorio
Copy link
Contributor

@jrieken thanks for cleaning this up (and sorry for introducing this the wrong way)!

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues
Projects
None yet
Development

No branches or pull requests

3 participants