Geolocation filtering

[petschki]

refactored from collective.venue ... open discussion

[thet]

In general: +1 for that. Didn't have the time to review yet, though.
I wasn't sure about the package structure of collective/collective.venue#13 anyways - I more or less added to collective.venue because this is where the map integration actually happens.
But I like your approach better.

[thet]

@petschki thanks for this PR!
I have some remarks, especially regarding restructuring generic features to collective.geolocationbehavior or plone.formwidget.geolocation (where leaflet is actually integrated).
if done so, the map-related configuration options from c.venue might also be better placed c.geolocationbehavior or so.
What's your opinion on this?

[thet]

+1 for the configuration, -1 for a per-tile config. This should better be defined portal-wide, like it's done in c.venue's controlpanel.

1. to have better consistency,
2. to not having to define an separate API key for each tile.

The API key configuration feature is on my TODO list for the c.venue controlpanel.
But maybe the controlpanel should go - similar to my remarks before - to c.geolocationbehavior or p.f.geolocation.

[petschki]

@thet thank you for the detailed review. Funny because yesterday I also thought that the map functionality isn‘t really the correct place here.

So I‘m +1 for nearly all your comments.

Map config within the tile was my idea because this makes it more flexible for the Editor to use different Maps in different contexts. Couldn‘t the API key be defined portal wide and the Maps Layout be configured locally? I like the flexibility in general and let the editor decide about consistency.

So I would suggest to bring the Maps logic to plone.formwidget.geolocation because other forms could then also benefit from this. If you agree I‘ll move the stuff over there and make a PR.

The index makes sense on the behavior. I‘d move it over there.

[petschki]

@thet @agitator map filtering is in alpha state but so far functional for testing. I've added a portlet too but this is not really usable in a column (I think) ... but for completeness sake. I decided to do the narrowing on move/zoom as an opt-in checkbox but maybe that's too conservative 😉 ... don't forget to checkout master from plone.patternslib in order to get the leaflet events on the pattern node ...

[thet]

@petschki Thanks for your work on this!
For me, to use this refactored features I have to get it feature complete to the original collective.venue approach. Basically what is missing is filtering for events with the ILocatation behavior from collective.venue.
I'm working on this but as long I haven't finished it I want to keep this PR from merging. However, should be done within a week.

[petschki]

@thet no problem. I already use this branch in a new project of us and it works quite well. But I also need some more features like different colors of the map pins depending on some state of the article, but that belongs to plone.patternslib and pat-leaflet.

Not to forget, that this PR needs collective/plone.formwidget.geolocation#12 and collective/collective.geolocationbehavior#6 too ... I suggest releasing everything together and provide a versions.cfg here for the geolocation dependencies (like mosaic does for it's dependencies) which you can extend from in buildout.cfg changed this to extra_requires in setup.py

[thet]

@petschki you might be interestend in: Patternslib/pat-leaflet@4828026 eventually also Patternslib/pat-leaflet@d342a65
looking at it, i should have named color rather pin_color.
it's already merged into plone.patternslib

[thet]

@petschki i'm currently working on all the collectionfilter/maps branches.