# Map markers on a canvas that pop up boxes when clicked

Here's a working version of my code - http://jsfiddle.net/sambenson/g3qsa/#base
How can this be improved upon?

$(function(){ var box = {}, island = {}, canvas = {}; box.el =$('#box');
canvas.el = $('#arrow'); function go(el){ if(box.el.css('visibility') == 'hidden'){ reset_name();$('.island').addClass('inactive');

box.top = box.el.offset().top;
box.corner = {};
box.corner.left = box.el.offset().left;
box.corner.right = box.corner.left + box.el.width();

island.el = el;
island.top = island.el.offset().top;
island.left = island.el.offset().left;

canvas.width = island.left - box.corner.left;
canvas.height = island.top - box.top;

canvas.el.css({
top: box.top,
left: box.corner.left
}).attr({
width: canvas.width,
height: canvas.height
});

var context = canvas.el[0].getContext('2d');

context.beginPath();
context.moveTo(0, 0);
context.lineTo(box.el.width(), 0);
context.lineTo(canvas.width, canvas.height);
context.closePath();

context.fillStyle = "#eaeaea";
context.fill();

box.el.css('visibility', 'visible');
canvas.el.show();
}
else {
close();
}
}

function reset_name(){
$('#name_holder').text('').css({ top: -9999, left: -9999 }) } function close(){$('.island').removeClass('inactive active');
box.el.css('visibility', 'hidden');
canvas.el[0].width = canvas.el[0].width;
canvas.el.hide();
}

$('.island').hover(function(){ if(box.el.css('visibility') == 'hidden'){ var t =$(this),
name = t.text(),
holder = $('#name_holder'); top = t.offset().top - 60; left = t.offset().left - 50; holder.css({ top: top, left: left }).text(name); } }, function(){ reset_name(); }) .click(function(){ go($(this)); })

$('.close').click(function(){ close(); }) })  Could it be made faster? Could it be made more efficient? ## 2 Answers Along with @kingjv's suggestions, you can improve the performance of your jQuery selections by making some simply tweaks to the selectors. Consider if I have the following html: <div id="container"> <div class="cell">Cell 1</div> <div class="cell">Cell 2</div> </div>  If you I am using a class selector, such as $(".cell") then the selector engine has to do a little more than it needs to. In this example, the selector engine has to walk the entire DOM to match the correct elements. We can improve this a couple of ways:

• Specify a tag in the selector, e.g. $("div.cell"). This allows the selector engine to use the native getElementsByTagName method to shrink the number of comparisons the engine has to do to match. • Specify a root object to start searching from. E.g., if I've previously cache my selector var root =$("#container"), I can reuse that selector: $("div.cell", root) that way we don't walk the entire DOM for matches. These are small performance improvements, you'll appreciate them more if you are doing a lot of DOM manipulation and lookup. More simplistic applications won't have such an apparent performance improvement. My comments are more just on style as opposed to performance: Instead of using : if(box.el.css('visibility') == 'hidden'){  it's more readable to use : if(box.el.is(":hidden")){  And similarly, instead of using : box.el.css('visibility', 'visible');  you can use : box.el.hide();  A note on this one though, these are not exactly equivalent because hide actually sets display: none. In this case though I believe that's fine. And one final comment, it's a good practice to name variables which contain jQuery objects with a $ at the beginning. This allows you to know that the variable is already a jQuery object without going back in the code to check and avoids accidentally creating jQuery objects out of objects that were already jQuery objects.

So something like this:

var t = $(this), name = t.text()  would become this: var$t = $(this), name =$t.text()

• Thanks for your review. With regards to your first point: I had to use visibility: hidden instead of .hide() etc because the jQuery needs to be able to measure the width of the box whilst it's hidden. Your \$variable_name suggestion is great though!
– Sam
Jul 14, 2011 at 14:05
• Ah okay, I looked to see if you had visibility for a reason but missed that. Can still use :hidden though I believe. Jul 14, 2011 at 14:08