Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#22 | interdiff.txt | 1.86 KB | jian he |
#22 | 2366069-d8_version-22.patch | 103.45 KB | jian he |
#6 | d8-version-fixes-2366069-6.patch | 100.5 KB | bforchhammer |
Comments
Comment #1
bforchhammer CreditAttribution: bforchhammer commentedAttached patch makes leaflet work with geofield in Drupal 8.0.0-beta2. It includes the following changes:
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? :)Comment #2
bforchhammer CreditAttribution: bforchhammer commentedNew patch with a few more more fixes, mostly minor.
Comment #3
Letharion CreditAttribution: Letharion commentedI 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...
Comment #4
Alex Bukach CreditAttribution: Alex Bukach commentedNew patch with a few more minor fixes.
Comment #5
bforchhammer CreditAttribution: bforchhammer commentedI 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. :)
Comment #6
bforchhammer CreditAttribution: bforchhammer commentedHere'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.
Comment #7
Alex Bukach CreditAttribution: Alex Bukach commented@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?
Comment #8
bforchhammer CreditAttribution: bforchhammer commentedThat'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.
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.
Comment #9
Alex Bukach CreditAttribution: Alex Bukach at This Little Duck commentedThe patch should be updated again to fix https://www.drupal.org/developing/api/8/assets incompatibility (see
leaflet_render_map()
).Comment #10
vdsh CreditAttribution: vdsh commentedI 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?
Comment #11
vdsh CreditAttribution: vdsh commentedOk, 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?
Comment #12
Dennis Cohn CreditAttribution: Dennis Cohn at ezCompany commentedhmmm, 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
Comment #13
jrgriffiniii CreditAttribution: jrgriffiniii commentedHello 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
Comment #14
kpv CreditAttribution: kpv commentedAttached 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
Comment #15
balagan CreditAttribution: balagan as a volunteer commentedApplying patch at comment 10 gives warning about whitespace error (trailing whitespace).
Comment #16
mradcliffeI 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.
Comment #17
samuel.mortensonHas anyone had luck with leaflet_views? I haven't been able to create a map view (yet).
Comment #18
samuel.mortensonMinor bugfixes on current patch.
Comment #19
pallavi_sugandhi CreditAttribution: pallavi_sugandhi as a volunteer commentedI 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.
Comment #20
mradcliffeUpdated issue summary to prevent confusion in #19
Comment #21
Alex Bukach CreditAttribution: Alex Bukach at This Little Duck commentedI 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.Comment #22
jian he CreditAttribution: jian he commentedAdd the schema definition file to avoid the annoying "missing schema" issue.
Comment #23
pvhee CreditAttribution: pvhee at Marzee commentedThanks 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.
Comment #25
pvhee CreditAttribution: pvhee at Marzee commented