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

make EdgeMarker track a single coronate or marker #16

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kwibus
Copy link
Contributor

@kwibus kwibus commented Jun 8, 2017

original EdgeMarker would add edgemarkers for all layers with getLatLng.
now its tracks a single coordinate with:
L.EdgeMarker(coordinate,options);
or a single marker with:
marker.bindEdgemarker(options);

kwibus added 3 commits June 8, 2017 15:24
original EdgeMarker would add edgemarkers for all layers with getLatLng.
now its tracks a single coordinate with:
    L.EdgeMarker(coordinate,options);
or a single marker with:
    marker.bindEdgemarker(options);
is no longer used
@ubergesundheit
Copy link
Owner

How would I create EdgeMarkers for a whole layer with this?

@kwibus
Copy link
Contributor Author

kwibus commented Jun 10, 2017

In the demo.html i included in all pull request (was not the smartest idee to include those in pull request, You should probably remove those)
I did it similar how it is done now

for (var l in map._layers) {
  if (typeof map._layers[l].getLatLng !== 'undefined') {
    map._layers[l].bindEdgeMarker({icon:icon});
   }
}

I could make it so you have

map.addEdgeMarkers 
map.removeEdgeMarkers

or/and something like this:

var layergroup = map.makeEdgeMarkers.addTo(map)

So you do something different then only remove. Like changing options

@kwibus
Copy link
Contributor Author

kwibus commented Jun 10, 2017

might be better to use

map.eachLayer(function(layer){
   if (layer.getLatLng !== 'undefined') {
    layer.bindEdgeMarker(option);
   }
});

And if you follow what leaflet is doing with bindTooltip and bindPopup, you would say it not necessary to make dedicated function for that.

@ubergesundheit
Copy link
Owner

I think its better to leave it as it is because of backward compatibility reasons. I'm not ready to completely change the way the plugin works..

@kwibus
Copy link
Contributor Author

kwibus commented Jun 11, 2017

If you like? I Think it's possible to make it more backwards compatible.
keep behavior of L.edgeMarker the same while keeping bindEdgmarker
if you are interested, I am prepared to look in to that.
how the plugin works will still change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants