remove 'getLatLng' property from PointSymbolizer - references #148

[chriszrc]

The error is visible on the official vectorgrid demo pages:

http://leaflet.github.io/Leaflet.VectorGrid/demo-points-icons.html

Just open the console and mouse over any of the markers. The error precludes any mouseover or click events on point features.

My Diagnosis:
Point Symbolizer instances inherit from marker or circlemarker, but they lack latlng information, and on mouseover or click, leaflet will error on these features if it finds the getLatLng method, since that indicates it should be treated as a real marker instance, but even though the method is there, the value isn't, so leaflet produces an error.

Maybe these instance should have a latlng, I don't know, this was just an easy fix so that I could continue to use vectorgrid protobuf support with points and have mouseover and click events work.

[tomchadwin]

My gut feeling is that there should be a latlng.

[chriszrc]

I fully agree, but I read through all the source code, and for the life of me, could not figure out how to add that. As I said, I definitely don't know if this is the right answer, but also wasn't getting any traction in the issues section, so I tried to put forth some good faith effort here-

[tomchadwin]

Absolutely - I have the same position with a different issue/PR. I just meant that this is probably fine for your fork, but that we need LatLng fixed here. Not trying to belittle your work one bit.

[tomchadwin]

Dredging this back up again because of requests in #148 that this be merged, here's an attempt at understanding this better:

• yes, point symbolizers inherit from markers/circle markers
• non-LVG markers have latlng
• LVG features have no intrinsic latlng, but only coordinates local to the tile
• other unmerged PRs have class methods to convert tile coordinates to latlng (see eg

  Leaflet.VectorGrid/src/Leaflet.VectorGrid.js

  Line 209 in 449a29c

  vtGeometryToLatLng: function(geometry, vtLayer, tileCoords) {

  , taken from Add a hook to be notified when point features are added #68)

If this summary is correct, we need the conversion method, and we need to reimplement CircleMarker.getLatLng() to use that new method.

Is that right?

[perliedman]

Hi and really sorry for taking forever to review this. As you might have guessed, no one is really maintaining Leaflet Vector Grid at the moment, and unfortunately at least for me I probably will not do it in any foreseeable future either. If you feel up to it, or know someone who would, feel free to notify me or some other maintainer and lets see what we can do.

Anyway, I think this looks like an ok fix, so I am fine with merging this except for some intrusive changes in this PR, unrelated to the actual fix. If you can find the time to adress that, I will merge it!

[chriszrc]

@perliedman ok, I'll make those changes-

[bagage]

Hello @chriszrc, any news on this? Thanks!

[chriszrc]

@perliedman Ok, I think I backed out the changes to dist and whitespace, let me know if there's anything else I can do to get this change merged-

[perliedman]

@chriszrc nice! It still looks like the dist/ files are part of the PR, though - they all show up under "Files changed". Also, looks like some unrelated changes to package-lock.json snuck in there, just get rid of that changes as well, and I think we are good to go!

[chriszrc]

Ok, I'm looking at this page now:

https://github.com/Leaflet/Leaflet.VectorGrid/pull/157/files

And it seems like maybe I got everything this time-

[perliedman]

Nice! Thanks for all the fixes, great!

[bagage]

Any chance to publish a new version on npm with this fix? Thanks!

[m311ow]

Hi, i came across the mentioned bug recently, and tried to fixed it by replacing leaflet.vectorgrid in package.json with @chriszrc fork, then i tried to run yarn install inside node_modules/leaflet.vectorgrid since i noticed that dist folder is missing, however i was unable to make it work because of errors in runas build (runas is one of dependencies)....Does anyone have similiar experience or can at least point me in right direction plz ? Im on linux btw

some lines from error message that could be of some use :

• .../node_modules/leaflet.vectorgrid/node_modules/runas: Command failed.
• runas.target.mk:103: recipe for target 'Release/obj.target/runas/src/main.o' failed