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

[Map] markers, polygons and polylines removal #2547

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion src/Map/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# CHANGELOG


## 2.23
- Add property `$identifier` to marker, polygon and polyline
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Add property `$identifier` to marker, polygon and polyline
- Add property `$identifier` to `Marker`, `Polygon` and `Polyline` constructors

- Add method `Symfony\UX\Map::removeMarker(Marker $marker)`, to allow removal of a marker from its identifier
- Add method `Symfony\UX\Map::removePolygon(Polygon $polygon)`, to allow removal of a polygon from its identifier
- Add method `Symfony\UX\Map::removePolyline(Polyline $polyline)`, to allow removal of a polyline from its identifier

## 2.22

- Add method `Symfony\UX\Map\Renderer\AbstractRenderer::tapOptions()`, to allow Renderer to modify options before rendering a Map.
Expand Down
23 changes: 23 additions & 0 deletions src/Map/doc/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,29 @@ You can add markers to a map using the ``addMarker()`` method::
))
;

Remove elements from Map
~~~~~~~~~~~~~~~~~~~~~~~~

You can remove markers, polygons and polylines from a map using the ``removeMarker(Marker $marker)``, ``removePolyline(Polyline $polyline)`` and ``removePolygon(Polygon $polygon)`` methods::

When you create Marker, Polygon or Polyne, you can add an identifier in construct :

$departureMarker = new Marker (
position: new Point(45.7640, 4.8357),
title: 'Lyon',
identifier: 'departure'
)

then retrieve the marker with ``Map::getMarker(string $identifier)``, also works with ``Map::getPolygon(string $identifier)`` and ``Map::getPolyline(string $identifier)``. It can be useful to remove an element from its identifier.

$map->addMarker($departureMarker);

// remove marker with
$map->removeMarker($departureMarker)
// or
$map->removeMarker($map->getMarker('departure'));


Comment on lines +142 to +161
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
You can remove markers, polygons and polylines from a map using the ``removeMarker(Marker $marker)``, ``removePolyline(Polyline $polyline)`` and ``removePolygon(Polygon $polygon)`` methods::
When you create Marker, Polygon or Polyne, you can add an identifier in construct :
$departureMarker = new Marker (
position: new Point(45.7640, 4.8357),
title: 'Lyon',
identifier: 'departure'
)
then retrieve the marker with ``Map::getMarker(string $identifier)``, also works with ``Map::getPolygon(string $identifier)`` and ``Map::getPolyline(string $identifier)``. It can be useful to remove an element from its identifier.
$map->addMarker($departureMarker);
// remove marker with
$map->removeMarker($departureMarker)
// or
$map->removeMarker($map->getMarker('departure'));
It is possible to remove elements like ``Marker``, ``Polygon`` and ``Polyline`` instances by using methods ``Map::remove*()``::
// Add elements
$map->addMarker($marker = new Marker(/* ... */));
$map->addPolygon($polygon = new Polygon(/* ... */));
$map->addPolyline($polyline = new Polyline(/* ... */));
// And later, remove those elements
$map->removeMarker($marker);
$map->removePolygon($polygon);
$map->removePolyline($polyline);
If unfortunately you were unable to store an element instance, you can still remove them by passing the identifier string::
$map = new Map(/* ... */);
// Add elements
$map->addMarker(new Marker(identifier: 'my-marker', /* ... */));
$map->addPolygon(new Polygon(identifier: 'my-polygon', /* ... */));
$map->addPolyline(new Polyline(identifier: 'my-marker', /* ... */));
// And later, remove those elements
$map->removeMarker('my-marker');
$map->removePolygon('my-polygon');
$map->removePolyline('my-marker');

Add Polygons
~~~~~~~~~~~~

Expand Down
67 changes: 55 additions & 12 deletions src/Map/src/Bridge/Google/tests/GoogleRendererTest.php

Large diffs are not rendered by default.

63 changes: 48 additions & 15 deletions src/Map/src/Bridge/Leaflet/tests/LeafletRendererTest.php

Large diffs are not rendered by default.

171 changes: 154 additions & 17 deletions src/Map/src/Map.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\UX\Map;

use SplObjectStorage;
use Symfony\UX\Map\Exception\InvalidArgumentException;

/**
Expand All @@ -29,19 +30,18 @@ public function __construct(
/**
* @var array<Marker>
*/
private array $markers = [],
private SplObjectStorage $markers = new SplObjectStorage(),
Copy link
Member

@Kocal Kocal Feb 2, 2025

Choose a reason for hiding this comment

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

This is a nice improvement, but I would like to see this logic (with methods *fromArray() and *toArray()) extracted from Map to an other class.

Something like:

  • abstract class Elements, with methods like add, remove, toArray and fromArray
  • class Markers extends Elements
  • class Polygons extends Elements
  • class Polylines extends Elements

