appuniversum / ember-appuniversum

Ember addon wrapping the appuniversum components.
https://appuniversum.github.io/ember-appuniversum
MIT License
14 stars 11 forks source link

[Experimental] Add support for using components as icons #481

Closed piemonkey closed 5 months ago

piemonkey commented 5 months ago

[!WARNING]
This setup is still in an experimental state. We want to try it out in a couple of specific places before considering this ready to use by all the Appuniversum users. We might still break the setup in future minor versions until we announce it as stable.


Another PR in the line of PRs attempting to address the issue of inline SVGs.

This time the approach is to allow passing of arbitrary components to AuIcon (and by extension, any component taking an @icon argument).

The advantages of this method are that this is very flexible to the user. Also, by overloading @icon it avoids confusion that could be caused by adding a new argument, as only one of string or Component can be passed.

A minor downside is that the user can potentially pass a component that could cause rendering issues.

A risk is that in order to pass arguments to the component, for example; to pass an AuIcon as a custom component, you need an argument to specify which icon to use; you must use the component helper. In this way it is easy to write code that embroider can't statically analyse.

Windvis commented 5 months ago

Thinking about this some more. I don't think this will fix the problem in the frontend-embeddable-notule-editor project, since we also use the AuIcon component internally with some "hardcoded icons" and those would still look at the symbolset version. Making those configurable would be strange, since the icon is just an implementation detail of the component.

So while I still think accepting component for @icon is a good improvement (to allow custom icons) it still won't fix the problem you are seeing in the project. 🤔