jquery - how to make loading images on demand better

[Jeanne Boyarsky]

This code does work. It is called inside a conditional to ensure it only happens once. It goes through the emoticons urls (we have a url in the panel to the help so that one doesn't match) and creates/loads an image adding it to the panel.

$("div[id=emoticon_panel_header] a[data-url^=http]").each(function() { var img = new Image(); $(img).attr('src', $(this).attr('data-url')); $(this).append($(img)); });

My doubts are
1) This feels unreadable. It may because I rarely use a lot of jQuery in a small space. Or it is like how reg exps are unreadable without a ton of effort.
2) I have a suspicion that something inefficient is going on. I don't think I'm doing any extra lookups. The only thing I can think of is extra page re-arranging. Not sure of the tradeoff between repainting the panel and not seeing anything until it is done. I went with the former because it is only a problem for people with slow bandwidth. Which is when seeing something happening helps.

[Eric Pascarello]

Well your code takes one of the fastest seector and brings it to a halt. NEVER use id as an attribute lookup.


BAD BAD BAD
$("div[id=emoticon_panel_header]")
BAD BAD
$("div#emoticon_panel_header")

GOOD
$("#emoticon_panel_header")

Why are you looking for the data-url to have http? That is also being overly protective. Just check to see if the attribute is there

a[data-url]

Other option is to not rely on the data attribute, but add a class and use the class selector.

a.preload

Now stop mixing DOM and jQuery, pick one. Also jQuery has data(), no need to use an attribute look up.

var link = $(this); //look it up once $ is expensive. var img = $("<img/>").attr('src', link.data("url")); link.append(img);

[Jeanne Boyarsky]

Thanks Eric! Nothing like having your code ripped apart in public getting an awesome code review and learning a lot.

Eric Pascarello wrote:Well your code takes one of the fastest seector and brings it to a halt. NEVER use id as an attribute lookup.

Good point. I didn't think about it because I added my code under a couple lines under $("img[id$=emoticon_toggle_image]").click(...). And yes "emoticon_toggle_image" is the name, not the ending. I've fixed this too.

Eric Pascarello wrote:Why are you looking for the data-url to have http?

Because I didn't know you could just check for the presence of an attribute. I thought about using a class, but I still need a way of passing in the URL to load for each image.

Eric Pascarello wrote:Now stop mixing DOM and jQuery, pick one. Also jQuery has data(), no need to use an attribute look up.

I pick jQuery . Didn't know about data(). That's nice! I did know that $() was expensive, but I falsely assumed $(this) wouldn't be because it was "this".

Oddly data() is returning undefined. The second link works, the first does not.

var img = $("<img />").attr('src', link.data('url'));
var img = $("<img />").attr('src', link.attr('data-url'));

I think I need to re-read Bear's book. I am clearly ready for understanding the basics better over piecing together working code. It used to be a struggle to piece together working code so I wasn't ready then.

Seriously though. I've fixed the code and learned a lot from this post. Thanks again.

[Bear Bibeault]

In general, while selection is comparatively expensive, it's highly optimized and I generally don't worry too much about performance until it actually proves to be a problem, or I'm selecting inside a tight loop. Generally, simpler selectors are faster (but of course, there are always corner cases.)

I usually try to use the simplest selector that makes reading the code easy, and adjust if necessary.

Selecting the same thing over and over again is to be avoided, not only because it takes extra time, but , like in other code, if somethings in two or more places, it's a bear to maintain. So if you're going to need a wrapped set to hang around for multiple references, store it in a variable:

var allImages$ = $('img');

The $ in the variable name is a hint that the variable references a jQuery wrapped set.