Anyway, changing things like that is a BC for users using new Map(markers: [new Marker()]:

We can change it to this, which is BC compatbile:

Suggested change
private SplObjectStorage $markers = new SplObjectStorage(),
array $markers = [],

Then, add a new property private Markers $markers.


/**
* @var array<Polygon>
*/
private array $polygons = [],
private SplObjectStorage $polygons = new SplObjectStorage(),

/**
* @var array<Polyline>
*/
private array $polylines = [],
) {
}
private SplObjectStorage $polylines = new SplObjectStorage(),
) {}

public function getRendererName(): ?string
{
Expand Down Expand Up @@ -86,23 +86,99 @@ public function hasOptions(): bool
return null !== $this->options;
}


public function getMarker(string $identifier): ?Marker
{
foreach ($this->markers as $marker) {
if ($this->markers->offsetGet($marker) === $identifier) {
return $marker;
}
}

return null;
}
Comment on lines +90 to +99
Copy link
Member

Choose a reason for hiding this comment

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

To remove (and similar methods too)


public function addMarker(Marker $marker): self
{
$this->markers[] = $marker;
$this->markers->attach($marker, $marker->identifier ?? $this->markers->getHash($marker));


return $this;
}

public function removeMarker(?Marker $marker): self
{
if ($marker === null) {
return $this;
}

if ($this->markers->contains($marker)) {
$this->markers->detach($marker);
}

return $this;
}
Comment on lines +109 to 120
Copy link
Member

Choose a reason for hiding this comment

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

To apply to similar methods too:

Suggested change
public function removeMarker(?Marker $marker): self
{
if ($marker === null) {
return $this;
}
if ($this->markers->contains($marker)) {
$this->markers->detach($marker);
}
return $this;
}
public function removeMarker(Marker|string $markerOrId): self
{
$this->markers->remove($markerOrId);
return $this;
}



public function getPolygon(string $identifier): ?Polygon
{
foreach ($this->polygons as $polygon) {
if ($this->polygons->offsetGet($polygon) === $identifier) {
return $polygon;
}
}

return null;
}

public function addPolygon(Polygon $polygon): self
{
$this->polygons[] = $polygon;
$this->polygons->attach($polygon, $polygon->identifier ?? $this->polygons->getHash($polygon));

return $this;
}

public function removePolygon(?Polygon $polygon): self
{
if ($polygon === null) {
return $this;
}

if ($this->polygons->contains($polygon)) {
$this->polygons->detach($polygon);
}

return $this;
}


public function getPolyline(string $identifier): ?Polyline
{
foreach ($this->polylines as $polyline) {
if ($this->polylines->offsetGet($polyline) === $identifier) {
return $polyline;
}
}

return null;
}

public function addPolyline(Polyline $polyline): self
{
$this->polylines[] = $polyline;
$this->polylines->attach($polyline, $polyline->identifier ?? $this->polylines->getHash($polyline));

return $this;
}

public function removePolyline(?Polyline $polyline): self
{
if ($polyline === null) {
return $this;
}

if ($this->polylines->contains($polyline)) {
$this->polylines->detach($polyline);
}

return $this;
}
Expand All @@ -124,12 +200,73 @@ public function toArray(): array
'zoom' => $this->zoom,
'fitBoundsToMarkers' => $this->fitBoundsToMarkers,
'options' => $this->options ? MapOptionsNormalizer::normalize($this->options) : [],
'markers' => array_map(static fn (Marker $marker) => $marker->toArray(), $this->markers),
'polygons' => array_map(static fn (Polygon $polygon) => $polygon->toArray(), $this->polygons),
'polylines' => array_map(static fn (Polyline $polyline) => $polyline->toArray(), $this->polylines),
'markers' => $this->markersToArray(),
'polygons' => $this->polygonsToArray(),
'polylines' => $this->polylinesToArray(),
Comment on lines +203 to +205
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'markers' => $this->markersToArray(),
'polygons' => $this->polygonsToArray(),
'polylines' => $this->polylinesToArray(),
'markers' => $this->markers->toArray(),
'polygons' => $this->polygons->toArray(),
'polylines' => $this->polylines->toArray(),

];
}

private function markersToArray(): array
{
foreach ($this->markers as $marker) {
$markers[] = $marker->toArray();
}

return $markers ?? [];
}

private static function markersFromArray(array $markers): SplObjectStorage
{
$markerObjects = new SplObjectStorage();
foreach ($markers as $marker) {
$markerObject = Marker::fromArray($marker);
$markerObjects->attach($markerObject, $markerObject->identifier);
}

return $markerObjects;
}

private function polygonsToArray(): array
{
foreach ($this->polygons as $polygon) {
$polygons[] = $polygon->toArray();
}

return $polygons ?? [];
}

