Updated: Comment #18

Problem/Motivation

Geofield 7.x-2.x has significant API changes and improvements. Leaflet widget should also support 7.x-2.x. Initial work is done in the geofield-2.x branch of Leaflet Widget module, and all patches are based off of that branch.

Proposed resolution

Support geofield 2.x in a new branch of Leaflet Widget (7.x-2.x proposed), currently geofield-2.x.

Patch Instructions

Remaining tasks

  • More testing on recent versions of leaflet library to identify follow-up issues or critical fixes to the current solution.
  • Patch needs review.

User interface changes

API changes

Original report by masher

Set-up fine in the content-type, however going to node/add gave the following error:

Fatal error: Call to undefined function geofield_get_base_element() in mymsite/sites/all/modules/leaflet_widget/leaflet_widget.module on line 92

Using:

• leaflet 7.x-1.0-beta1
• libraries 7.x-1.0
• Geofield 7.x-2.0-alpha1
• geoPHP 7.x-1.7

I've tried it on 2 separate installation and received the same error

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tnightingale’s picture

Title: Error on node/add » Support Geofield 2.x
Category: bug » task
Priority: Normal » Major

Thanks for the detailed report. Currently the widget only supports Geofield 1.x.
I've updated the README & project description.

I will take a look at Geofield 2.x soon but if you get there before me, don't hesitate to post a patch! :)

Leaving this issue open and repurposing to track Geofield 2.x support.

tnightingale’s picture

Work has begun in the 'geofield-2.x' branch.

nedjo’s picture

The better way forward here I think would be to refactor in Geofield. I opened #1907914: Refactor OpenLayers widget to pull out generic methods.

I'm thinking specifically of the code I added to the geofield widget in #1627940: Add optional geocoding to map input widget. It's complex and ideally we wouldn't have to fork and repeat it in another module.

XenonofArcticus’s picture

Anything I can do to push the progress on a Geofield-2.x Leaflet widget? I was forced to update to Geofield 2 for another module and now I can't use Leaflet Widget, which is a real bummer.

steinmb’s picture

Geofield 2 is useful but perhaps we should try and release a 1.0 stable first? Look at the issue q and it's not clear to me what's holding back a 1.0. Anyone know?

jjchinquist’s picture

GeoField 2 really has some nice features and it seems like the stable release is due soon. To the maintainers, can you make a list of items that would need to be completed to support the 2.x branch? Then we could better help too.

mradcliffe’s picture

Version: 7.x-1.0-beta1 » 7.x-1.0-beta2
Status: Active » Needs review
FileSize
1.73 KB

Here's an initial patch that can be applied to the geofield-2.x branch.

I found that the issue was that leaflet_widget is decoding json itself and then also trying to get GeoPHP's GeoJSON adapter to decode it again. I removed the redundant json_decode and let GeoPHP handle decoding. It reduces Collections into the most common geometry (i.e. MultiPoint for a collection of Points).

The other change was to use WKT instead of WKB when loading the values because leaflet_widget is saving as WKT.

osopolar’s picture

What about implementing client side geocoding?
See:
#2078255: Add optional client side geocoding to map input widget
and
#1949954: Client-side geocoding for proximity searches, to avoid Google's new 2500 per day limit.

To point out: patch in #7 applies against the geofield-2.x branch not current 7.x-1.0-beta2.

Patch works, but still needs work to include Address Field.

vlooivlerke’s picture

Hi I get this error when editing a node with the widget

Fatal error: Call to undefined function geofield_get_base_element() in \sites\all\modules\contrib\leaflet_widget\leaflet_widget.module on line 92

I make use of address module and are trying to have the map geocode from the address fields. But also open up on the HTML 5 location to make maker placement easier.

Commenting out line 92 makes the widget load,but does not work properly.

boabjohn’s picture

G'Day @mradcliffe, thanks heaps for the start on the patch. We're not using addressfield and just need leaflet_wiget to work with 2.x.
Applying the patch appears to fail though:
~/bnhcrc/sites/all/modules/leaflet_widget (master)
$ patch -p1 < 0001-Issue-1834446-by-mradcliffe-Use-GeoPHP-native-GeoJSO.patch
patching file `leaflet_widget.module'
Hunk #1 FAILED at 163.
Hunk #2 FAILED at 257.
2 out of 2 hunks FAILED -- saving rejects to leaflet_widget.module.rej

We're patching against: 7.x-1.0-beta2
Any clues much appreciated!

ar-jan’s picture

From #8:

patch in #7 applies against the geofield-2.x branch not current 7.x-1.0-beta2.

You can get that branch from git, or get it from the repository viewer.

boabjohn’s picture

Hey there @ar-jan, thanks for the attempted help, but still confused...
You say above that the #7 @mradcliffe patch

applies against the geofield-2.x branch not current 7.x-1.0-beta2.

The patch is clearly aimed at the leaflet_widget module, right? I should not apply this patch "against" geofield (correct?)
So, following your repository link (thanks for that!) we find as expected the dev version of the leaflet_widget module, and I still get the same hunk fails as noted in #10.
I have twice deleted my local copy of leaflet_widget.module, re-dowloaded the dev version, and re-attempted the patch. Always fails.

ar-jan’s picture

I haven't tried the patch, but others say it applies. What I linked to was not the regular 7.x-1.x branch, but a branch of leaflet_widget called geofield-2.x. So you should make sure you're getting that one (use the 'snapshot' link at the top).

boabjohn’s picture

Hi guys,
Sorry for the slow reply here... Christmas/summer holidays here in Oz.
Ok, so I chased the link above at #11 (thanks @ar-jan) and can see the module code here:
http://drupalcode.org/project/leaflet_widget.git/blob_plain/refs/heads/g...

I grabbed this code, pasted it over my local version and ran @mrradcliffe's 0001-xx patch against it. It applied fine, no compliants.
Now I uploaded that .module file to the live server, drush updb, and cc all...
When I go to admin/modules however, there is a warning that the module is incompatible with geofield 2.1 and will not allow the module to be enabled.
Drush also hits the compatibility check.
So I opened up the leaflet_widget.info file and commented out the dependency for geofield 1.x:

...
core = 7.x
;dependencies[] = geofield (1.x)
dependencies[] = libraries (2.x)
...

This seems to have sorted out the patching process.
However, we are still not seeing any maps either in the entity edit form or in the display (because no spatial data has been entered yet, since there is no map to drop a pin on).
Since we're persisting with the use of GeoField 2.x and this patched version of the laflet widget, I guess the fact that we have no map display stays in this issue queue, or should I start a new issue?

mradcliffe’s picture

Here's exactly what I'm using. I am able to see the map and the widgets.

SocialNicheGuru’s picture

this patch no longer applies cleanly.

mradcliffe’s picture

Incorrect. The patch applies cleanly to the geofield-2.x branch. Just tested this with patch from comment #7. Please follow the comment in #11.

mradcliffe’s picture

Issue summary: View changes

Updated issue summary.

SocialNicheGuru’s picture

i'll grab it from git and not drupal.org/project/geofield page.

Edit:
Ok you meant a branch of leaflet_widget called geofield.. LOL now I understand.

vlooivlerke’s picture

I got this working.

I can see a map and widgets. I battle to place a marker or use any of the other shapes. The select and delete functions work fine.

There is no HTML5 geocodeing on the Lat/Long fields only a static map centre. Can HTML5 geocoding be implemented?
Also there is no option to geocode from another field like Address field.

Is there a new version of leaflet library I need?

Thanks

nedjo’s picture

Also there is no option to geocode from another field like Address field.

That's because geocoding support was added to Geofield's widget after the code was forked into Leaflet Widget for Geofield.

Like I suggested in #3, I recommend postponing this issue and focusing instead on #1907914: Refactor OpenLayers widget to pull out generic methods. Some refactoring in Geofield likely would make writing - and, more importantly, maintaining - a fully featured Leaflet widget a fairly trivial task.

boabjohn’s picture

Hi Nedjo,
Your thoughts on #1907914: Refactor OpenLayers widget to pull out generic methods make perfect sense.

Unfortunately it seems as though progress in this area has stalled and the issue referenced is a loop between a closed issue and one marked as a duplicate...and no one has had anything to say in either thread for over a year now (at least that's how I read it...maybe I missed something).

Could you (or others? @brandonian? @phayes?) weigh in with some current thoughts?

It feels like Drupal mapping via openlayers is getting more fragmented and less supported as time goes on...and this is just at the time when geospatial content is more in demand than ever before.

Are there any strategic places where a bit of funding might unblock a whole raft of issues?

Sorry to go off on a strategic rant in the middle of an issue thread...I'm just not sure where "the mapping conversation" is happening these days?

Kind regards,
JB

pmackay’s picture

It would be very helpful if a more detailed outline of the refactoring steps needed could be added here or in #1907914: Refactor OpenLayers widget to pull out generic methods.

boabjohn’s picture

Status: Needs review » Needs work

Doesn't seem like the current patch in this issue actually delivers enough support to be marked for review.
Suggest setting it back to Needs Work and again ask around for people who might be interestted in getting involved with the development.
We are not a code house, but can certainly help attract a modest amount of funding if that's the issue.
At the moment, this area is a real drag for projects that are trying to stay on the edge of geospatial data.
Keen to get some action sparked up!
Kind regards,
JB

vlooivlerke’s picture

I would love to see some action here, but everyone is waiting for open layers. I think the code should be split from open layers and made an official leaflet module. The location module offers this feature in its D5 branch already.

Is there any other modules except open layers and location module that offers a map to place your marker in the node edit form.
Geocoding sucks in my country and I need to be able to manually place markers.

I use address field and geofield and can get amazing things done, It just does not make sense to install open layers to get one small feature from it.

I actually got this module to place a marker, it is the polygons, lines, and the fallback to geocode from address field that is not working.

mradcliffe’s picture

Doesn't seem like the current patch in this issue actually delivers enough support to be marked for review.
Suggest setting it back to Needs Work and again ask around for people who might be interestted in getting involved with the development.

Anything in particular not working?

vlooivlerke’s picture

I got this working.

I can see a map and widgets. I battle to place a marker or use any of the other shapes. The select and delete functions work fine.

There is no HTML5 geocodeing on the Lat/Long fields only a static map centre. Can HTML5 geocoding be implemented?
Also there is no option to geocode from another field like Address field.

mradcliffe’s picture

Aren't the following feature requests rather than related to porting current functionality to work with Geofield 2.x?

  • HTML 5 geocoding
  • Geocode fallback from another field

I battle to place a marker or use any of the other shapes

Is there anything specific that can be identified and compared to current version of leaflet widget?

vlooivlerke’s picture

Ok, I started from scratch and now have even worse results than before.

I get the module from git.
I apply the patch cleanly
I change an existing geo-field from lat/long to leaflet.widget
I set the map center and zoom, and select Google road map (leaflet more maps)

On the node edit form everything is now broken
1. CKeditor does not load
2. All collapsible fieldsets are now broken
3. The leaflet.widget is blank and show nothing

I use the latest leaflet and geofield

I have seen the map before with an older version of leaflet, but there has been many changes since in that module.

tormi’s picture

IMHO it would be a much clearer if there is no middleman between Leaflet.Draw and Drupal module which incorporates Leaflet.Draw's editing functionality.

Tormi

das-peter’s picture

Fix and extended patch:

  • Adusted path to leaflet js lib: from "$leaflet/dist/" to $leaflet
  • Readme: Fixed Link to Leaflet.draw
  • Readme: Added installation instructions for Leaflet.draw
  • Requirements: Added requirements check in leaflet_widget.install
das-peter’s picture

Next patch.
I guess this deserves a review now.
The new patch introduces a geocoder integration.
If the module is present you can activate the integration and define the handler.
In the widget you'll seen the a new field to geo-code points.

das-peter’s picture

Mini update. If geo-coding was successful the map is centred around the newly added marker.

steinmb’s picture

@das-peter can take your patch for a spin. Any special things that does not work or needs more manual testing?

das-peter’s picture

@steinmb A review would be appreciated very much! I basically just did smoketesting while developing. So edge cases e.g. like ajax failures in the geocoder integration could have interesting results, even though it tried to be defensive.
However, the most important part might is the change from "$leaflet/dist/" to $leaflet - I don't know why there was a "/dist" suffix to the library path. As far as I could figure out in a rough evaluation there wasn't the need for the "dist" prefix in earlier leaflet versions either. So I'm a bit lost there.

May I propose the creation of an official 7.x-2.x dev branch? And to commit the changes as soon as possible?
The branch geofield-2.x is a bit uncommon and since it seems broken right now committing the patch won't hurt to much anyway :P

mradcliffe’s picture

It doesn't work with the latest Leaflet.js library, but it does work for older versions. It would be interesting to see at what version it broke or if the newer Leaflet.draw works with older Leaflet.js versions (to 0.5?).

core44’s picture

Using geofield-2.x branch of Leaflet Widget with leaflet_widget-support-geofield-2.x-1834446-33.patch and Leaflet 0.4.5 (included in Leaflet.widget library). Saving the node without a value in the field throws an error when you attempt to edit the node again. Which breaks the map.

TypeError: obj is undefined.
if (typeof obj[0] === 'number' || obj instanceof L.LatLng) {

Also, can still only add markers to the map, no polylines or polygons.

Tested leaflet-0.5.1 and it works as above. The next version leaflet-0.6 causes an error when adding a marker to the map:

TypeError: el is undefined.
el._leaflet_pos = point;

Hope this helps.

steinmb’s picture

Status: Needs review » Needs work
das-peter’s picture

Status: Needs work » Needs review

Could please someone clarify what the minimal requirements regarding leaflet / leaflet plugin versions are?
I don't think we need to support all the versions back in history, but need to pick sane defaults.
That said, I think following combination is sound:

Could we make that the minimal requirements?
Btw. the current stable version of the Leaflet module even defines the Leaflet Version 0.7.1 as download url.

I personally run:

With those versions I don't have any issues using markers, polygons or lines.

Regarding:

TypeError: el is undefined.
el._leaflet_pos = point;

I've seen the same error, there are even more/other errors. However, I don't think this is related to the module integration with geofield 2.x but to Leaflet.draw (or maybe Leaflet.widget) and I don't think an issue there should stop this patch.

mradcliffe’s picture

I think minimum version would be whatever the leaflet module supports as its the dependency of leaflet_widget.

steinmb’s picture

I think we should not care that much about supporting older generation of the library. Version 0.7.x is supposed to contain a lot of bugfixes. Version 0.8 is a rewrite that introduce a lot of changes. Let's make 0.7.x work. Found more info in http://leafletjs.com/2013/11/18/leaflet-0-7-released-plans-for-future.html

vlooivlerke’s picture

I think we should start with just placing the marker as the first working feature, leaving draw and polygons for later releases.

This could speed things on 0.7.x leaflet as the library.

das-peter’s picture

I think we should start with just placing the marker as the first working feature, leaving draw and polygons for later releases.

Why? For me those features seem to work. There are might some JS issues depending on the version combination of the JS libraries but with my setup I can draw shapes, lines and set markers.

core44’s picture

Just a quick update, will provide more info soon. I've managed to get everything working well, using 0.7.1 of leaflet (I wanted match the version dist with the leaflet module). Geofield 7.x 2.1 and Leaflet.draw 0.2.3. I am able to add and edit all shapes and have yet to run into any bugs (but sure I will with more testing).

I patched the geofield branch of Leaflet widget with #33 and then manually merged the module with https://github.com/acouch/leaflet_draw_widget to enable use of leaflet.draw 0.2.3. (mainly just enabling the draw.js instead of widget.js). I wasn't able to get things working without upgrading to v2 of leaflet.draw.

I'll create a sandbox when I get a moment so others can test. Aware that updating to leaflet draw 0.2.x is a separate issue https://www.drupal.org/node/1926942 but it was something I needed to include in my project so looked at both issues together.

das-peter’s picture

I've just come across another issue.
If the JS compression is enabled the handling to determine L.Icon.Default.imagePath doesn't work anymore. I thus suggest to explicitly set a value using the library hook / settings - see the interdiff for my current approach.

  • das-peter committed e3ecbd4 on 7.x-2.x
    Issue #1834446 by das-peter, mradcliffe | masher: Support Geofield 2.x.
    
das-peter’s picture

Status: Needs review » Fixed

I just had contact with @tnightingale and requested a review and / or offered co-maintainership.
As visible in #46 I've got maintainer permission, thanks @tnightingale! :)

I was bold enough to create a new branch and commit the code. Let's push forward from here.
A development release should be available soon too.
If there still are concerns about the minimal version I'd like to ask you to create a dedicated issue for that.
Also testing and feedback still are highly appreciated as I'm sure I didn't cover all cases ;) Please open dedicated issues for them, I'll take care of them asap.

I hope with an usable branch (no need to apply patches) in the wild we get more feedback.

steinmb’s picture

A BIG thanks to @tnightingale and @das-peter for moving this issue along :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.