-
Notifications
You must be signed in to change notification settings - Fork 153
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
Feature request: Add option for soft vs hard icon missing error #441
Comments
If this feature request is accepted, I’d like to point out that #440 also asks for configuration options that determine whether an error is thrown or a message is logged to console. For consistency, we should probably use the same identifiers in both cases, so #440’s options aren’t called 'throwError' and 'warnInConsole' while this issue’s options are 'throw' and 'logConsole'. |
Are you aware that you can use Rendering component without any icon is not a valid use, so the proposal of changing error to warning does not sound good to me. However, I would be open to add an opt-in warning when fallback icon is rendered or perhaps expose a I would guess you were hit by this issue when using icon library approach, because with explicit reference approach missing icon should be caught by type-checking. |
@devoto13 we use a mix of the icon library as well as strong typing (imports in the typescript and just html markup for the fa-icon tag). I tried the FaConfig.fallbackIcon, but it didn't work. I still got an error thrown by faWarnIfIconDefinitionMissing. So I'm guessing the fallbackIcon doesn't cover the icon library approach. I don't want to change the error to a warning, I'd like an opt-in config setting that faWarnIfIconDefinitionMissing() will respect where it will just write an error to the console. So the existing behavior of throwing an error will still be applied by default. |
I understand that you don't want to change the default, but I don't think that rendering an empty component (with a warning) is a good behaviour. Let me think a bit more about this issue. You're right that |
So my thinking is to add This should solve both use cases:
|
Describe the problem you'd like to see solved or task you'd like to see made easier
When an icon reference is missing, and you try to use that icon in markup, it will crash an Angular component.
I'd like an option to use a soft versus hard error. So that the function faWarnIfIconDefinitionMissing would either do a console.error or throw a new Error.
Is this in relation to an existing part of angular-fontawesome or something new?
It's related to Fort Awesome checking if an icon reference has been added.
What is 1 thing that we can do when building this feature that will guarantee that it is awesome?
It will make it awesome by ensuring the stability of an application's functionality in the event of a missing icon.
Why would other angular-fontawesome users care about this?
Others might care about this because right now a missing icon will crash a component. Others, like myself might prefer a component run without finding an icon, than crash because it can't find it.
On a scale of 1 (sometime in the future) to 10 (absolutely right now), how soon would you recommend we make this feature?
8 - because it should be relatively easy to implement, it's just adding an option for how the existing faWarnIfIconDefinitionMissing function operates.
Feature request checklist
Feature request: moar cowbell
)The text was updated successfully, but these errors were encountered: