Skip to content

Icon Aliases #29

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

Closed
natewiebe13 opened this issue Apr 5, 2021 · 6 comments · Fixed by #42
Closed

Icon Aliases #29

natewiebe13 opened this issue Apr 5, 2021 · 6 comments · Fixed by #42
Milestone

Comments

@natewiebe13
Copy link
Collaborator

It might be useful to assign aliases to icons. This can be especially useful if during a project you want to change an icon that's used across multiple templates/components.

@natewiebe13 natewiebe13 added this to the v2 milestone Apr 5, 2021
@Pixelrobin
Copy link
Owner

I would push back on this... I feel like it can lead to confusion down the line. There are other ways outside the library that this can be handled. Are there any icon libraries that allow you to do this?

@natewiebe13
Copy link
Collaborator Author

This is a practice we use for things, especially colors. Typically in a project we start off with icons for things, but then as we go, the client or designer might want to change the icon used for something. This is typically something between x vs x-circle, plus vs plus-circle, or trash vs trash-2. Doing a search and replace isn't terrible unless the icon name isn't super explicit, like x. But this can lead to missing an icon here or there.

In order to avoid that situation, we use aliases (or contexts as we call them) for things. So instead of using something like trash, trash-2, x or x-circle, we'd use something like delete or remove. So if they want to change the icon used for delete buttons, it's an easy alias change.

Another use case I thought of would be for if/when feather-icons renames the icons. We could likely leverage aliases to provide a backwards compatible version, at least as a temporary measure.

How I would implement this is just an empty alias array that could be used in a project to map strings to icons. If you try to register an alias that has an icon with the same name, an exception would be thrown.

That being said, a similar approach could be achieved with #18, the only drawback would be the icons array being larger, but at this point it's pretty negligible.

In order to avoid confusion and since the size difference is likely not worth it, I'd be okay to close this in favor of #18. @Pixelrobin I'll assign to you for the final call on this one.

@Pixelrobin
Copy link
Owner

Ah, I slightly misunderstood what you originally meant. My biggest concern was the possibility of one icon name returning something completely different and unexpected. anchor should never return the feather icon. But checking for existing icon names and throwing exceptions makes sense. So I'll say let's do it.

As a suggestion to avoid too much complexity: what if instead of a separate alias array there could be a optional icon object alias key? That way we can use the add icon API for both features and it would avoid needing to check both icons and aliases when adding either.

$icons->addIcon('delete', ['alias' => 'x'])

Also side note: would it be on this project to ship aliases for icons out of the box in the case that feather does rename an icon?

@Pixelrobin
Copy link
Owner

Pixelrobin commented Apr 8, 2021

Thought about it overnight, and I guess sharing the same array for icons and aliases wouldn't be ideal for some cases. For instance if you want to list all icons, you might not want to have "repeated" icons in the list. A separate alias array would be a simpler option on second thought.

@natewiebe13
Copy link
Collaborator Author

I think if we add #18, it would be nice to include aliases in one go, but internally, I agree. I think it makes sense to store as a separate array. We can tackle this after we start to nail down the API.

@natewiebe13
Copy link
Collaborator Author

Regarding if it would be on us to add aliases, I think we'd make that call if/when that comes up. IMO if we wanted to not release a major version (since renaming an icon is a backwards compatibility break), we would add aliases, but there's no reason that we couldn't just release a new major and include migration notes like we'll be doing for v2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants