[Map] add polyline support #2340

sblondeau · 2024-11-04T21:19:29Z

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

Add polyline support to Map for Leaflet and GoogleMap (useful e.g. for itinerary drawing)

public function index(): Response
    {
        $map = (new Map('default'))
            ->center(new Point(45.7534031, 4.8295061))
            ->zoom(6)

            ->addPolyline(
                new Polyline(
                    title: 'my title',
                    points: [
                        new Point(48.8566, 2.3522),
                        new Point(45.7640, 4.8357),
                        new Point(43.2965, 5.3698),

                    ]
                )
            )
      ;          
        return $this->render('hom/index.html.twig', [
            'map' => $map,
        ]);;
    }

github-actions · 2024-11-04T21:28:09Z

📊 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)
Autocomplete
`controller.js`	`15.04 kB` / `3.62 kB`	`15.02 kB`^0% / `3.6 kB`^0%
Map
`abstract_map_controller.d.ts`	`3 kB` / `741 B`	`3.86 kB`^{+29% 📈} / `815 B`^{+10% 📈}
`abstract_map_controller.js`	`1.88 kB` / `610 B`	`2.32 kB`^{+23% 📈} / `682 B`^{+12% 📈}
Map (Bridge Google)
`map_controller.d.ts`	`2.07 kB` / `729 B`	`2.38 kB`^{+15% 📈} / `769 B`^{+5% 📈}
`map_controller.js`	`7.07 kB` / `1.67 kB`	`5.77 kB`^{-18% 📉} / `1.4 kB`^{-16% 📉}
Map (Bridge Leaflet)
`map_controller.d.ts`	`1.31 kB` / `544 B`	Removed
`map_controller.js`	`5.52 kB` / `1.77 kB`	Removed

…t/add-polylines

WebMamba · 2024-11-04T21:47:54Z

Hey, thanks a lot for this PR @sblondeau! Could you please add more info about this new feature to your PR description? Some screenshots, details and what you trying to achieve! Thannnnnks 😁

Kocal

Thanks for the feature, I didn't even know that "polyline" existed :)

I've added some comments that must be resolved if we want to merge your work.

Also, a lot of CI checks are failing, can you take a look please? Thanks!

Kocal · 2024-11-05T18:39:16Z

src/Map/doc/index.rst

+You can add Polylines, which represents a path made by a series of `Point` instances
+    $myMap->addPolyline(new Polyline(
+        points: [
+            new Point(48.8566, 2.3522),
+            new Point(45.7640, 4.8357),
+            new Point(43.2965, 5.3698),
+            new Point(44.8378, -0.5792),
+        ],
+        infoWindow: new InfoWindow(
+            content: 'A line passing through Paris, Lyon, Marseille, Bordeaux',
+        ),
+    ));


Can you also move this section under Add Polygons please? Thanks :)

Suggested change

You can add Polylines, which represents a path made by a series of `Point` instances

$myMap->addPolyline(new Polyline(

points: [

new Point(48.8566, 2.3522),

new Point(45.7640, 4.8357),

new Point(43.2965, 5.3698),

new Point(44.8378, -0.5792),

],

infoWindow: new InfoWindow(

content: 'A line passing through Paris, Lyon, Marseille, Bordeaux',

),

));

You can add Polylines, which represents a path made by a series of ``Point`` instances::

$myMap->addPolyline(new Polyline(

points: [

new Point(48.8566, 2.3522),

new Point(45.7640, 4.8357),

new Point(43.2965, 5.3698),

new Point(44.8378, -0.5792),

],

infoWindow: new InfoWindow(

content: 'A line passing through Paris, Lyon, Marseille, Bordeaux',

),

));

Kocal · 2024-11-05T18:41:52Z

src/Map/src/Bridge/Google/assets/src/map_controller.ts

+            element.addListener('click', (event: any) => {
+                if (definition.autoClose) {
+                    this.closeInfoWindowsExcept(infoWindow);
+                }
+                infoWindow.setPosition(event.latLng);
+                infoWindow.open(this.map);
+            });


It looks like this logic is the same for Polygon, maybe we can merge the two blocks together?

Kocal · 2024-11-05T18:44:18Z

src/Map/src/Twig/MapRuntime.php

@@ -70,7 +76,7 @@ public function renderMap(

    public function render(array $args = []): string
    {
-        $map = array_intersect_key($args, ['map' => 0, 'markers' => 0, 'polygons' => 0, 'center' => 1, 'zoom' => 2]);
+        $map = array_intersect_key($args, ['map' => 0, 'markers' => 0, 'polygons' => 0, 'polylines' => 0, 'center' => 1, 'zoom' => 2]);


I think there is an issue here, I don't fully remember why it has been done like this, but values should be sequential 0, 1, 2, 3, 4, ...

No need here, as it's then using named arguments with the spread.

But it's a good idea at least to prevent future errors if we change to some array_reverse :)

src/Map/CHANGELOG.md

src/Map/doc/index.rst

src/Map/src/Bridge/Google/tests/GoogleRendererTest.php

src/Map/src/Polyline.php

src/Map/src/Twig/UXMapComponent.php

src/Map/src/Bridge/Google/assets/dist/map_controller.js

smnandre · 2024-11-09T23:35:32Z

src/Autocomplete/assets/dist/controller.js

@@ -15,7 +15,7 @@ LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
 OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 PERFORMANCE OF THIS SOFTWARE.
 ***************************************************************************** */
-/* global Reflect, Promise, SuppressedError, Symbol */
+/* global Reflect, Promise, SuppressedError, Symbol, Iterator */


Let's revert this change (or see in another PR) to focus on one package :)

sblondeau · 2024-11-10T07:57:11Z

Hello, I tried a lot of things but some tests (js ans php) failed and I do not understand why :(
If someone has some time to investigate...
Thanks :-)

Kocal · 2024-11-11T21:20:03Z

Hi @sblondeau, where are stuck?

Please ensure to rebase your branch from the latest 2.x quite often, but also try to run some yarn scripts to easy your development:

yarn lint
yarn format
yarn workspaces foreach -Rp --from '{src/Map/assets,src/Map/src/Bridge/Google/assets,src/Map/src/Bridge/Leaflet/assets}' run build
yarn workspaces foreach -Rp --from '{src/Map/assets,src/Map/src/Bridge/Google/assets,src/Map/src/Bridge/Leaflet/assets}' run test

Also, please start to squash your commits, we don't want to see commits 1517fe1 (#2340) like this because it will spam the history for nothing (meaning, harder to blame if necessary).

Thanks!

sblondeau added 2 commits November 4, 2024 22:10

add polyline support

9e0ef18

update changelog

d9b2fc7

sblondeau requested a review from Kocal as a code owner November 4, 2024 21:19

carsonbot added Feature New Feature Status: Needs Review Needs to be reviewed labels Nov 4, 2024

Merge branch '2.x' into feat/add-polylines

90f4d3b

sblondeau added 2 commits November 4, 2024 22:35

Merge branch 'feat/add-polylines' of github.com:sblondeau/ux into fea…

608ec1f

…t/add-polylines

fix linter

d872427

sblondeau added 2 commits November 5, 2024 08:49

fix linter js

fde4dee

yarn build

67befe6

sblondeau changed the title ~~add polyline support~~ [Map] add polyline support Nov 5, 2024

Kocal requested changes Nov 5, 2024

View reviewed changes

carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Nov 5, 2024

Kocal added the Map label Nov 5, 2024