bozdoz / wp-plugin-leaflet-map

Add leaflet maps to Wordpress with shortcodes
https://wordpress.org/plugins/leaflet-map/
GNU General Public License v2.0
140 stars 71 forks source link

Add leaflet plug-in version to enquire scripts #222

Open bozdoz opened 1 year ago

bozdoz commented 1 year ago

From: https://wordpress.org/support/topic/add-leaflet-version-to-enqueue-commands/

webd-uk commented 1 year ago

In the meantime, I'm using this ...

add_action('wp_enqueue_scripts', 'temporary_wp_enqueue_scripts', 11);
function temporary_wp_enqueue_scripts(){

            global $wp_scripts, $wp_styles;

            if (isset($wp_scripts->registered['leaflet_js'])) {

                $wp_scripts->registered['leaflet_js']->src = parse_url($wp_scripts->registered['leaflet_js']->src, PHP_URL_PATH);
                $wp_scripts->registered['leaflet_js']->ver = apply_filters('leaflet_version', Leaflet_Map::$leaflet_version);

            }

            if (isset($wp_styles->registered['leaflet_stylesheet'])) {

                $wp_styles->registered['leaflet_stylesheet']->src = parse_url($wp_styles->registered['leaflet_stylesheet']->src, PHP_URL_PATH);
                $wp_styles->registered['leaflet_stylesheet']->ver = apply_filters('leaflet_version', Leaflet_Map::$leaflet_version);

            }

            if (isset($wp_scripts->registered['wp_leaflet_map'])) {

                $wp_scripts->registered['wp_leaflet_map']->src = parse_url($wp_scripts->registered['wp_leaflet_map']->src, PHP_URL_PATH);

            }

}

The use of parse_url() is because I'm trying to make your plugin compatible with this plugin: https://wordpress.org/plugins/merge-minify-refresh/

Currently your plugin breaks and we have to set it to ignore /wp-content/plugins/leaflet-map/scripts/construct-leaflet-map.js

hupe13 commented 1 year ago

Due to your question I was just wondering why the minified versions are not loaded for me, even though they exist. But the code is:

https://github.com/bozdoz/wp-plugin-leaflet-map/blob/b7fb694f6695e1b5f2b1df8ee17abd0bf40bb082/class.leaflet-map.php#L207-L211

and

https://github.com/bozdoz/wp-plugin-leaflet-map/blob/b7fb694f6695e1b5f2b1df8ee17abd0bf40bb082/class.leaflet-map.php#L218

My setting WP_DEBUG is true in wp-config.php. Does it make sense to load the non-minified version when WP_DEBUG is true? There better be an option in the plugin?

webd-uk commented 1 year ago

OMG I was actually going to address that in the code above :)

I have WP_DEBUG on all the time because I need to know immediately if there are any issues ... so this doesn't work for me.

Personally I wouldn't have done that because none of your users will want the non-minified version and probably won't realise that's what's causing it to load (as I didn't until I saw that bit of code). Perhaps you could use define('LEAFLET_JS_MIN', true) and if (defined('LEAFLET_JS_MIN') && LEAFLET_JS_MIN) {} in your development environment instead ... ?

In the meantime, I've amended my temporary code above to ...

                $wp_scripts->registered['wp_leaflet_map']->src = str_replace('map.js', 'map.min.js', parse_url($wp_scripts->registered['wp_leaflet_map']->src, PHP_URL_PATH));
webd-uk commented 1 year ago

OK, and now I've worked out why using "Merge + Minify + Refresh" plugin breaks the Leaflet JS maps. It's because you have set $in_footer to true when registering the script ...

        wp_register_script('leaflet_js', $js_url, Array(), null, true);

So the inline JS for each map can't find the L variable.

Instead I would use this (but change null for the version of Leaflet JS) ...

        wp_register_script('leaflet_js', $js_url, Array(), null);

So, here's the new code which puts leaflet.js in the header and uses the minified code ...

add_action('wp_enqueue_scripts', 'temporary_wp_enqueue_scripts', 11);

function temporary_wp_enqueue_scripts(){

            global $wp_scripts, $wp_styles;

            if (isset($wp_scripts->registered['leaflet_js'])) {

// Change to relative URL
                $wp_scripts->registered['leaflet_js']->src = parse_url($wp_scripts->registered['leaflet_js']->src, PHP_URL_PATH);
// Add Leaflet JS version
                $wp_scripts->registered['leaflet_js']->ver = apply_filters('leaflet_version', Leaflet_Map::$leaflet_version);
// Put the script in <head>
                unset($wp_scripts->registered['leaflet_js']->extra['group']);

            }

            if (isset($wp_styles->registered['leaflet_stylesheet'])) {

// Change to relative URL
                $wp_styles->registered['leaflet_stylesheet']->src = parse_url($wp_styles->registered['leaflet_stylesheet']->src, PHP_URL_PATH);
// Add Leaflet JS version
                $wp_styles->registered['leaflet_stylesheet']->ver = apply_filters('leaflet_version', Leaflet_Map::$leaflet_version);

            }

            if (isset($wp_scripts->registered['wp_leaflet_map'])) {

// Change to relative URL and use minified script
                $wp_scripts->registered['wp_leaflet_map']->src = str_replace('map.js', 'map.min.js', parse_url($wp_scripts->registered['wp_leaflet_map']->src, PHP_URL_PATH));

            }

}
webd-uk commented 1 year ago

Actually, this works too (loading construct-leaflet-map.js in the footer instead of the header) ...

        wp_register_script('wp_leaflet_map', plugins_url(sprintf('scripts/construct-leaflet-map%s.js', $minified), __FILE__), Array('leaflet_js'), LEAFLET_MAP__PLUGIN_VERSION, true);

Basically I think both scripts need to be loaded in the header or the footer but not one in each.

bozdoz commented 1 year ago

Check the PR #217 to see if that's what you're looking for

webd-uk commented 1 year ago

Great. I've added a couple of comments.