private static function polygonsFromArray(array $polygons): SplObjectStorage
{
$polygonObjects = new SplObjectStorage();
foreach ($polygons as $polygon) {
$polygonObject = Polygon::fromArray($polygon);
$polygonObjects->attach($polygonObject, $polygonObject->identifier);
}

return $polygonObjects;
}

private function polylinesToArray(): array
{
foreach ($this->polylines as $polyline) {
$polylines[] = $polyline->toArray();
}

return $polylines ?? [];
}

private static function polylinesFromArray(array $polylines): SplObjectStorage
{
$polylineObjects = new SplObjectStorage();
foreach ($polylines as $polyline) {
$polylineObject = Polyline::fromArray($polyline);
$polylineObjects->attach($polylineObject, $polylineObject->identifier);
}

return $polylineObjects;
}

Comment on lines +209 to +268
Copy link
Member

Choose a reason for hiding this comment

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

To remove / externalize into Elements class


/**
* @param array{
* center?: array{lat: float, lng: float},
Expand Down Expand Up @@ -159,23 +296,23 @@ public static function fromArray(array $map): self
$map['fitBoundsToMarkers'] = false;
}

$map['markers'] ??= [];
$map['markers'] ??= new SplObjectStorage();
Copy link
Member

Choose a reason for hiding this comment

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

Should be reverted, $map['markers'] has not the same goal than Map::$markers. Same thing for similar code below

if (!\is_array($map['markers'])) {
throw new InvalidArgumentException('The "markers" parameter must be an array.');
}
$map['markers'] = array_map(Marker::fromArray(...), $map['markers']);
$map['markers'] = self::markersFromArray($map['markers']);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$map['markers'] = self::markersFromArray($map['markers']);
$map['markers'] = Markers::fromArray($map['markers']);

Copy link
Member

Choose a reason for hiding this comment

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

And same below


$map['polygons'] ??= [];
$map['polygons'] ??= new SplObjectStorage();
if (!\is_array($map['polygons'])) {
throw new InvalidArgumentException('The "polygons" parameter must be an array.');
}
$map['polygons'] = array_map(Polygon::fromArray(...), $map['polygons']);
$map['polygons'] = self::polygonsFromArray($map['polygons']);

$map['polylines'] ??= [];
$map['polylines'] ??= new SplObjectStorage();
if (!\is_array($map['polylines'])) {
throw new InvalidArgumentException('The "polylines" parameter must be an array.');
}
$map['polylines'] = array_map(Polyline::fromArray(...), $map['polylines']);
$map['polylines'] = self::polylinesFromArray($map['polylines']);

return new self(...$map);
}
Expand Down
12 changes: 8 additions & 4 deletions src/Map/src/Marker.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@
* use them later JavaScript side
*/
public function __construct(
private Point $position,
private ?string $title = null,
private ?InfoWindow $infoWindow = null,
private array $extra = [],
public Point $position,
public ?string $title = null,
public ?InfoWindow $infoWindow = null,
public array $extra = [],
public ?string $identifier = null,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public ?string $identifier = null,
public ?string $id = null,

Copy link
Member

Choose a reason for hiding this comment

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

Same for other elements

) {
}

Expand All @@ -38,6 +39,7 @@ public function __construct(
* title: string|null,
* infoWindow: array<string, mixed>|null,
* extra: array,
* identifier: string|null
* }
*/
public function toArray(): array
Expand All @@ -47,6 +49,7 @@ public function toArray(): array
'title' => $this->title,
'infoWindow' => $this->infoWindow?->toArray(),
'extra' => $this->extra,
'identifier' => $this->identifier,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'identifier' => $this->identifier,
'id' => $this->id,

You must also update TypeScript types definition to reflect this change.

];
}

Expand All @@ -56,6 +59,7 @@ public function toArray(): array
* title: string|null,
* infoWindow: array<string, mixed>|null,
* extra: array,
* identifier: string|null
* } $marker
*
* @internal
Expand Down
9 changes: 6 additions & 3 deletions src/Map/src/Polygon.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public function __construct(
private ?string $title = null,
private ?InfoWindow $infoWindow = null,
private array $extra = [],
) {
}
public ?string $identifier = null,
) {}

/**
* Convert the polygon to an array representation.
Expand All @@ -39,15 +39,17 @@ public function __construct(
* title: string|null,
* infoWindow: array<string, mixed>|null,
* extra: array,
* identifier: string|null
* }
*/
public function toArray(): array
{
return [
'points' => array_map(fn (Point $point) => $point->toArray(), $this->points),
'points' => array_map(fn(Point $point) => $point->toArray(), $this->points),
'title' => $this->title,
'infoWindow' => $this->infoWindow?->toArray(),
'extra' => $this->extra,
'identifier' => $this->identifier,
];
}

Expand All @@ -57,6 +59,7 @@ public function toArray(): array
* title: string|null,
* infoWindow: array<string, mixed>|null,
* extra: array,
* identifier: string|null
* } $polygon
*
* @internal
Expand Down
Loading
Loading