By default, in the Leaflet field default formatter, the label used for marker is the entity label.

We could allow to set a custom text, supporting tokens replacement, in the Leaflet widget, and so permit any fields of the entity to be used as description for the marker.

Patch follow.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

flocondetoile created an issue. See original summary.

flocondetoile’s picture

Status: Active » Needs review
FileSize
7.72 KB

Thanks for your review.

flocondetoile’s picture

Issue summary: View changes
flocondetoile’s picture

FileSize
7.02 KB

Patch rerolled against beta12

itamair’s picture

tnx @flocondetoile ... sounds great.
Something that I was minding to do, at least to pair the leaflet map viewstyle option / functionality.
Will review this asap.

itamair’s picture

Status: Needs review » Needs work

Then here is the outcome of my (quite deep) review ... (nice approach and effort, but not satisfing and sufficient).

I enabled the Token module and applied this patch, but I hit the following snugs, that IMO should be addressed and properly fixed for you patch be committed in the module.

Additional work is needed to complete the following:

  • once I checked the Popup Infowindow option and I fill the Popup title OR the Popup text with some free text, or token … nothing happens on the Content View /Lefalet Map when I click on the marker (no Infowindow Popup): so actually it means this doesn’t work, in its most important functionality. (No backend log message neither any js console warning/error).

Besides that the following also matter and should be addressed:

  • Replacement tokens are exposed to the user ONLY if the module Token exists in the project. (but the Token module shouldn’t be a Leaflet module requirement). So ALL the Token related functionalities should be added (wrapped) checking if the Token module exists and is enabled. (\Drupal::moduleHandler()->moduleExists(‘token’). Everything should work seamlessly and differently in the two scenarios. And the end user should be properly informed of the Token related additional funciotnalities.
  • As the actual present behaviour (without patching), the Content Title should be output as Pop Up default content, if Popup Infowindow option is checked and Popup title & Popup text are both empty.
  • I don’t really think there is the need of two fields to compose the Pop Up content. One text (long text) field should be enough, as it is usually for similar cases, such as in the “Override the output of this field with custom text” option on a view display field output setup.
  • This text field would/might be able to accept tokens and simple text and html tags (and even Twig?) exactly as the above mentioned. It would be better if the Pop Up related form elements are grouped together in the form UI. (under a fieldset or detail element) taking care to correctly manage/save the new settingsForm structure and adapt the viewElements method results accordingly (for the subsequent Leaflet js processing)

  • itamair committed 048610b on 8.x-1.x authored by flocondetoile
    Issue #2794993 by flocondetoile: Permit to customize the label displayed...

  • itamair committed 99a725e on 8.x-1.x
    Revert "Issue #2794993 by flocondetoile: Permit to customize the label...
Panyamin’s picture

Changes since the last patch:
- Fix: used wrong array

Tested, now working great!

Patch and interdiff created by Dorgflow.

Panyamin’s picture

itamair’s picture

Thanks @Panyamin for the update.
Yes it is seems to work properly now with these two new text fields (Popup title, Popup text) and html text/token inside them,
but still the following (among the above listed) enhancements should be implemented, to make the patch ready to be committed into the dev branch:

  • Replacement tokens are exposed to the user ONLY if the module Token exists in the project. (but the Token module shouldn’t be a Leaflet module requirement). So ALL the Token related functionalities should be added (wrapped) checking if the Token module exists and is enabled. (\Drupal::moduleHandler()->moduleExists(‘token’). Everything should work seamlessly and differently in the two scenarios. And the end user should be properly informed of the Token related additional funciotnalities.
  • I don’t really think there is the need of two fields to compose the Pop Up content. One text (long text) field should be enough, as it is usually for similar cases, such as in the “Override the output of this field with custom text” option on a view display field output setup.
  • all the Pop Up related form elements should be grouped together in the form UI (under a fieldset or detail element) taking care to correctly manage/save the new settingsForm structure and adapt the viewElements method results accordingly (for the subsequent Leaflet js processing)
itamair’s picture

Here is the patch that starting from the previous is accomplishing all the above listed (needed) additional requirements,
and that is going to be committed to dev (and next release).
tnx @flocondetoile. I am granting you for the first suggesting patch ...

  • itamair committed 3e2d7db on 8.x-1.x
    Issue #2794993 by flocondetoile, Panyamin, itamair: Permit to customize...
  • itamair committed 91a88de on 8.x-1.x authored by Panyamin
    Issue #2794993 by flocondetoile, Panyamin, itamair: initial draft to...
itamair’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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