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] Leaflet vector layers #2445

Open
dannyvw opened this issue Dec 11, 2024 · 8 comments
Open

[Map] Leaflet vector layers #2445

dannyvw opened this issue Dec 11, 2024 · 8 comments
Labels
Map RFC Status: Waiting Feedback Needs feedback from the author

Comments

@dannyvw
Copy link

dannyvw commented Dec 11, 2024

Is it possible to use vector layers like https://github.com/Esri/esri-leaflet-vector? What is the best way to implement this?

@dannyvw dannyvw added the RFC label Dec 11, 2024
@carsonbot carsonbot added the Map label Dec 11, 2024
@Kocal
Copy link
Member

Kocal commented Dec 12, 2024

Hi, I think yes, see https://symfony.com/bundles/ux-map/current/index.html#interact-with-the-map

  1. create your own Stimulus Controller and use-it to render the map (configure data-controller)
  2. in this controller, import esri-leaflet
  3. in this controller, listen to the event ux:map:connect, access L and map instances from event.details

@Prometee
Copy link

Prometee commented Dec 30, 2024

Hi @Kocal!

I tried what you explained with another leaflet plugin (Leaflet.Editable) and I ended up retrieving L var using window.L because event.details.L is not the same variable and does not have the plugin instantiated.

import {Controller} from "@hotwired/stimulus";
import 'leaflet-editable';

//  listen to the event ux:map:connect and pre-connect to set editable: true

_onConnect(event) {
        const { map, L } = event.detail;
        
        // L.Editable does not exist
}
import {Controller} from "@hotwired/stimulus";
import 'leaflet-editable';

//  listen to the event ux:map:connect and pre-connect to set editable: true

_onConnect(event) {
        const { map } = event.detail;
        const L = window.L;
        
        // L.Editable exists
}

Do you see why it's not the same variable ?

@Kocal
Copy link
Member

Kocal commented Jan 4, 2025

Hi, and sorry for the late reply.

It is not the same variable because:

  1. in Symfony UX, we import use import * as L from 'leaflet' (https://github.com/symfony/ux/blob/2.x/src/Map/src/Bridge/Leaflet/assets/src/map_controller.ts#L10) which create a new object
  2. but in leaflet-editabe, it use import L from 'leaflet' (https://cdn.jsdelivr.net/npm/[email protected]/+esm)

Which can be equivalent, but not identical:

    _onConnect(event) {
        console.log(event.detail.L); // Module
        console.log(window.L); // Object
        console.log(event.detail.L === window.L);
    }
image

If I edit the Leaflet map_controller.ts to use import L from 'leaflet', then it references the same object and event.detail.L.Editable exists :
image

I'm not sure how to fix it correctly, because here both syntax are valid.
Maybe, in the Leaflet case, we can use import L? Can it break things if we switch to this syntax?

@Kocal Kocal added the Status: Waiting Feedback Needs feedback from the author label Jan 4, 2025
@Prometee
Copy link

Prometee commented Jan 4, 2025

I don't know the implication of this change but reading the Leaflet doc for creating a plugin it says require('leaflet') which is the same as import L from 'leaflet', right?
https://github.com/Leaflet/Leaflet/blob/main/PLUGIN-GUIDE.md#module-loaders

Then I guess most of the plugin creator will follow this.

@Kocal
Copy link
Member

Kocal commented Jan 4, 2025

Well, yes and no, require() and UMD things are relic from the past and we now use ESM import/export system.

When you type import L from 'leaflet', L is the value of the default export (export default ...), but when you use import * as L from 'leaflet' it imports every exportable members and put them under L, which are not the same.

To me, the leaflet-editable API is broken, it should not implant itself under L namespace, but expose bunch of methods that accepts L (or L.*) instances.

@Kocal
Copy link
Member

Kocal commented Jan 4, 2025

Also, note that even if we change import * as L from 'leaflet' to import L from 'leaflet' (which I don't really want), the library still won't work because it requires the option editable: true to be passed to L.Map() options, which is not possible (yet?).

@Prometee
Copy link

Prometee commented Jan 4, 2025

@Kocal about the options : setting a custom options is not easy since the PHP class LeafletOptions only allows tileLayer option to be set. That's why in my case I set it during the pre-connect event.

Thank you very much for this explanations it's now very clear and I hope this issue will helps other devs creating their own Stimulus Controller.

@Kocal
Copy link
Member

Kocal commented Jan 5, 2025

For the L.Map options (and google.maps.Map for Google Maps), we can implement the same behavior than markers, infoWindows (...) , with the rawOptions options.

export default class extends Controller {
    connect() {
        this.element.addEventListener('ux:map:pre-connect', this._onPreConnect);
    }

    disconnect() {
        this.element.removeEventListener('ux:map:pre-connect', this._onPreConnect);
    }

    _onPreConnect(event) {
        event.detail.rawOptions = {
            editable: true,
            // other custom options if you want
        }
    }
}

About importing Leaflet inside a ESM context, I am unable to see on leaflet documentation what is the "official": import * as L from 'leaflet' or import L from 'leaflet'? On Google/StackOverflow I see both syntaxes.

Finally, for the plugin API issues, to me a good plugin should not automatically register itself, but explicitly by passing a L instance (so only the developer is responsible to import leaflet):

import {Controller} from "@hotwired/stimulus";
import {install as installLeafletEditable} 'leaflet-editable';

export default class extends Controller {
    connect() {
        // ...
        this.element.addEventListener('ux:map:connect', this._onConnect);
    }

    disconnect() {
        // ...
        this.element.removeEventListener('ux:map:connect', this._onConnect);
    }

    _onConnect(event) {
        installLeafletEditable(event.detail.L);
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Map RFC Status: Waiting Feedback Needs feedback from the author
Projects
None yet
Development

No branches or pull requests

4 participants