Conversation
| public static readonly IconId CloseIcon = Gtk.Stock.Close; | ||
| public static readonly IconId ClosedFolder = "md-closed-folder"; | ||
| public static readonly IconId ClosedReferenceFolder = "md-closed-reference-folder"; | ||
| public static readonly IconId ClosedResourceFolder = "md-closed-resource-folder"; |
There was a problem hiding this comment.
This is a public API break. I think we need to keep the old names otherwise this will break any third party addins that use these Stock icons.
So we don't break the API.
|
Could you please review this PR? I'd like to know if this is fine before I do the same thing in |
| <StockIcon stockid="md-services-folder" resource="folder-services-16.png" size="Menu" /> | ||
|
|
||
| <!-- File icons --> | ||
| <StockIcon stockid="md-generic-file" resource="file-generic-16.png" size="Menu" /> |
There was a problem hiding this comment.
why the pattern (md-file-* -> md-*-file) change? Imo we should use the same pattern like we have for other categories (md-prefs-*, md-file-* etc.). Also the Stock IDs belong to the public API too, kind of. AddIns can reference and use them directly (instead of Gui.Stock). I remember that we changed some id's before and it was really hard for AddIn devs to track that, since we don't expose ALL icons throught Gui.Stock. @slluis?
There was a problem hiding this comment.
Right, I can use md-folder-* and md-file-*. The reason why I did it was that there's a huge chaos in IDs. Take a look: sometimes they're reversed, sometimes there's an -icon suffix. A mess.
There was a problem hiding this comment.
I might be wrong, but I really think we should clean up these IDs if we want to have more extensions in the marketplace. There should be a clear pattern, or it will look like they're glued together from different sources (which they in fact are). We might break some APIs today, but if we design a pattern now, it will pay off later in the longer run.
There was a problem hiding this comment.
I'm not against cleaning them up, I'm just for a unified pattern and for not breaking AddIns that use the IDs directly. And we should definitely remove the -icon suffix everywhere. If we don't care about the IDs to be changed without an API bump, then it's ok, otherwise we should sync it with the next API bump.
There was a problem hiding this comment.
If @slluis agrees to rename icon IDs, I'd make sure we have same pattern for all names. Plus I'd document it so it's super-clear.
slluis
left a comment
slluis
left a comment
There was a problem hiding this comment.
We can't remove existing icon ids, as this could break existing extensions. Before doing such a change, I'd like to have a document in the monodevelop wiki that explains what's the icon naming policy.
| public static readonly IconId ServicesFolder = "md-services-folder"; | ||
|
|
||
| // Deprecated folder icons | ||
| public static readonly IconId ClosedFolder = "md-folder"; |
There was a problem hiding this comment.
Nit: To make this easier in the future, in case we do other renames, I'd say the pattern should be:
public static readonly IconId Folder = "md-folder";
public static readonly IconId ReferenceFolder = "md-reference-folder";
...
public static readonly IconId ClosedFolder = Folder;
public static readonly IconId OpenFolder = Folder;
public static readonly IconId ClosedReferenceFolder = ReferenceFolder;
5 participants
We have a huge chaos in file and folder icons. I am trying to clean it up here.
As a part of https://github.com/xamarin/icons/issues/194 I cleaned up our special folder references. Results:
md-component-folder-*andmd-folder-assetsare unused. I'll have more icons to use soon, so we'll work on references afterward.Please don't merge, this is WIP.