doublesecretagency / craft-googlemaps

Google Maps plugin for Craft CMS - Maps in minutes. Powered by the Google Maps API.
https://plugins.doublesecretagency.com/google-maps/
Other
10 stars 9 forks source link

Custom marker clustering icons not working (using Sprig) #56

Closed darylknight closed 2 years ago

darylknight commented 2 years ago

I'm in the process of updating and testing to fix https://github.com/putyourlightson/craft-sprig/issues/175 and ran into a new issue, but not sure if it's related to Craft 4.

Following this guide: https://plugins.doublesecretagency.com/google-maps/guides/setting-clustering-icons/#download-the-renderer-js-file, I haven't changed anything from the Craft 3 version, and now loading the page I'm seeing "Uncaught ReferenceError: CustomRenderer is not defined" in the console when loading the map. Everything works except that the clustering icons are back to the default red/blue (because it can't load the custom renderer).

Is it possible that something broke here, or am I doing / not doing something I'm supposed to? I know the path is correct as I added a console.log() to the customrenderer JS file I created.

Stripped down version:

{# Create the options for map appearance #}
{% set mapRenderOptions = {
    id: 'projects-map',
    height: 650,
    cluster: {
        renderer: 'CustomRenderer',
        imagePath: '/assets/images/map-clusters/m'
    },
} %}

{{ map.tag() }}

{% js url('/assets/js/renderer.js') %}
lindseydiloreto commented 2 years ago

The solution for that original Sprig conflict was to move a large chunk of JavaScript. Instead of being rendered in the footer, that chunk of JS is now rendered inline, immediately after the map <div>. This allows the entire thing to be contained within your Sprig tag.

As a side effect, it means that some of the Google Maps JS is configured before the footer has been reached. You'll most likely just need to load your clustering file in the page's header (instead of the footer).

Try adding at head to this line of code...

{% js url('/assets/js/renderer.js') at head %}
darylknight commented 2 years ago

That solved it, thank you!

lindseydiloreto commented 2 years ago

Ok great, thanks for reporting back! 👍

bencroker commented 2 years ago

Just came across this same issue, please consider updating the docs at https://plugins.doublesecretagency.com/google-maps/guides/setting-clustering-icons/#load-js-script-into-the-page to reflect this.

lindseydiloreto commented 2 years ago

Done...

Ω  2022-07-21  14 35 02
bencroker commented 2 years ago

That appears to have broken the page at https://plugins.doublesecretagency.com/google-maps/guides/setting-clustering-icons/

Screenshot 2022-07-21 at 23 40 33
lindseydiloreto commented 2 years ago

Whoops, just a minor Vuepress issue. Should be all fixed now! 👍

bencroker commented 2 years ago

Thanks!

bencroker commented 2 years ago

May I ask why this only applies when using Sprig? Was this behaviour not changed universally in the Google Maps plugin?

lindseydiloreto commented 2 years ago

It was not changed universally. For non-Sprig sites, any map-related JS is still injected into the footer (which is preferred).

However, that isn't possible for Sprig sites... when someone is dynamically replacing DOM elements with Sprig, the injected JS must be included immediately after the relevant <div> tag. This ensures that the JS gets properly re-written when the Sprig elements are reloaded.

bencroker commented 2 years ago

Hmm, it looks like you're only checking whether the Sprig Core class exists: https://github.com/doublesecretagency/craft-googlemaps/blob/228219e34881e964e348163a4d855a65bce979c4/src/models/DynamicMap.php#L509-L519

Which doesn't cover a few scenarios such as:

  1. Sprig Core is required in composer but not installed.
  2. Sprig Core is installed but not being used in this particular instance of the map.

Is there any reason to not always place the markup inline, regardless of whether Sprig is being used or not? That seems like it would take care of other scenarios in which people might want to dynamically load maps via AJAX.

lindseydiloreto commented 2 years ago

Because the map could possibly generate a lot of JS... typically, that much JS should be loaded in the footer, if at all possible.

Let's hop over to Discord if you want to discuss the philosophy behind the change. I think we've gotten a bit off topic from this original Github thread.

bencroker commented 2 years ago

Sounds good, thanks!

lindseydiloreto commented 2 years ago

Per our conversation on Discord, you suggested adding a new option in order to manually control whether or not the JS appears inline (as opposed to in the footer). This is a great suggestion, because usage does not match 1:1 with whether or not Sprig is installed.

So as of Google Maps v4.2.5, there is a new inline option to specifically load dynamic JS inline...

Release notes for v4.2.5

Here's how it's described in the docs...

Documentation for new inline option

Great suggestion, thanks @bencroker! 🍺

bencroker commented 2 years ago

Thanks @lindseydiloreto!!