Use HTML element with data-attributes instead of JavaScript for rendering shortcodes

[scrobbleme]

Hello,

The current behavior, directly rendering JavaScript into the content is quite error prone with page builder and other tools.
I.e. I'm using Toolset quite often and can't use the shortcodes within templates as the JavaScript is broken than. I've always workaround this with escaping the code and than executing on DOMContentLoaded (which also improves Caching compatibility btw)

I think it would be better to render a hidden div (or whatever fits) with data attributes and than load it via JavaScript instead.
(beside that it needlessly much JavaScript code printed...)

[bozdoz]

The main script does execute on load: https://github.com/bozdoz/wp-plugin-leaflet-map/blob/master/scripts/construct-leaflet-map.js#L372-L377

As for the "hidden div (or whatever fits)", are you suggesting that instead of rendering <script> tags just to render DOM nodes with attributes that can be parsed? Something like:

<div class="wp-leaflet-map-plugin-node" data-instructions="{...some JSON data here}">

Or are you suggesting something else?

If that is what you're suggesting, I love it. Generally, I'd like to have PHP merely create some JSON-input for Javascript. It was an Issue I created awhile back, but closed because it was too vague: #93

I'm not sure if the above DOM example with data attributes would work or not, but I'd love to test it out.

[scrobbleme]

Exactly like this, the div itself would be hidden...

Just one encoded JSON is fine, it might make sence to extract some "key" attributes directly, to make it more obvious.
The name might be data-leaflet to be more precise as well.

Maybe also, the content of the popup (if any) could be the content of the div, so this must not be encoded as JSON.

<div data-leftlet="map1" data-leaflet-lat="" data-leaflet-long="" ... data-leaflet-configuration="{...some JSON data here}">
      This is the marker "content"
</div>

Than whenever map was loaded it could load its markers:

document.querySelectorAll('[data-leaflet="+ mapId +"]').forEach(marker => {
    // Add marker to map
});

An alternative to a div might be a script of type text/plain:

<script type="text/plain" data-leftlet="map1" data-leaflet-lat="" data-leaflet-long="" ... data-leaflet-configuration="{...some JSON data here}">
      This is the marker "content"
</div>

[emojized]

is this an issue belonging to mine? I render 1000 markers with the shortcodes and this is really slow... can I put them in one json`?

[scrobbleme]

@emojized What is your issue?

But generally yes, the way it works makes the page slower on load as all markers are evaluated immediately instead of deferred after page load (if i'm not wrong with this).
Especially with so many markers, you possibly have a lot of superfluous JavaScript code within your page.

[emojized]

so whart should i do then?

[scrobbleme]

We are using a custom marker shortcode to escape the leaflet shortcode and then load it later.
You can try this as well.

Unfortunately I can't give you the full code, but the idea is as follows:

add_shortcode('custom-leaflet-marker', function($atts, $content) {
  $result =  Leaflet_Marker_Shortcode::shortcode($atts, $content);
  $result  str_replace(['<script>', '</script>'], ['', ''], $result);
  return '<script data-leaflet="yes" type="text/plain">' . $result . '</script>';
});

Then you need a JavaScript based on load hook, like this:

document.addEventListener('DOMContentLoaded', _ => {
  document.querySelectorAll('script[data-leaflet="yes"]').forEach(script => {
      const markerScript = script.textContent;
      eval(markerScript);
  });
})

[bozdoz]

Thinking about this some more. I'm concerned about asynchronous sections of the page; the section loads with a map: currently it renders a script tag which directly calls the main script. If we rendered a div, or equivalent, instead, then I'm not sure how the main script will be made aware of it. Does this make sense to you? This makes me wary of making such a change. Does it solve your issue to wrap each script in DOMContentLoaded, like you said in your original post? I'm curious about your current solution involving escaping the code.

[scrobbleme]

@bozdoz Good point.

The main reason for me to use data attributes is not to avoid JavaScript at all, but to improve readability and esp. compatibility with i.e. caching plugins...

Ideas:

• still use divs with data attributes → just to improve readability, caching compatibility + only a very tiny "inline" JavaScript call to invoke the loading
• maybe have a look at MutationObserver. I also never used it so far, but I think this might work