[Icons] Allow to add custom pre-renderers

[norkunas]

Q	A
Bug fix?	no
New feature?	yes
Issues	N/A
License	MIT

I want to add data-test-id="icon_name" attribute to all used icons, but there is no easy way without overriding whole IconRenderer and then duplicating the logic.

But IconRendererInterface and Icon is marked as internal and IDE complains..

I think adding ux_icons.pre_renderer tag is a nice extension point where you can control them.

[smnandre]

This is the thing i was going to do after #2156 😄

... but i'm not sure we want to open / tag this to be honest.
I had more in mind a config options, enablable, with prefix and/or sufix. wdyt ?

There is a big performance goal with ux:icons... and i'd like to not reproduce the Twig/Live component problem.

[norkunas]

Not sure how can you make a difference here,you still need to collect them,and with tag at least it's an easy way

[norkunas]

I had more in mind a config options, enablable, with prefix and/or sufix. wdyt ?

I don't understand this.

There is a big performance goal with ux:icons...

I know. If that becomes a problem then dev can override renderer and do all the things there,like I've done now.

[smnandre]

If that becomes a problem then dev can override renderer and do all the things there

The event system cannot be changed on Twig/Live component for BC reasons, and this make things hard for a lot of people.

So we'll see after we discuss and/or merge the icon configuration PR.

But i'm really not sure we would like to open to extensions in this place..
.. do you have other use cases in mind (than the icon i mean).. so i can have a better vision / see the advantages ?

[norkunas]

Yeah, I had to add multiple prerenderer's to avoid making many changes in migration.
One for testId attribute so if passed like in any other our components it check's if it's test env and then applies data-test-id and overrides the default given by the icon name.
Other are classes related shortcuts: fontSize="**" transform to class="SvgIcon SvgIcon-fontSize**"

Basically without this it's not possible to have this functionality without overriding main renderer

[smnandre]

I guess we will find a way to open while not slowing everything down :)

Couple of questions to precise the needs:

• Do they need "state", container ? Or are they all "static" ?
• Do they only "add" things or can they remove things ?
• Do they need the icon, the attributes values ? Or just the name ?

[norkunas]

Do they need "state", container ? Or are they all "static" ?

As i said I check kernel env determining If data-test-id must be rendered, so it cannot be static :)

Do they only "add" things or can they remove things ?

Currently doesn't remove anything

Do they need the icon, the attributes values ? Or just the name ?

Currently only attributes, but I see use-cases also with name

[smnandre]

Not sure how can you make a difference here,you still need to collect them,and with tag at least it's an easy way

I was going to hard-code one method to add "icone identity"-attribute (data-id="lu:circle") with some options to configure.

Still thinking to do it, but this is not your need here, clearly :)

[norkunas]

But this would increase output size if will be rendered always :) it could replace our testId attribute,just wouldn't be consistent with all our components :D

[smnandre]

Yes no worry i now understand your needs :)

[smnandre]

This is not possible, there is no plan to expose the Icon object for now :/

Do you see another way ?

[norkunas]

Icon object is currrntly passed down to prerenders without my changes,so I don't understand you and the reasoning

[smnandre]

The "prerenders" are currently internal functions hard-coded in the renderer. Currently again, this is nothing else than an implementation detail, that could change at any time.

So i'm open to discuss both, but it's not at all a minor thing

And if we expose the Icon object (internal for now) here, we both know this is a question of days before beeing asked, logically, to expose it elsewhere..

[norkunas]

Okay, so what do you prefer to expose if not the icon object? Why current prerenderers are getting the icon object?

[smnandre]

I'm sorry this is me not understanding right now...

You are talking about prerenderers like it was a thing documented, exposed, etc.

This "current prerender" (no plural) is right now one callable that is part of the internal mecanism of the IconRenderer, in a private method.

Again, i'm not against discussing it, but it seems fair to describe the current state objectively :)

[norkunas]

I can live with it if you decide to pass only attributes,but I see no harm if we'd have like I've suggested.

function __invoke(array $attributes) {
  $attributes['test'] = 'something';

  return $attributes;
}

vs

function __invoke(Icon $icon) {
  return $icon->withAttributes(['test' => 'something']);
}

To me the second solution looks more appealing than the first one, because intention is to modify the icon :)

[norkunas]

I'm sorry this is me not understanding right now...

You are talking about prerenderers like it was a thing documented, exposed, etc.

This "current prerender" (no plural) is right now one callable that is part of the internal mecanism of the IconRenderer, in a private method.

Again, i'm not against discussing it, but it seems fair to describe the current state objectively :)

Yeah, but you decided to pass the Icon, not me:)

[smnandre]

To a private internal function ;) And i know you know very well the difference 😉

[smnandre]

So indeed it would probably be (not saying "ok" for now, we need to discuss the impacts with the team) something like

callable(string, array): array 

with the icon name and the attributes passed during runtime

These attributes would then be added to the current ones..

But i'm realizing.... why don't just decorate the IconRenderer ?

[smnandre]

Just pushed this example of decoration (i'm sure you don't need an example to know how to do this, but if anyone comes reading this exchange ;) )

https://github.com/symfony/ux/blob/cd7cf7ff7c8db4f3aba54add9ee046df4a9025d9/ux.symfony.com/src/UX/IconRenderer.php