Better support for custom icons

[devoto13]

1. Document how to render custom icon using angular-fontawesome.
2. Investigate what can be done to improve support for custom icons without sacrificing auto complete for stock Font Awesome icons. E.g.

<fa-icon icon="my-custom-icon"></fa-icon>

will fail to compile, because my-custom-icon is not a known icon name. However if we change type to allow arbitrary string as an icon name we'll loose the auto complete for stock icons because of microsoft/TypeScript#29729.

[thepercival]

It would be very useful for me to add custom-icon support. My custom icons are now in a flaticon-collectio which is completely loaded when needed. Not the ideal way. Your library works very nice by the way. I am very pleased. If I can help in anyway to implement this issue, i am at your service.

[devoto13]

Thanks for kind words @thepercival!

In runtime world this just works. To support this feature in types world it is necessary to update type definitions for all FA packages ecosystem. I've been experimenting with it in https://github.com/devoto13/fa-icons-merge (last commit), but the current approach is pretty cumbersome.

I think the key to support this feature is some way of declaration merging, which will allow to add custom icons' names to the IconName and IconPrefix types. The key requirements are to not loose auto-completion for icon names and to not break FA users.

Unfortunately neither type unions, nor enums support declaration merging. So if you have any ideas how to make syntax for adding custom icons less cumbersome this should allow to move this feature forward.

[thepercival]

Hello,

I am not a tech expert and I hope I don't waste your time.

I took a look at the type-definitions. With custom icons the library does not know the names of the icons. That is obvious.
I wrote some lines of code which seems fine from the users-perspective(mine):

export type IconNameCustom = 'faMyCustomIcon1' | 'faMyCustomIcon2';

export class MyFaIconLibrary extends FaIconLibrary {
  getIconDefinition(prefix: IconPrefix, name: IconName | IconNameCustom): IconDefinition | null {
    if (prefix === 'fac') { /* 'fac' where c stands for custom */
      if (name === 'faMyCustomIcon2') {
        // createDefinition of faMyCustomIcon and return
      }
    }
    return super.getIconDefinition(prefix, <IconName>name);
  }
}

The custom icons will not show up in the autocompletion, but that should not be a problem for custom icons. Otherwise I do not see a solution.

Hopefully this gives you an idea of the users-perspective.

[devoto13]

This issue is really about the type safety and auto-completion for custom icon names on the usage site, i.e. <fa-icon icon="<IN HERE>"></fa-icon>.

If you're okey with some casts you can use custom icons already today without any code modifications: https://stackblitz.com/edit/angular-z8v4ux-i7gazy?file=src%2Fapp%2Fapp.module.ts.

You'll probably need to wrap it into $any() in the template as well, but StackBlitz does not trigger the error for some reason.

The custom icons will not show up in the autocompletion, but that should not be a problem for custom icons. Otherwise I do not see a solution.

The problem is that it will not just be missing from the compilation, but will also result in a type error during production build, because custom icon name is not assignable to the icon input property typed as IconName.

[jtinsley85]

I know we’re dealing with Angular here but, I was just reading through this and it made me think about how Material UI allows you to “Augment” types in custom theme implementations for React.js projects. Their documentation isn’t exhaustive when it comes to ALL the different use cases for this but would encourage you to check it out in case it could be a route to explore.

Scroll down till you see the “ declare module“ part.

https://material-ui.com/customization/palette/

[devoto13]

Thanks for the info @jtinsley85. This is exactly what I've been thinking about. Unfortunately such declarations are rather cumbersome. We only need icon name, but because declaration merging does not work with string literal unions we have to define such names as properties of the object.

[lightman76]

Just ran into this issue while upgrading my angular app. Has there been any progress/updates on this?

Wondering if instead of using the [icon] binding for the custom icons, instead add a [customIcon] binding for those with a more open typing. This would allow the [icon] binding to continue to use more strict types to enable autocomplete.

Based on what Devoto13 said above about it working today with some casts, it seems that both bindings could map to the same underlying property so hopefully wouldn't be too difficult to implement. I'm not very familiar with this codebase at this point, but if this sounds like an approach the maintainers would be ok with, I could take a stab at a pull request.

[devoto13]

@lightman76 I think a customIcon binding is an interesting approach, but then we'll also need to add a customMask as mask binding takes icon as well. I'm somewhat hesitant about broadening the API in this way as it essentially is just a "native" way to opt-out of the type checking. IMO when you're opting out of type checking it is better to do it explicitly in your own code using [icon]="$any('icon')" or in the custom icon definition if you use explicit reference approach.

I've been returning to this issue couple of times, but there are just two approaches: remove type safety and completion for the native icons or very cumbersome type definitions as I've pointed out in this comment. I'm leaning towards the latter approach, but hope that something will happen in TypeScript to make it less cumbersome 😄 Your proposal looks like a better alternative to completely removing the type safety at the cost of the API complexity and I'm not sure if we should accept this trade-off 🤔

It's worth noting that the main blocker for this to move forward is the icon library approach (where one would pass icon name as a string). It is pretty straightforward to define a more relaxed version of the IconDefinition type and allow to pass it to the icon and mask bindings. I was holding that off to avoid making custom icons work only in some cases, but maybe we should ship it, so at least some users can make fewer casts.

[lightman76]

@devoto13 Thanks for the consideration. I agree that having to have a separate binding just for type checking reasons isn't ideal, especially if it looks like there is a possible solution from typescript in the future to handle this.

I had some time to play around with this more and I was able to use the casting method successfully (at least so far - haven't released to production yet - still working on migrating the rest of the project.) We have a centralized definition for our custom icons, so that's the only place it seems I needed to worry with the casting. We import all our icons as constants instead of referring to them by name, so didn't need to do any casting in the actual usages of fa-icon (yay!)

In case it helps anyone else, here's how I've defined our icons with the casting.

export const myIcon:IconDefinition = { prefix: 'myCustomPrefix' as IconPrefix, iconName: 'myIcon ' as IconName, icon: [65,65,[],"f908","M28.643 ... svg data....z"] };

[fplusf]

Also allowing to add SVG attributes like "fill-rule" and "clip-rule" for the element would greatly enhance creating & using custom icons. Probably it makes sense to include such a field in the IconDeifnion interface.