Skip to content
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

Custom icon rendering to support SVG icons #1860

Open
osre77 opened this issue Dec 17, 2024 · 8 comments
Open

Custom icon rendering to support SVG icons #1860

osre77 opened this issue Dec 17, 2024 · 8 comments

Comments

@osre77
Copy link
Contributor

osre77 commented Dec 17, 2024

Hi,
i was looking into a solution to support SVG icons along with the font icons used by Radzen.

I made a rapid prototype that uses an interface for icons that offers one method to return a RenderFragment for the icon.
CustomIconRendererPrototypeDiff

The idea is to replace all the string Icon properties with IRadzenIcon Icon.
Instead of having the hardcoded <i> element, the interface implementation provides the RenderFragment to render.

My goal was to make it code compatible by implementing an implicit cast operator from string to IRadzenIcon, but that failed because the razor files do not recognize the icon names as string anymore once the property type is not string.
This is why for the prototype I added the IconX property to the RadzenIcon. If it is set it will be rendered instead of the <i> element.

I made an implementation for the FontIcon. The MaterialIcons class offers all known standard icons (just a subset in the prototype). This class would best be generated with a source generator. Using this class instead of just the string for the icon name has the advantage to offer auto completion in the editor.

I also added a SvgIcon implementation that loads the SVG content from an embedded resource and makes some adjustments to it for sizing and tinting.
This solution supports sizing and coloring of the icon just like the font icons. Using the <img> element and load the file from wwwroot would not allow coloring and probably would have sizing issues as well.

The only downside I can see here is that if the string Icon in all components is replaced with IRadzenIcon Icon, all existing code out there would break. My solution in the prototype with the additional property would make it a non-breaking change but might be a bit confusing to use. It also requires more changes to all components with icons.

This prototype is by no means something to merge into the Radzen codebase. It is just a prototype/prove of concept.
If you say you would like to have this in Radzen, and maybe have some more ideas as well, I would happily continue development to a releasable state.

@akorchev
Copy link
Collaborator

Hi @osre77,

I like the idea in general but I am not sure why this is a feature of RadzenIcon. To be honest it doesn't use anything from it and seems to just complicate things.

@osre77
Copy link
Contributor Author

osre77 commented Dec 17, 2024

I just used RadzenIcon for my prototype. The idea is to use IRadzenIcon everywhere where icons are currently used, like RadzenButton, ... If it were just for standalone SVG icons a SvgIcon component would be the easier solution.
But if you want custom icons on all the other components you currently must not use the icon and put it in the content if the component supports templated content.

@akorchev
Copy link
Collaborator

How would this be used in RadzenButton for example? Wouldn't it require a new property? The diff doesn't show such usage at the moment.

@osre77
Copy link
Contributor Author

osre77 commented Dec 17, 2024

If implemented as a braking change then the RadzenButton.Icon property would be of type IRadzenIcon.

And you would use it like that:

<RadzenButton Icon="@MaterialIcons.Close" ... />
or
<RadzenButton Icon="@MySvgIcons.SomeAwsomeIcon" ...>

where MaterialIcons.Close and MySvgIcons.SomeAwsomeIcon are IRadzenIcon implementations.
but the old

<RadzenButton Icon="close" ... />

Would cause a compilation error.
I tried to solve that with an implicit cast from string to IRadzenIcon, but razor files don't know that "close" is a sting if the property is not a string.

As a non-breaking change a new property like IRadzenIcon IconXwould be added to RadzenButton:

<RadzenButton IconX="@MaterialIcons.Close" ... />
or
<RadzenButton IconX="@MySvgIcons.SomeAwsomeIcon" ...>

Using both Icon and IconX would favor one over the other.

@akorchev
Copy link
Collaborator

The thing is a lot of the components are not using RadzenIcon but simply render the <i /> element. RadzenButton does the same. Support all this would be quite an endeavour with a lot of changes. This all could be easily tackled by setting the child content where required:

<RadzenButton><SvgIcon />Click me</RadzenButton>

@osre77
Copy link
Contributor Author

osre77 commented Dec 17, 2024

I know that all components with icons would have to be touched: I would offer to do that, either as a breaking change or with the alternative property for the icon.

And I'm also aware that the SVG icon could simply be put into the content.

But I would prefer if the icons would just work as is with the property but is not locked to font icons.
But if you don't want that big change (especially the breaking one) I can understand that.
This is why I wanted to discuss this before I put in the effort.

@akorchev
Copy link
Collaborator

Honestly I think this is too much of an effort and quite a big of a change. The workaround is simple enough - to use the content of the component. So let's not do that but use the child content instead.

@osre77
Copy link
Contributor Author

osre77 commented Dec 17, 2024

OK, thank you for your input.
I'll scrap my fork then.

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

No branches or pull requests

2 participants