[Map] Add Marker Icon customization capability

[sblondeau]

Q	A
Bug fix?	no
New feature?	yes
Issues	Fix #2109
License	MIT

A nicely asked feature, adding Marker icon customization, it can be an UX Icon, an URL, or a SVG content:

// It can be a UX Icon (requires `symfony/ux-icons` package)...
$icon = Icon::ux('fa:map-marker')->width(24)->height(24);
// ... or an URL pointing to an image
$icon = Icon::url('https://example.com/marker.png')->width(24)->height(24);
// ... or a plain SVG string
$icon = Icon::svg('<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24">...</svg>');

$map->addMarker(new Marker(
    // ...
    icon: $icon
));

Rendering

[github-actions]

📊 Packages dist files size difference

Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
Please review the changes and make sure they are expected.

File	Before (Size / Gzip)	After (Size / Gzip)
Map
abstract_map_controller.d.ts	4.43 kB / 1023 B	4.95 kB+12% 📈 / 1.15 kB+15% 📈
abstract_map_controller.js	3.88 kB / 1.02 kB	3.97 kB+2% 📈 / 1.07 kB+4% 📈
Map (Bridge Google)
map_controller.d.ts	2.55 kB / 794 B	2.73 kB+7% 📈 / 828 B+4% 📈
map_controller.js	10.16 kB / 2.25 kB	11.16 kB+10% 📈 / 2.52 kB+12% 📈
Map (Bridge Leaflet)
map_controller.d.ts	1.9 kB / 647 B	2.03 kB+7% 📈 / 667 B+3% 📈
map_controller.js	8.58 kB / 2.37 kB	9.63 kB+12% 📈 / 2.58 kB+9% 📈

[Kocal]

Hey, and thanks for the PR!
I didn't look at the code yet, but by any chance did you see #2109? we had discussed a lot about what we wanted and how.

Thanks!

[sblondeau]

Hey, and thanks for the PR! I didn't look at the code yet, but by any chance did you see #2109? we had discussed a lot about what we wanted and how.

Thanks!

I have read it some time ago. I have kept in mind that we should be able to use image url OR svg (OR directly Ux-icon). It was the goal, but maybe I only did a small first step ;-)

[Kocal]

Thanks for working on this!

About the IconFactory, we want to keep things simple to use for the user, and so we should not expose it to the user. Instead, we can create Icon named constructor:

• Icon::fromUrl(url: string, width: int, height: int)
• Icon::fromInlineSvg(html: string, width: int, height: int)
• Icon::fromUxIcon(name: string, width: int, height: int)

For each named constructor, I think we must create distinct child classes like Icon\Url, Icon\InlineSvg, Icon\Ux, to keep things cleaner, and it will also helps for later...

Finally, to render a Icon\Ux with the UxIconRenderer, I think it should be done at rendering time in \Symfony\UX\Map\Renderer\AbstractRenderer::getMapAttributes(), or maybe a bit before, to transform an Icon\Ux to Icon\InlineSvg.

Thanks!

[sblondeau]

@Kocal thanks for the review, I've just send an update. Let me know if you have other remarks.
A test is still failing, but it's in TwigComponent, I do not understand why, if you have any idea...
Thanks :-)

[Kocal]

Thanks, it's not far from being perfect, just a last round and it should be fine

[Kocal]

I believe we want to throw an error if the user wanted to use an UxIcon but without symfony/ux-icons, wdyt?

Suggested change

	if (null !== $this->uxIconRenderer && null !== $marker['icon'] && 'ux-icon' === $marker['icon']['type']) {
	$attrs['markers'][$key]['icon']['content'] = $this->uxIconRenderer->render($marker['icon']['content']);
	$attrs['markers'][$key]['icon']['type'] = 'inline-svg';
	}
	if (UxIcon::TYPE === ($marker['icon']['type'] ?? null)) {
	if (!$this->uxIconRenderer) {
	throw new ...;
	}
	$attrs['markers'][$key]['icon']['content'] = $this->uxIconRenderer->render($marker['icon']['content']);
	$attrs['markers'][$key]['icon']['type'] = InlineSvg::TYPE;
	}

[Kocal]

(and it requires to add TYPE constants to InlineSvg, UxIcon, etc.. classes, we don't want to hardcode them)

[sblondeau]

We already throw a similar exception in UxIconRenderer, it might be redundant ?

[Kocal]

But so, $uxIconRenderer property does not need to be nullable right?

[Kocal]

Don't worry for the failing check on TwigComponent, that's not related

[Kocal]

I've pull your PR locally and played a bit with what you've done, but unfortunately some things are broken:

• on Leaflet, there is a white square around SVG icons, IIRC that's because it uses a default CSS class that we should override:

• it does not render anything on Google Maps, due to an error Cannot read properties of undefined (reading 'parseFromString'):

There are also some changes that were not totally applied, don't worry I'm on it!

[Kocal]

PR's description updated

[Kocal]

I reworked many minor things (see commits), here is what the following code renders:

$map = (new Map())
    ->center(new Point(48.8566, 2.3522))
    ->zoom(6)
;

$cities = require_once __DIR__.'/../../config/cities.php';
foreach (array_slice($cities, 0, 500) as $i => $city) {
    $map->addMarker(new Marker(
        position: new Point($city['latitude'], $city['longitude']),
        title: $city['label'],
        icon: match(true) {
            $i < 150 => Icon::ux('fa:map-marker'),
            $i < 325 => Icon::url('https://cdn.jsdelivr.net/npm/bootstrap-icons@1.11.1/icons/geo-alt.svg'),
            default => Icon::svg(file_get_contents(__DIR__.'/../../assets/icons/el/map-marker.svg')),
        }
        
    ));
}

For Google
and Leaflet

I will merge tomorrow I think

[sblondeau]

Great job @Kocal ! Thanks for this final touch :)

[Kocal]

Thank you @sblondeau.