Account created on 19 February 2011, about 14 years ago
#

Merge Requests

Recent comments

🇮🇹Italy itamair

ok ... also interaction on the Geofield Map View page was improved. Now it works in an acceptable way.

🇮🇹Italy itamair

Hi @òrahulkhandelwal1990 thanks for your head-up.
Indeed the Live Demo link was still pointing to a previous address.
I corrected it in this post description into this update and correct one:

https://www.geodemocracy.com/drupal_geofield_stack_demo/web/geoplaces-gm...

Also the weblink to the geofield_map_interaction.js file mentioned in the Left sidebar block as initial guidance & inspiration of this interactivity was corrected.

Additional Note: On this Geofield Map View page the interaction is not working so great (the Google Map Infowindow sometimes doesn't open, etc.) as it is in the corresponding Leaflet View interaction example: https://www.geodemocracy.com/drupal_geofield_stack_demo/web/geoplaces-ma...
Not sure why, at the moment ... but I will find sometime to better inspect and improve it.

🇮🇹Italy itamair

Ok ...
so, do you really want to keep this open, with "needs work"?
I don't think this can cope in code every possible user use cases (and custom Leaflet Map Info definitions),
and that can end up into the Leaflet module code base.

Could we rather close as "Closed (works as designed)"?

🇮🇹Italy itamair

Ok ... I went much better through all this,
and required & enabled the Content Security Policy module on my local instances of the following 2 (Leaflet powered) websites:

https://www.geodemocracy.com/drupal_geofield_stack_demo/web/geoplaces-ma... (Official Leaflet Module Live Demo)
https://www.taranto-viva.com/it

Yes indeed, all the Leaflet maps break 8and not only the Leaflet widgets maps), with a long list of errors/warnings in the inspector console ... etc.

But also the MR !31 Draft doesn't help on this ... and all still looks broken.
It looks that is trying to cope some very specific use case and Leaflet context, that means if the Open Street Map Tile (default in the module) is used ... and for the widgets.

But it looks that also Leaflet Formatters and Leaflet View Styles are breaking ... isn't it?
And e should consider that in most cases users will implement different Leaflet Map background tiles ... (from Open Street).

May be I didn't get the proper issue case (as I am not fully understanding all the CPS options and functionalities, etc)
BUT I don't think/feel the correct use of the CSP should be restored with whitelisting with additional code in the Leaflet module,
but rather with specific whitelisting in the CSP module settings, or eventually in custom modules by users implementing it ...

What could be a general fix in this Leaflet module instead, eventually? (that could solidly cope all the possible Leaflet Map Tiles implementations?)

🇮🇹Italy itamair

I cannot replicate this issue ...
To me, the actual Leaflet module release (10.2.43) can correctly render different Leaflet Popups and Tooltips
according to the translated version of the content and the language selected for the specific page.

For instance you can see this Italian / English implementation:
https://www.taranto-viva.com/en
https://www.taranto-viva.com/it

Please, close the first welcome popup, just zoom in and check both the Popup and Tooltips on some of those icons,
such the "Cathedral of San Cataldo", for instance ...

Suggestion: make sure you set in the View settings:
Language
Rendering Language: Content language selected for page

Screenshots attached ...

🇮🇹Italy itamair

10.000 thousands features is pretty a lot ... and may be too much indeed.
And for sure you should enable the Leaflet Markecluster (sub)module (and its settings), at least trying that.

Views GeoJson module could be a nice option, but for sure won't reduce the number of features that you try to load ...

With that you should try to implement a Decoupled Solution that simply fetches that GeoJson data response.

Leaflet View map is only going to fetch data from a Leaflet Map View style,
or may be you simply can try to replicate this Leaflet View Geojson Demo example here:
https://www.geodemocracy.com/drupal_geofield_stack_demo/web/geojson-map-...
(@see the linked "leaflet_map_geojson.js" file in the right sidebar ... )

but for sure that approach won't easy the load and effort of such a big amount of features.

🇮🇹Italy itamair

hi @joaomachado ...
thanks for posting this.

Your description is a bit confusional.
You mention different content types labelled as Origins, or Destinations, then you also mention the Proximity Module, that is not clear what it is ... and also you ask if you could mix the Leaflet module functionalities with the Google Routes API,

Well. it's hard to be exhaustive here, without spending a lot of time (that personally I don't have now ...).
But I would point you into some helpful directions, possibly.

If you implement a Leaflet View Map then you should eventually be able to add to it the:
Leaflet Routing Machine (https://github.com/perliedman/leaflet-routing-machine) and build your own interactive solution on top of it ...
How?
Well, you should be able to master Leaflet Js library and its plugins and know how to extend the Drupal Leaflet Map View.
Probably this Drupal Leaflet issue should provide you with proper hints: https://www.drupal.org/project/leaflet/issues/3047091 📌 Leaflet Map & Markers external interaction, on events - LIVE DEMO Needs review

Google Routes API? may be also ... but in that case you will send the user out of the Drupal website for processing the navigation operations.

May be also the Geofield Directions module could be inspirational to you: https://www.drupal.org/project/geofield_directions
Also see this dedicated brief video: https://www.youtube.com/watch?v=7HxuVQ8WlNA
As far as I understood it simply implements a Geofield Formatter that Links out to a Google Map that presets that Target Direction from ... well, I guess where you are, or whatever you choose as Google Map starting point.

But may be you could grab its code, and eventually generate a Computed Field that automatically generates a Google Map link with preset both origin and destination taken from a pair of Origin and Destination geofield entities ...

Kind of all that / those ...

🇮🇹Italy itamair

Thanks @bachbach for reporting this ...
could Test and QA the attached patch?
It should restore backward compatibility on this.

🇮🇹Italy itamair

Well ... the server type shouldn't make any difference in my opinion,
BUT (anyway) of course we are also testing all this on Linux based servers.

More specifically it works correctly locally on DDEV docker web image: https://hub.docker.com/r/ddev/ddev-webserver
and it also works correctly here: https://www.geodemocracy.com/drupal_geofield_stack_demo/web/
that is (of course) based on linux servers ...

BUT, rather then going around vague assumptions, why you don't provide more info on your error logs ... ?
what is not working with your "arcgisonline" provider instance?
Are you able to XDebug what is happening on your code workflow?
Are you checking it locally .. or directly on the production server? (not nice practice, in such latter case).

I am pretty confident that it could highly depend on your specific configuration setup. May be something was messed up in your Arcgisonline configuration setup ...
Would highly recommend you to remove all of it and re require it via composer
composer require geocoder-php/arcgis-online-provider
and re-enable it from scratch with your Drupal backend configuration.

The original lower case
id = "arcgisonline"
never changed over the last bunch of years, and no-one reported this same issue of your, for a Geocoder provider that is basically free for use, and definitely much used also with this Drupal Geocoder module ...
No-one else have reported this same issue ...

🇮🇹Italy itamair

Great catch @ivnish ... thanks.
QAed successfully.
Merging and deploying a new 2.1.3 release with this ...

🇮🇹Italy itamair

Adding reference to the parent issue, that I had do autonomously look for ... and find,
and that wasn't approved and merged, because not needed.

🇮🇹Italy itamair

I better inspected this, and nothing looks wrong to me in the actual 8.x-4.x code,
and everything (still) works correctly with the "ArcGisOnline" (id = "arcgisonline") Geocoder provider.

Changing this into "Support Request" (because no proof/evidence of Bug) and closing as everything "works as designed" ...

Please provide better info context next time, and make better internal debug, because everybody time is precious.
and please don't reopen this without clear evidence / proof of your assumptions..

🇮🇹Italy itamair

Thanks @dianacastillo for reporting this
But it looks to me you are rushing and not providing proper context.

You mention that this change was already made for another version, but you don’t provide where this change was made, and for which version.
Can you provide reference to the parent (or related) issue you are speaking about, or just the link to “that” commit?
So this audience could better understand what kind of regression you are referring to, and how to reproduce.

Also, once you create a fork you should also generate a MR (merge request) if you ask for final review and eventual merge of it.

I will be able to better inspect this tomorrow (if you please provide better info and context) and merge it, eventually, if worth to be approved.

🇮🇹Italy itamair

Thanks ... committed into dev, will be part of next Geocoder release.

🇮🇹Italy itamair

Patch #4 committed into dev branch, will be part of next Geocoder release.

🇮🇹Italy itamair

Ok ... got all this.
New leaflet 10.2.43 release (just deployed) is fixing this,
and is also keeping fixed the related https://www.drupal.org/project/leaflet/issues/3377484 🐛 Uncaught Error: Invalid LatLng object: (NaN, NaN) Active one.

Closing this as fixed.

🇮🇹Italy itamair

Hey @wsantell ... thanks for reporting this, and sorry for this regression.

Please, let me properly understand the specific context.

You say ...

Steps to reproduce
View a geofield which uses the Leaflet Map format, with "Field" Map Icon using custom icon size and anchor settings
Observe that the marker is 12px x 12px and offset -6px left and up.

BUT, is this happening only in case of DivIcons ... right?
Can you confirm that we should test and reproduce this specifically with DIVIcon?

Because that commit only changed the Drupal.Leaflet.prototype.create_divicon function ...

🇮🇹Italy itamair

thus ... could you QA and test the last attached patch and eventually mark this as RTBC eventually ... to approve (green flag) its commit into dev?

🇮🇹Italy itamair

ah ok ... better got it.
It is CommerceGuys\Addressing DefaultFormatter adding that "\n"
but it is the Geocoder AddressService here:
https://git.drupalcode.org/project/geocoder/-/blob/8.x-4.x/modules/geoco...
that is converting it into a space ... rather than something more clear separator for the Geocoder provider, such comma ...
right?

Thus what about replacing both the "\n" and the "
" eventually into a comma instead (",")??
You propose ...

Ok. I attache here an updated patch that adds these new replacements (in place of a space).
Could you carefully and extensively test that and eventually confirm this would improve the Geocoder AddressService on a general basis, in your feelings?
I think/assume that a comma (,) separator between different Address parts should be more clear with every Geocoder provider, isn't it?

Let me know your outcomes with this new patch, please.

🇮🇹Italy itamair

thanks for reporting all this @tklawsuc
BUT outcomes of my QA and tests reproducing your use cases say that all this happen on singular cases (Issue 1)
and strangely depending on the Geocoder Provider that you use.
Issue 2 happens only with Nominatim ... and the address line 2 doesn't compromises correct Geocoding with others, such as Photon or Google Maps.

The best approach that I would propose is contained into the attached patch,
that I feel could also benefit the related issue: https://www.drupal.org/project/geocoder/issues/3335292 🐛 Incorrect geocoding when suite number included in address Postponed: needs info

A new "geocoder_address_values_alter" hook (in the "geocoder_address" submodule) that might allow any custom module to change/alter the Address Values array (at your specific use case) before it is stringified by the AddressService: addressArrayToGeoString.

That can also do something like the following, to remove the "address_line2" value:

/**
 * Implements hook_geocoder_address_values_alter().
 */
function [custom_module]_geocoder_address_values_alter(array &$values) {
  $values["address_line2"] = '';
}

Please Qa and test this attached patch.
I am pretty convinced to commit and add that into a new incoming Drupal geocoder 8.x-4.28 release ...

🇮🇹Italy itamair

Sure we should! Thanks @acbramley
Committing into 8.x-4.x dev branch ...

🇮🇹Italy itamair

itamair made their first commit to this issue’s fork.

🇮🇹Italy itamair

thanks @ovquiaf ... Qa and Tested what you say and it makes sense to me.
Committed into dev (with some better ternary operator implementation) is going to be part of the news (incoming) Geocoder release.

🇮🇹Italy itamair

Hey @bavramor what are you referring and looking into?
What you state here looks totally wrong ...

Did you ever inspect the Drupal Leaflet module libraries definitions?
And in particular the leaflet/leaflet library definition one here (?):
https://git.drupalcode.org/project/leaflet/-/blob/10.2.x/leaflet.librari...

leaflet:
  remote: http://leafletjs.com/
  version: 1.9.4
  license:
    name: Leaflet-License
    url: https://github.com/Leaflet/Leaflet/blob/v1.9.4/LICENSE
    gpl-compatible: true
  js:
    js/leaflet/dist/leaflet.js: {}
  css:
    component:
      js/leaflet/dist/leaflet.css: {}

All is indeed (already) being embedded in the module itself and loaded from it.

And if not (if it wasn't the case) you should know that you could always implement your own hook_library_info_alter() and alter it in your own way ...
(rather than defining and adding a new my_theme.libraries.yml, kind of un-appropriate).

Further more you could have also better inspected already existing Drupal leaflet module issues and easily found that GDPR issues / matters have already been discussed in this existing fixed issue:
https://www.drupal.org/project/leaflet/issues/3086698 Use Leaflet GDPR conform Active

Please, make your deep(er) investigation and inspection of all these things before opening new issues, that look so far from reflecting the actual version of the Drupal Leaflet module (because everybody time is precious ... ).

Confident all this helps and points you in the proper direction ... and happy Drupal use & contribution too
(sorry to mention, but your longstanding Drupal profile looks still soo sadly empty on that side).

🇮🇹Italy itamair

thanks @aren33k for reporting this ...
well, indeed something could have happened moving from 10.3.x into 10.4.x Drupal Core that I am not clearly aware of ...
but let's try to better describe and fix this issue of you.

First of all, if you the Leaflet Map that it means that the following events:

'leafletMapInit': https://git.drupalcode.org/project/leaflet/-/blob/10.2.x/js/leaflet.drup...
'leaflet.map': https://git.drupalcode.org/project/leaflet/-/blob/10.2.x/js/leaflet.drup...

are both being fired (for sure).
They act exactly as the same, and could be use without any difference.
The 'leafletMapInit' was introduced to provide a better (self explaining) naming, and the 'leaflet.map' was still kept not to break backward compatibility.

What is rather and (very) probably happening is that something changed in the ways different js libraries are being loaded, in terms of each other dependency.

For having this code of you:

$(document).bind('leaflet.map', function (event, settings, lMap) {
        console.log('fired')
      });

correctly intercept (react upon) on of those 2 triggered events you js library (that embeds that code) should be loaded BEFORE the leaflet.leaflet-drupal library itself, that has now the following default definition:

leaflet-drupal:
  js:
    js/leaflet.drupal.js: {}
  dependencies:
    - core/jquery
    - core/once
    - core/drupal
    - leaflet/leaflet

has you can see from here: https://git.drupalcode.org/project/leaflet/-/blob/10.2.x/leaflet.librari...

So, it means that [your_custom_module.your_custom_library] should become an additional dependency on that.
How could this be done?

Well ... you need to implement the following hook_library_info_alter() in [your_custom_module.module] file,
such the following:

/**
 * Implements hook_library_info_alter().
 */
function  your_custom_module_library_info_alter(&$libraries, $extension) {
  if ($extension == "leaflet") {
   $libraries['leaflet-drupal']['dependencies'][] = ' your_custom_module/.your_custom_library';
  }
}

In that way you will force and inject your library as dependency for the leaflet-drupal library itself (as defined by the Leaflet module/extension),
and correctly make your code ready for listening and catch the 'leaflet.map' (or 'leafletMapInit) event ...

All clear?

Otherwise, second quick (no code) option would be to simply enable " Lazy load map" for your Leaflet View Style or Leaflet Formatter option ... (@see the attached screenshot).

Hope (and very confident) this will help and fix this issue of you ... that s not indeed a clear Bug of the Leaflet module.
Let's change all this into a Support Request.

And let us know if indeed all this fixes this, and move it accordingly to Fixed or Needs Work/Active, etc.

🇮🇹Italy itamair

could you also try yourself on a local Drupal CMS instance of you ... or any other Drupal 9 or 10 instance?

🇮🇹Italy itamair

thanks for the tip ...
but no way, still.

~/Sites/Drupal_CMS_package/drupal-cms git:[master]
composer show drupal/drupal_cms_geo_images -a --no-cache
name     : drupal/drupal_cms_geo_images
descrip. : 
keywords : 
versions : 1.1.0, 1.0.6, 1.0.5, 1.0.4, 1.0.3, 1.0.2, 1.0.1, 1.0.0
type     : drupal-module
license  : GNU General Public License v2.0 or later (GPL-2.0-or-later) (OSI approved) https://spdx.org/licenses/GPL-2.0-or-later.html#licenseText
homepage : https://www.drupal.org/project/drupal_cms_geo_images
source   : [git] https://git.drupalcode.org/project/drupal_cms_geo_images.git 1.1.0
dist     : [zip] https://ftp.drupal.org/files/projects/drupal_cms_geo_images-1.1.0.zip 1.1.0
names    : drupal/drupal_cms_geo_images

support
source : https://git.drupalcode.org/project/drupal_cms_geo_images

requires
drupal/core ~8.0
🇮🇹Italy itamair

Sorry @drumm but I feel to reopen this,
as the following composer require commands:

ddev composer require drupal/drupal_cms_geo_images
ddev composer require drupal/drupal_cms_geo_images:^1.0

are always and still returning the following (same require drupal/core ~8.0) error:

  Problem 1
    - Root composer.json requires drupal/drupal_cms_geo_images * -> satisfiable by drupal/drupal_cms_geo_images[1.0.0, ..., 1.1.0].
    - drupal/drupal_cms_geo_images[1.0.0, ..., 1.1.0] require drupal/core ~8.0 -> found drupal/core[8.0.0-beta6, ..., 8.9.x-dev] but the package is fixed to 11.1.2 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.
You can also try re-running composer require with an explicit version constraint, e.g. "composer require drupal/drupal_cms_geo_images:*" to figure out if any version is installable, or "composer require drupal/drupal_cms_geo_images:^2.1" if you know which you need.

and also the following

ddev composer show drupal/drupal_cms_geo_images -a

still returns

requires
drupal/core ~8.0

Why is this still not working as expected, and simply

requires
"drupal/core": ">=10.4"

Its code base is indeed the same of this parallel packagist, deployed on the branch 2.x with composer.json name: itamair/drupal_cms_geo_images
and on which the following composer require works and applies fine on Drupal CMS (11.2):

ddev composer require itamair/drupal_cms_geo_images

Could you also test and confirm that I am not doing anything wrong here and what could be wrong still and instead?

🇮🇹Italy itamair

thanks for reporting this @morganlyndel ... I quickly inspected this, but indeed this is not on the Drupal side.
It is all on the Google Maps way and how they are providing those map controls structure ...

Closing this as "works as designed".

Feel free to re-open if you better inspect and rather find that it is the opposite case (all due to Drupal)

🇮🇹Italy itamair

thanks a lot @drumm for this quick fix.

Well, of course I looked into the
Drupal.org Recipes page: https://www.drupal.org/docs/extending-drupal/drupal-recipes
(and nothing from there, and also both in the Distributions and Recipes initiative and the Drupal Recipe Cookbook referenced pages, etc.)

I also think something should / might be clearly mentioned in the following pages:

🇮🇹Italy itamair

Ok good. yes now you Leaflet Popups look perfect and well aligned!

And let's keep this as my further comment to attach the Credits to.
Well yes, I committed the new version because I clearly saw that fix...
and I (and my anarchistic workflow :-) ) was just expecting a RTBC here, before my final Fix.

🇮🇹Italy itamair

@ressa a mean this ... (@see attachment)

🇮🇹Italy itamair

Nice ... and thanks @ressa for marking this as Fixed,
but two final questions from me:

  1. in the map you mentioned it looks the Leaflet Popup is appearing in place, but not perfectly aligned (horizontally) with the DIV Icon ... May be that is the wanted outcome or a minor issue of the Div Icon itself?
  2. did you properly assign / checked Credits to all the contributors here, before setting this as Fixed?
🇮🇹Italy itamair

thanks @ressa ... your report better brought me to the proper direction and fix (hopefully).

Could you (everybody) QA and Review the 10.2.39 release that I just released (very confidentially), with this last commit: https://git.drupalcode.org/project/leaflet/-/commit/a8a203b21d3932238760...
for fixing this bug?

🇮🇹Italy itamair

hey ... @ducdebreme thanks for reporting this.
BUT why do you assume this is a bug?
Do you have clear evidence of that?
Do you provide it ... ?
Or do you simply feel things don't work as you expect?
Well, that is not a clear bug ... but probably something that you missed or misunderstand.
Please don't provide Issues marked as BUG, when you don't have (and provide) clear evidence of that.
Rather create Support requests, that could be eventually turned into Bugs, when confirmed.

Indeed in this case you are simply mislead.
If the geocoder_address submodule is enabled, Address field can be indeed:
be Geocoded from an existing field
or Reverse Geocode from a Geofield type existing field.
(as mentioned in your screenshot itself).

🇮🇹Italy itamair

Ah pretty weird @orkut murat yılmaz eh!
It would be great (much better) if you could also add some screenshot that make more sense clear your use case
(i.e. that show your maker at zoom 5 and how then it looks, and where, at zoom:12, etc).

Just going kind of blind,
and according to this solution (https://stackoverflow.com/questions/17875438/leafletjs-markers-move-on-zoom)
you might try to set dimensions of the Leaflet Icon
(@see the attached screenshot)

🇮🇹Italy itamair

Ah damn! Right you are ... I simply forgot about that "get push access" button.
So I did and pushed my further commit.
As said, in this state MR!458 fixes this issue and use case of mine ...

🇮🇹Italy itamair

Only when it is helpful …
In this case it was quick and helpful.

🇮🇹Italy itamair

@ I QA and Tested the MR!458 state, but it just forward another error further in the ai_automator code workflow.

It looks that I am not permitted to push commits into it,
thus I describe my contribution / solution ...
https://git.drupalcode.org/project/ai/-/merge_requests/458#note_458716

That further change (that I suggest) solves this isse to me.

🇮🇹Italy itamair

Thanks al lot @mrdalesmith ... amazing explanation and support!
I will read properly all the documentation.

Hope you will make this error breaking everything more silently ..

🇮🇹Italy itamair

Here it is ... @mrdalesmith

uuid: f0db98a4-198c-463c-ad89-4156ade5cf36
langcode: en
status: true
dependencies:
  config:
    - field.field.node.geo_image.field_geoplace_type
id: node.geo_image.field_geoplace_type.default
label: 'Geoplace Type Default'
rule: llm_entity_reference
input_mode: token
weight: 100
worker_type: direct
entity_type: node
bundle: geo_image
field_name: field_geoplace_type
edit_mode: true
base_field: revision_log
prompt: ''
token: 'Select a Value based on the [node:field_geo_image_media:entity:field_media_image] of the Geo Image Media field for this content'
plugin_config:
  automator_enabled: 1
  automator_rule: llm_entity_reference
  automator_entity_reference_bundle: geoplaces_types
  automator_entity_field_enable_tid: 0
  automator_entity_field_generate_tid: ''
  automator_entity_field_enable_revision_id: 0
  automator_entity_field_generate_revision_id: ''
  automator_entity_field_enable_revision_log_message: 0
  automator_entity_field_generate_revision_log_message: ''
  automator_entity_field_enable_name: 1
  automator_entity_field_generate_name: 'Select a Value based on the [node:field_geo_image_media:entity:field_media_image] of the Geo Image Media field for this content'
  automator_entity_field_enable_description: 0
  automator_entity_field_generate_description: ''
  automator_entity_field_enable_weight: 0
  automator_entity_field_generate_weight: ''
  automator_entity_field_enable_publish_state: 0
  automator_entity_field_generate_publish_state: ''
  automator_entity_field_enable_unpublish_state: 0
  automator_entity_field_generate_unpublish_state: ''
  automator_mode: token
  automator_base_field: revision_log
  automator_prompt: ''
  automator_token: 'Select a Value based on the [node:field_geo_image_media:entity:field_media_image] of the Geo Image Media field for this content'
  automator_edit_mode: 1
  automator_label: 'Geoplace Type Default'
  automator_weight: '100'
  automator_worker_type: direct
  automator_ai_provider: default_json
🇮🇹Italy itamair

Markecluster options in Drupal Leaflet module can be fine tuned in the MARKER CLUSTERING section of the Leaflet View Display settings.
Didn't you look at it?

@sceensot attached

That section redirects you to the Leaflet Markercluster Js Library (https://github.com/Leaflet/Leaflet.markercluster).
All the Options mentioned in the Options section should be supported.

🇮🇹Italy itamair

Because of comment #3 this doesn't look a Leaflet module bug,
and anyway there is no clear proof/evidence of Leaflet module bug anyway
(things that don't work the way you expect don't naturally translate in a module bug ... but you should provide clear evidence it is a bug).

Hence changing this into Support Request ... (this clearly seems an issue in properly imposing js libraries in a specific Drupal instance setup).

🇮🇹Italy itamair

No big deal, no worries ... @jcory.
Did you check the code changes here (?): https://git.drupalcode.org/project/leaflet/-/commit/5b84bb901aad08941642...

There is no AI / DeepSeek checking if the FileExists,
but I just tryed DeepSeek out for that method refactoring and I, after successfully QAing and approving that,
I decided to commit that result (and refactoring), and just wanted to mention that DeepSeek helped on that.

What kind of issue do you see on this?
Do you report any issue with that specific code refactoring?

🇮🇹Italy itamair

Thanks here.
I pushed a couple of commits that basically exactly reflect the status of the MR !54 (at this status ... with @ressa addictions, and a couple of minor tweaks by me).
I QAed and tested and all working fine on my side.
I am thus going to close both this issue (as Fixed) so as the MR !54 (that cannot be merged anymore) and assign credits to all of you.

Going to deploy a new Drupal Leaflet module release with all this very soon ...

🇮🇹Italy itamair

I am having the same issue here and it looks to me that is not possible with this Color Field 3.x branch
Changing this into Feature Request ...

🇮🇹Italy itamair

thanks for reporting this @lakhal
not easy to reproduce your use case, that looks pretty specific ...

I would start better inspecting why that Chart JSON message is appearing instead of the expected chart, and that looks more on the Chart module, then Leaflet side.
Also I would inspect any js console error that happen, and it looks you have 1 at least in the not working case (@see attached screenshot).

You tech approach could look correct to me, in the Leaflet PopUp setup ("Add the embed chart view into leaflet block view as a view field"),
but I would also try another way with the ViewFiled module ( https://www.drupal.org/project/viewfield ) so to:

  1. generate the Chart View with a contextual filter based on a specific node id (so that the rendered Chart relates to the specific content / node / entity);
  2. add and properly setup a Viewfield to the content / node / entity type that you are going to make the Leaflet View map upon / based on, ad make it rendering the Chart View (with the related contextual filter;
  3. add that Viewfield field to the Leaflet View fields list;
  4. setup the Leaflet Popup to use that Viewfield field ...

and check if that way it works.

🇮🇹Italy itamair

thanks @danharper .. nice catch,
as I missed these last adjustments to get rid of PHP 8.4 warnings ...
Merging and Committing into a new 11.0.5 release.

🇮🇹Italy itamair

Thanks @uridrupal ... all clear.
Nevertheless the new 10.2.35 release if worth with more defensive check.

🇮🇹Italy itamair

Ok. Let's add that ... as I QAed and is not harmful and rather should guarantee better check that .bindPopup method is supported on each lFeature.
I committed #7 into 10.2.x branch and just deployed a new leaflet 10.2.35 release with all this ...

Please @uridrupal still upgrade to 10.2.35 and let us know if this new release fixes also this use case issue of you, eventually.

🇮🇹Italy itamair

May you try the attached patch, asap?
Latest release 10.2.34 did change part of that code (that triggers your error) but I cannot figure out why you now trigger/intercept an lFeature object for which the bindPopup method is not defined ...

Let me know ASAP if it solves this issue of you. Might be a good defensive code addition to commit into a new (last minute) Leaflet release,
just one day before the official launch of the Drupal CMS (Leaflet module is part of ...)

🇮🇹Italy itamair

Thanks @uridrupal for reporting this.
BUT ... could you be more detailed in reporting this?
When the regressions exactly happened to you? In which circumstances?

Did you upgrade from which Leaflet version into the 10.2.34 mentioned in this issue of you.
Because important API changes happened from previous Leaflet versions into 10.x ones.

Or did it happened in a more recent 10.x.x upgrade?
You should inspect your upgrades and specify all that ... and may be you (yourselves) should be able to understand what impacted in your custom code alters ...

🇮🇹Italy itamair

Please @ekes feel free to remove leaflet-3498789 fork, if manually needed.

🇮🇹Italy itamair

Nice job @ekes (as usual). Fork leaflet-3498789 merged, though MR not created.
Thanks!

🇮🇹Italy itamair

Thanks here. Confirming RTBC ... going to merge this.
Going also to extend all this to other Classes methods still Missing proper parameter's type declaration.

🇮🇹Italy itamair

Hey guys here ... I have received some direct email claiming for credits on this,
but at the moment the Drupal Crediting system is a bit weird and not efficient.
I mentioned all of you in this merge commit: https://git.drupalcode.org/project/geofield/-/commit/cbfe9979bbe62379a7a...

BUT it seems Credits are only given the first time the issue is marked as Fixed, and the contributors names are checked ...
and if they don't, may be the credits are not really assigned in this weird system.

Sorry. May be I missed that ... then I retried doing that (set bak to Needs Work and then again into Fixed, having the contributors checked) but may be second time we do it doesn't apply anymore.

I DON'T KNOW what is not working in the Drupal.org Crediting system ... or why it is so unreliable.
Sorry if someone wasn't credited for all this.
But I cannot do more than this ... (and I have a lot of other stuff to stay after, and not really for free).

🇮🇹Italy itamair

Ok thanks ... I just provided a new commit with the attached more appropriate Geofield Icon, better matching its Project Browser correspondent.

Merging MR !39 and deploying a new Geofield 8.x-1.63 release with this.

🇮🇹Italy itamair

itamair made their first commit to this issue’s fork.

Production build 0.71.5 2024