Skip to content

[Map] Add possibility to configure extra options #2810

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

Closed
wants to merge 1 commit into from

Conversation

dannyvw
Copy link
Contributor

@dannyvw dannyvw commented Jun 2, 2025

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

TODO

Related to #2445 (comment)

Usage:

new LeafletOptions(extra: ['maxZoom' => 1])

@dannyvw dannyvw requested a review from Kocal as a code owner June 2, 2025 08:01
@carsonbot carsonbot added Feature New Feature Map Status: Needs Review Needs to be reviewed labels Jun 2, 2025
@carsonbot
Copy link

It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Cheers!

Carsonbot

Copy link
Contributor

github-actions bot commented Jun 2, 2025

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

FileBefore (Size / Gzip)After (Size / Gzip)
Map (Bridge Google)
map_controller.d.ts 2.73 kB / 828 B 2.78 kB+2% 📈 / 840 B+1% 📈
map_controller.js 11.18 kB / 2.53 kB 11.21 kB0% / 2.53 kB0%
Map (Bridge Leaflet)
map_controller.d.ts 2.04 kB / 672 B 2.07 kB+2% 📈 / 678 B+1% 📈
map_controller.js 9.71 kB / 2.59 kB 9.74 kB0% / 2.6 kB0%

@smnandre
Copy link
Member

smnandre commented Jun 4, 2025

As this is serialized, I guess we should make some minimal type check on the content of extra, no ?

cc @Kocal

@dannyvw
Copy link
Contributor Author

dannyvw commented Jun 5, 2025

@smnandre @Kocal Do you have something in mind?

Copy link
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

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

extra is already used for Marker and cie to let the developper pass extra data from the back to the front, but the difference with the actual system is that extra are not used by UX Map, these extra data are only used in ux:map:* events.

Instead, I suggest you to implement rawOptions on Map options front-side, nothing should be modified backend-side.

Thanks!

@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Jun 9, 2025
@dannyvw
Copy link
Contributor Author

dannyvw commented Jun 10, 2025

extra is already used for Marker and cie to let the developper pass extra data from the back to the front, but the difference with the actual system is that extra are not used by UX Map, these extra data are only used in ux:map:* events.

Instead, I suggest you to implement rawOptions on Map options front-side, nothing should be modified backend-side.

Thanks!

If we only change the front-side how can we set options in the backend which are passed to the front? We want to set some options to the map configuration based on configuration from Symfony DI. Example: for the vector layers we need to pass a layerName and apiKey.

@Kocal
Copy link
Member

Kocal commented Jun 22, 2025

If we only change the front-side how can we set options in the backend which are passed to the front? We want to set some options to the map configuration based on configuration from Symfony DI. Example: for the vector layers we need to pass a layerName and apiKey.

This could be done through extra property that you will intercept through ux:map:pre-connect event, and pass it to rawOptions which.. does not exist yet :)

You can't pass extra to new google.maps.Map() or L.Map().

I'm not fan how things are done for Map and options, raw options, etc, I will open a new PR asap.

Kocal added a commit that referenced this pull request Jun 23, 2025
…onnect` event (e.g.: `zoom`, `options`, `bridgeOptions`...) (Kocal)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[Map] Allows Map options customization in `ux:map:pre-connect` event (e.g.: `zoom`, `options`, `bridgeOptions`...)

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Docs?         | yes <!-- required for new features -->
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - For new features, provide some code snippets to help understand usage.
 - Features and deprecations must be submitted against branch main.
 - Update/add documentation as required (we can help!)
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

This feature allows users to override the options passed to `L.Map()` or `new google.maps.Map()` trough the `ux:map:pre-connect` event.

The configurable options are:
- zoom
- center
- options
- bridgeOptions, not injected by default, but can be defined on-the-fly

This is a blocking step for #2810, but also for one of my need when I wanted to persist the zoom/center in the URL, to share my map (https://hugo.alliau.me/places 😛) :

```js
    _onMapPreConnect = (event) => {
        const { L } = event.detail;

        if (window.location.hash) {
            try {
                const state = Object.fromEntries(new URLSearchParams(window.location.hash.slice(1)));
                const zoom = Number(state.z);
                const center = state.center.split(",").map(Number);

                event.detail.zoom = zoom;
                event.detail.center = L.latLng(center[0], center[1]);
            } catch (e) {
                console.error("Invalid state in URL hash:", e);
            }
        }
    }

    _onMapConnect = (event) => {
        const { map } = event.detail;

        const updateState = () => {
            const center = map.getCenter();
            const zoom = map.getZoom();

            const state = {
                z: zoom,
                center: [center.lat.toFixed(5), center.lng.toFixed(5)],
            };

            window.history.replaceState(state, "", `#${new URLSearchParams(state).toString()}`);
        };

        map.addEventListener("zoom", () => updateState());
        map.addEventListener("move", () => updateState());
    };
```

Commits
-------

afc7db6 [Map] Add missing doc for Circle and Rectangle
b0463b7 [Map] Allows Map options customization in `ux:map:pre-connect` event (e.g.: `zoom`, `options`, `bridgeOptions`...)
@Kocal
Copy link
Member

Kocal commented Jun 23, 2025

Replaced by #2863, thanks anyway for your contribution :)

@Kocal Kocal closed this Jun 23, 2025
@dannyvw dannyvw deleted the map-extra-options branch June 23, 2025 12:38
Kocal added a commit that referenced this pull request Jun 23, 2025
This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[Map] Add `extra` data to `Map`

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Docs?         | yes <!-- required for new features -->
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

Replace #2810

Like for `Marker` and cie, `extra` data can now be defined on the `Map` in order to pass extra data from your PHP code to your custom Stimulus Controller, through `ux:map:pre-connect` and `ux:map:connect` events.

Combo-ed with `bridgeOptions` from #2861, it means that you can fully customize the Map creation given extra data given your PHP code (e.g.: injecting some API keys):
```php
$map = new Map(extra: ['vector_layer_api_key' => 'bar']);
// or
$map->extra(['foo' => 'bar']);
```
```js
this.element.addEventListener('ux:map:pre-connect', (event) => {
    const { detail } = event;

    console.log(detail.extra); // {'foo': 'bar'}

    if (detail.extra.foo === 'bar') {
        detail.extra.bridgeOptions = { a_bridge_specific_option: 'foobar' };
    }
});
```

Commits
-------

0363d5d [Map] Add `extra` data to `Map`
b4b7f61 [Map] Reword PHPDoc about "extra" properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Map Status: Needs Work Additional work is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants