Problem/Motivation

The 8.x-1.x branch is broken with Drupal 8.0.x due to API changes.

Proposed resolution

Write patch to fix leaflet module to bring it up-to-date so that it does not have fatal errors. This issue should not be about making leaflet "perfect", but to bring it back to a working state so that follow-up issues can be created.

Remaining tasks

  • Write patch (DONE)
  • Review patch
  • Manually test patch based on basic functionality. Apply current patch to leaflet 8.x-1.x branch.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bforchhammer’s picture

Status: Active » Needs review
FileSize
53.35 KB

Attached patch makes leaflet work with geofield in Drupal 8.0.0-beta2. It includes the following changes:

  • Fixed fatal errors for D8, removed usage of deprecated functions.
  • Moved plugins into new PSR-4 structure + fixed namespace for auto-discovery of plugins.
  • Fixed problems with library definition/detection and JS rendering; implemented `libraries.yml` for D8.
  • Switched to using leafletjs CDN version by default; this can easily be changed by implementing `hook_library_info_alter()`. Adjusted `hook_requirements` implementation to support CDN as well as local versions.
  • Fixed and cleaned up views plugin.

Because I have moved the plugins into different files, it might be easier review the individual commits on my github fork instead of the patch.

I am also thinking about converting the map definitions into config entities, i.e. get rid of hook_leaflet_map_info and switch to yaml-based approach... any thoughts/input? :)

bforchhammer’s picture

FileSize
53.35 KB

New patch with a few more more fixes, mostly minor.

Letharion’s picture

I came to the issue queue because I couldn't find the leaflet style option in Views. This patch solves that.

One thing, that I don't know if it should be addressed in this issue, is that when setting leaflet style options, there no scrollbar, so without using [TAB] to move between fields, I can't access the entire settings form.
Nvm, scrollbar showed up now...

Alex Bukach’s picture

FileSize
53.36 KB

New patch with a few more minor fixes.

bforchhammer’s picture

I have been working on a few more changes as well (improving views integration, mostly). I'll try to merge your changes into my branch and provide a new patch soon. :)

bforchhammer’s picture

FileSize
100.5 KB

Here's an updated patch with a bunch more cleanups. I refactored the javascript widget a fair bit, to make it easier to extend via 3rd party plugins, and also added a couple of hooks

As mentioned before, it is probably easier to either look at the new code directly, or review the github changelog instead of the patch, because some files have been moved and renamed.

I have also created a bunch of modules which integrate themselves with leaflet and can be configured for individual maps via the "plugins" key in the map definition. The respective repository readme files provide more details:

@Alex Bukach: I integrated your changes here and here.

Alex Bukach’s picture

@bforchhammer, will you release that modules at d.o?

Also, is https://github.com/bforchhammer/leaflet_markercluster related to https://www.drupal.org/project/leaflet_markercluster?

bforchhammer’s picture

@bforchhammer, will you release that modules at d.o?

That's the plan. :) I'd like to wait for this patch to go in first though, so we don't get modules depending on different API versions of the leaflet module on d.o.

Also, is https://github.com/bforchhammer/leaflet_markercluster related to https://www.drupal.org/project/leaflet_markercluster?

No, mine was a fresh start. Functionality-wise both are pretty much identical; mine comes with additional views support, which allows to select "markercluster" as a views-style plugin for grouping leaflet markers. I'd like to see my changes be added into the existing d.o. project eventually, but I'd like to wait for this patch to go in first before I start work on a respective patch.

Alex Bukach’s picture

The patch should be updated again to fix https://www.drupal.org/developing/api/8/assets incompatibility (see leaflet_render_map()).

vdsh’s picture

I corrected the following:
- there was a javascript error preventing the display of the nodes
- you could not set a custom logo (nor customize its size)

And I also added the possibility to define the zoom level that you want to use at the administration level (with the minimum and maximum zoom also now definable in the administration)

As I am very new to this module, can someone please review the patch?

vdsh’s picture

Ok, now I am a bit confused regarding Views. Long story short: I can't make any view display my nodes.

There is LeafletMap.php and Leaflet.php that are both Views Format. LeafletMap.php is tagged deprecated, so why don't we remove it?

Also, with Leaflet.php, you cannot choose which field is the geofield you want to use with your view, and my map is rendering but is empty. Am I missing something here?

Dennis Cohn’s picture

hmmm, same here!
On a nodepage it works, I see a map.
But when I create a view to show all nodes on the map... I get this error:
Notice: Undefined index: #attached in Drupal\leaflet_views\Plugin\views\style\Leaflet->render() (line 117 of modules/leaflet/leaflet_views/src/Plugin/views/style/Leaflet.php).

I've downloaded your updated leaflet module from https://github.com/bforchhammer/leaflet

jrgriffiniii’s picture

Hello Everyone,

We're looking to explore the possibility of integrating Leaflet.js into Drupal 7.x sites (which, potentially, could be migrated into Drupal 8). Has there been any progress in relation to resolving this issue (and, more generally, working with recently-released betas)?

Thank you.

Sincerely,
James

kpv’s picture

Attached minor fix for the following error (after core update to 8.0.0-rc1).

Fatal error: Declaration of Drupal\\leaflet\\Plugin\\Field\\FieldFormatter\\LeafletDefaultFormatter::viewElements() must be compatible with Drupal\\Core\\Field\\FormatterInterface::viewElements(Drupal\\Core\\Field\\FieldItemListInterface $items, $langcode) in modules/contrib/leaflet/src/Plugin/Field/FieldFormatter/LeafletDefaultFormatter.php

balagan’s picture

Applying patch at comment 10 gives warning about whitespace error (trailing whitespace).

mradcliffe’s picture

I re-rolled #10 to remove the whitespace errors and the deletion of leaflet.api.php, and attached a patch with all fixes per patch standards.

  • interdiff-6-10.txt: displays changes from #6 to my re-roll of #10.
  • interdiff-10-14.txt: displays changes from my re-roll of #10 and #14.
  • interdiff-6-16.txt: displays changes from #6 to the latest patch that I uploaded.
samuel.mortenson’s picture

Has anyone had luck with leaflet_views? I haven't been able to create a map view (yet).

samuel.mortenson’s picture

FileSize
101.59 KB
1.17 KB

Minor bugfixes on current patch.

pallavi_sugandhi’s picture

I am facing the following fatal error:

"Fatal error: Declaration of Drupal\leaflet\Plugin\Field\FieldFormatter\LeafletDefaultFormatter::viewElements() must be compatible with Drupal\Core\Field\FormatterInterface::viewElements(Drupal\Core\Field\FieldItemListInterface $items, $langcode) in D:\xampp\htdocs\drupal-8\docroot\modules\leaflet\src\Plugin\Field\FieldFormatter\LeafletDefaultFormatter.php on line 244"

Even if I clone the module from github: https://github.com/bforchhammer/leaflet.

mradcliffe’s picture

Issue summary: View changes

Updated issue summary to prevent confusion in #19

Alex Bukach’s picture

FileSize
101.59 KB
691 bytes

I have noticed that entity_type property of the views plugin is always empty, since views base table is a data table for an entity, while it's compared with entities base tables.

jian he’s picture

FileSize
103.45 KB
1.86 KB

Add the schema definition file to avoid the annoying "missing schema" issue.

pvhee’s picture

Thanks everyone for the great work in making Leaflet work with the latest Drupal 8 core. I've tested this and will go ahead and commit #22 to the 8.x branch. Any new work can then can go in separate issues from here onwards.

  • pvhee committed de6d2ad on 8.x-1.x authored by bforchhammer
    Issue #2366069 by bforchhammer, Alex Bukach, mradcliffe, samuel....
pvhee’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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