Skip to content

[Icons] Allow to add custom pre-renderers #2157

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[Icons] Allow to add custom pre-renderers #2157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Icons/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- Add `aliases` configuration option to define icon alternative names.
- Add support for `int` and `float` attribute values in `<twig:ux:icon />`.
- Allow to add custom pre-renderers

## 2.19.0

Expand Down
5 changes: 5 additions & 0 deletions src/Icons/config/services.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\DependencyInjection\Loader\Configurator;

use Symfony\UX\Icons\AriaHiddenPreRenderer;
use Symfony\UX\Icons\Command\WarmCacheCommand;
use Symfony\UX\Icons\IconCacheWarmer;
use Symfony\UX\Icons\IconRenderer;
Expand Down Expand Up @@ -58,6 +59,7 @@
service('.ux_icons.icon_registry'),
abstract_arg('default_icon_attributes'),
abstract_arg('icon_aliases'),
tagged_iterator('ux_icons.icon_pre_renderer'),
])

->alias('Symfony\UX\Icons\IconRendererInterface', '.ux_icons.icon_renderer')
Expand All @@ -79,5 +81,8 @@
service('.ux_icons.cache_warmer'),
])
->tag('console.command')

->set('.ux_icons.aria_hidden_pre_renderer', AriaHiddenPreRenderer::class)
->tag('ux_icons.icon_pre_renderer')
;
};
24 changes: 24 additions & 0 deletions src/Icons/src/AriaHiddenPreRenderer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\UX\Icons;

final class AriaHiddenPreRenderer implements IconPreRendererInterface
{
public function __invoke(string $name, Icon $icon): Icon
{
if ([] === array_intersect(['aria-hidden', 'aria-label', 'aria-labelledby', 'title'], array_keys($icon->getAttributes()))) {
return $icon->withAttributes(['aria-hidden' => 'true']);
}

return $icon;
}
}
5 changes: 5 additions & 0 deletions src/Icons/src/DependencyInjection/UXIconsExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Symfony\Component\HttpKernel\DependencyInjection\ConfigurableExtension;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\UX\Icons\Iconify;
use Symfony\UX\Icons\IconPreRendererInterface;

/**
* @author Kevin Bond <kevinbond@gmail.com>
Expand Down Expand Up @@ -128,5 +129,9 @@ protected function loadInternal(array $mergedConfig, ContainerBuilder $container
if (!$container->getParameter('kernel.debug')) {
$container->removeDefinition('.ux_icons.command.import');
}

$container
->registerForAutoconfiguration(IconPreRendererInterface::class)
->addTag('ux_icons.icon_pre_renderer');
}
}
17 changes: 17 additions & 0 deletions src/Icons/src/IconPreRendererInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\UX\Icons;

interface IconPreRendererInterface
{
public function __invoke(string $name, Icon $icon): Icon;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Do you see another way ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
28 changes: 6 additions & 22 deletions src/Icons/src/IconRenderer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@
*/
final class IconRenderer implements IconRendererInterface
{
/**
* @param iterable<IconPreRendererInterface> $preRenderers
*/
public function __construct(
private readonly IconRegistryInterface $registry,
private readonly array $defaultIconAttributes = [],
private readonly ?array $iconAliases = [],
private readonly iterable $preRenderers = [],
) {
}

Expand All @@ -42,30 +46,10 @@ public function renderIcon(string $name, array $attributes = []): string
->withAttributes($this->defaultIconAttributes)
->withAttributes($attributes);

foreach ($this->getPreRenderers() as $preRenderer) {
$icon = $preRenderer($icon);
foreach ($this->preRenderers as $preRenderer) {
$icon = $preRenderer($name, $icon);
}

return $icon->toHtml();
}

/**
* @return iterable<callable(Icon): Icon>
*/
private function getPreRenderers(): iterable
{
yield self::setAriaHidden(...);
}

/**
* Set `aria-hidden=true` if not defined & no textual alternative provided.
*/
private static function setAriaHidden(Icon $icon): Icon
{
if ([] === array_intersect(['aria-hidden', 'aria-label', 'aria-labelledby', 'title'], array_keys($icon->getAttributes()))) {
return $icon->withAttributes(['aria-hidden' => 'true']);
}

return $icon;
}
}
29 changes: 25 additions & 4 deletions src/Icons/tests/Unit/IconRendererTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
namespace Symfony\UX\Icons\Tests\Unit;

use PHPUnit\Framework\TestCase;
use Symfony\UX\Icons\AriaHiddenPreRenderer;
use Symfony\UX\Icons\Exception\IconNotFoundException;
use Symfony\UX\Icons\Icon;
use Symfony\UX\Icons\IconPreRendererInterface;
use Symfony\UX\Icons\IconRegistryInterface;
use Symfony\UX\Icons\IconRenderer;
use Symfony\UX\Icons\Tests\Util\InMemoryIconRegistry;
Expand Down Expand Up @@ -61,7 +63,7 @@ public function testRenderIconWithAttributes(): void
$registry = $this->createRegistry([
'foo' => '<path d="M0 0L12 12"/>',
]);
$iconRenderer = new IconRenderer($registry);
$iconRenderer = new IconRenderer($registry, [], [], [new AriaHiddenPreRenderer()]);
$attributes = ['viewBox' => '0 0 24 24', 'class' => 'icon', 'id' => 'FooBar'];

$svg = $iconRenderer->renderIcon('foo', $attributes);
Expand All @@ -74,7 +76,7 @@ public function testRenderIconWithDefaultAttributes(): void
$registry = $this->createRegistry([
'foo' => '<path d="M0 0L12 12"/>',
]);
$iconRenderer = new IconRenderer($registry, ['viewBox' => '0 0 24 24', 'class' => 'icon']);
$iconRenderer = new IconRenderer($registry, ['viewBox' => '0 0 24 24', 'class' => 'icon'], [], [new AriaHiddenPreRenderer()]);

$svg = $iconRenderer->renderIcon('foo');

Expand Down Expand Up @@ -172,7 +174,7 @@ public function testRenderIconWithAutoAriaHidden(string|array $icon, array $attr
$registry = $this->createRegistry([
'foo' => $icon,
]);
$iconRenderer = new IconRenderer($registry);
$iconRenderer = new IconRenderer($registry, [], [], [new AriaHiddenPreRenderer()]);

$svg = $iconRenderer->renderIcon('foo', $attributes);
$this->assertSame($expectedSvg, $svg);
Expand Down Expand Up @@ -237,7 +239,7 @@ public function testRenderIconWithAliases(): void
'bar' => '<path d="M0 BAR"/>',
'baz' => '<path d="M0 BAZ"/>',
]);
$iconRenderer = new IconRenderer($registry, [], ['foo' => 'bar']);
$iconRenderer = new IconRenderer($registry, [], ['foo' => 'bar'], [new AriaHiddenPreRenderer()]);

$svg = $iconRenderer->renderIcon('bar');
$this->assertSame('<svg aria-hidden="true"><path d="M0 BAR"/></svg>', $svg);
Expand All @@ -249,6 +251,25 @@ public function testRenderIconWithAliases(): void
$this->assertSame('<svg aria-hidden="true"><path d="M0 BAZ"/></svg>', $svg);
}

public function testRenderWithMultiplePreRenders(): void
{
$registry = $this->createRegistry([
'foo' => '<path d="M0 0L12 12"/>',
]);
$customPreRenderer = new class implements IconPreRendererInterface {
public function __invoke(string $name, Icon $icon): Icon
{
return $icon->withAttributes(['data-test-id' => $name]);
}
};
$iconRenderer = new IconRenderer($registry, [], [], [new AriaHiddenPreRenderer(), $customPreRenderer]);
$attributes = ['viewBox' => '0 0 24 24', 'class' => 'icon', 'id' => 'FooBar'];

$svg = $iconRenderer->renderIcon('foo', $attributes);

$this->assertSame('<svg viewBox="0 0 24 24" class="icon" id="FooBar" aria-hidden="true" data-test-id="foo"><path d="M0 0L12 12"/></svg>', $svg);
}

private function createRegistry(array $icons): IconRegistryInterface
{
$registryIcons = [];
Expand Down