2
\$\begingroup\$

I have inherited a site which uses the Google API to create a map on my webpage. I don't know much about JavaScript but from the research I've done, I dont think this code is optimal as it is not loading asynchronously. Any help or pointers would be appreciated.

function initialize() {
var myLatlng = new google.maps.LatLng(<?php echo get_post_meta($post->ID,'school_latitude',true); ?>, <?php echo get_post_meta($post->ID,'school_longitude',true); ?>);

var myOptions = {
  zoom: 11,
  center: myLatlng,
  mapTypeId: google.maps.MapTypeId.ROADMAP
 }

var map = new google.maps.Map(document.getElementById("map_canvas"), myOptions);

var marker = new google.maps.Marker({
  position: myLatlng,
  map: map,
  title: 'School'
});
}

function loadScript() {
var script = document.createElement("script");
script.type = "text/javascript";
script.src = "http://maps.google.com/maps/api/js?sensor=false&callback=initialize";
document.body.appendChild(script);
}

window.onload = loadScript;
</script>
\$\endgroup\$
  • \$\begingroup\$ Hi, welcome to Code Review! I hope you receive great answers! \$\endgroup\$ – Tunaki Apr 12 '16 at 15:43
  • \$\begingroup\$ Correct me if I'm wrong, but your trying to initialise google maps with markers on set locations? \$\endgroup\$ – Jackson Jacob Apr 12 '16 at 17:31
  • \$\begingroup\$ Yes, I have some custom fields in a wordpress page which take the postcode of an address and place a marker in the map \$\endgroup\$ – Paul Simpson Apr 13 '16 at 6:59
2
\$\begingroup\$
var myLatlng = new google.maps.LatLng(<?php echo get_post_meta($post->ID,'school_latitude',true); ?>, <?php echo get_post_meta($post->ID,'school_longitude',true); ?>);

Instead of echoing directly onto the script, consider putting the values in an array, convert it to JSON using json_encode, and echo the JSON to a variable outside your script. Then use that variable like any other JS variable. This is possible because JSON is (most of the time) valid JS object syntax. Afaik, this is how Drupal passes down configs from server to the client.

<?php
  $configs = [
    'lat' => get_post_meta($post->ID,'school_latitude',true),
    'lng' => get_post_meta($post->ID,'school_longitude',true)
  ];
?>

var configs = <?php echo json_encode($configs); ?>; // var configs = { "lat": "VALUE", "lng": "VALUE" };

initialize(configs.lat, configs.lng);

The advantage of this approach is that you lessen the area where PHP is in touch with JS, reducing any breakage caused by some mishap in printing values.

function loadScript() {
var script = document.createElement("script");
script.type = "text/javascript";
script.src = "http://maps.google.com/maps/api/js?sensor=false&callback=initialize";
document.body.appendChild(script);
}

window.onload = loadScript;

I'm pretty sure you did it this way because you wanted it to load async. First off, onload fires after the entire page, images and all are loaded and is probably the very last event that fires in a page load cycle. If you have a page that's heavy with images, onload will take forever to fire. Consider listening to DOMContentLoaded instead.

Next is the entire procedure itself. If you take a look at the Google script, there are hints that it's already doing the same thing. So I believe this procedure isn't necessary. Using a script tag should suffice.

In the end, only the following should be necessary. Order of the two shouldn't matter since the maps script will probably call callback asynchronously. initialize would have been defined by then.

<script src="http://maps.google.com/maps/api/js?sensor=false&callback=initialize"></script>
<script>
  function initialize() {
    ...
  }
</script>
\$\endgroup\$
  • \$\begingroup\$ Thanks for the detailed answer. I dont really understand it fully but will test it on my dev site and report back. \$\endgroup\$ – Paul Simpson Apr 14 '16 at 12:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.