Closed (fixed)
Project:
Leaflet
Version:
2.0.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
4 Jun 2017 at 17:25 UTC
Updated:
27 Nov 2020 at 13:27 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
grimreaperHere is the patch.
Thanks for the review.
Comment #3
grimreaperAlso seing a missing "view_mode" in the schema of the leaflet_map plugin.
Comment #4
super_romeo commentedRerolled.
Comment #5
super_romeo commentedWith my patch I have following errors:
And same error for other fields:
Now we need hook_update_N to convert existing field values to
booltype.Comment #6
stephen-cox commentedPatch #5 didn't work for me. There where missing definitions for gesture_handling, icon.html and geocoder. Attaching patch for these.
Comment #7
itamair commentedOn this I am worrying about the #5 comment on the need to hook_update_N to convert existing field values to bool type,
that eventually need to be packed with the patch itself.
Cannot juts we keep managing those settings as integer ones (instead to boolean)? and adjust the #6 patch accordingly ... ?)
I also need a RTBC to validate the last patch ...
Comment #8
finn lewisEvent with the patch from #6, leaflet_views schema still has problems.
Steps to reproduce:
1. Fresh install of Drupal 8.9.5, Leaflet 2.0.9
2. Add a geofield to a content type, create a leaflet_map view called 'test_leaflet'
3. Install config_inspector, go to /admin/reports/config-inspector and tick 'Only show errors'.
4. Click on 'List'
5. See the following on /admin/reports/config-inspector/views.view.test_leaflet/list
I've tried a bunch of things to suppress the errors as they also block our automated tests from passing.
It looks like some of these items are defined in https://git.drupalcode.org/project/leaflet/-/blob/2.0.x/modules/leaflet_..., so why the missing schema?
Any ideas most welcome!
Comment #9
finn lewisLooking further at the config inspector, and comparing with another views style schema that does not produce errors (views_accordion) I see that the 'display.default.display_options.style.options' config looks to be defaulting to
label: Default style
type: views.style.*
See the following excerpt from config_inspector:
whereas the views_accordion config for display.default.display_options.style.options defines the expected style plugin:
display.default.display_options.style.options Views Accordion views.style.views_accordion <array>So, is the leaflet_views schema somehow not defining the style configuration correctly?
The config schema appears to be falling back to 'views.style.*:' from
https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/modules/view...
Comment #10
finn lewisI stared at this for so long, but it only started making sense when @progga pointed out the miss-spelling of 'leafet_map' rather than 'leaflet_map' on https://git.drupalcode.org/project/leaflet/-/blob/2.0.x/modules/leaflet_...
I've updated the config file in the patch attached also updating some of the default settings in /src/LeafletSettingsElementsTrait.php to ensure they are saved as FALSE rather than 0, which seems to be interpreted as an integer by the config_inspector. There may be more to this than I've seen, but the patch attached hopefully fixes our immediate failing tests.
Comment #11
finn lewisComment #12
itamair commented@thanks Finn. I am not a Master on the schemas ... so I am relying on this contributions.
Are the 2 patches attached on the #10 identical?
Is any of them ok to be committed into dev???
Comment #13
finn lewisAfter testing these patches, it is clear that the patch from #6 and the patch from #10 don't apply together, as the patch from #6 makes some changes to the leaflet_views config/schema file too.
Here's a patch that combines the changes in #6 and #10 and allows my local config tests to pass.
@itamair, let's get someone else to review this patch perhaps before committing to dev. I too am no expert on the config schemas, but I'm learning!
Comment #14
finn lewisSeeing an error when applying the patch from #13
Trying new patch to avoid that error.
Also changing version to 2.0.x-dev.
Comment #15
itamair commentedOk thanks @finn. Let's keep this still in "Needs review" for a while.
I will try to review it if/when/as soon as I find some time ...
Comment #17
itamair commentedComment #19
jbfelix commentedHello,
For the views style plugins, this would be
to keep decimals and a perfect centering.
Regards