jquery loops using .each to detect width and height

Question

I'm having a bit of trouble with jquery looping through a set of html code. I wrote a wee conditional script that detects if an image if portrait or landscape and adds a class for either. This worked great in isolation but now I need to to detect multiple instances on the same page...

<li>
 <figure class="eventimage">
  <img class="" src="images/home1.jpg">
 </figure>
 <div>
  some other code that is not important
 </div>
</li>
<li>
 <figure class="eventimage">
  <img class="" src="images/home2.jpg">
 </figure>
 <div>
   some other code that is not important
 </div>
</li>
<li>
 <figure class="eventimage">
  <img class="" src="images/home3.jpg">
 </figure>
 <div>
   some other code that is not important
 </div>
</li>
<li>
 <figure class="eventimage">
  <img class="" src="images/home4.jpg">
 </figure>
 <div>
   some other code that is not important
 </div>
</li>

So I thought it take advantage of the .each method...

$(".eventimage img").each(function() {
    if ( .width() > .height()){
        //it's a landscape
        $(this).addClass("landscape");
    } else if ( .width() < .height()){
        //it's a portrait
        $(this).addClass("portrait");
    } else {
        $(this).addClass("canttell");
    }
});

My problem is each img tag is outputting the exact same result and my test images are a good mixture of landscape and portrait so something isn't right.

Can anybody help?

`( .width() < .height()){` - ehh, `.width()` isn't prefaced with an element? — tymeJV, May 17 '14 at 18:30
You can't use `.width()` and `.height()` without context. The width and height of what element? — JJJ, May 17 '14 at 18:30
I don't think JS will know right off the sizes of the images, as it takes varying amounts of time to load the images. This post describes how to use the onload event. http://stackoverflow.com/questions/623172/how-to-get-image-size-height-width-using-javascript (not the accepted answer but the one with 356 upvotes) — Dave, May 17 '14 at 18:33

score 1 · Answer 1 · answered May 17 '14 at 18:32

I guess you forgot to mention the jQuery object infornt of .width() and .height()

$(".eventimage img").each(function() {
    if ( $(this).width() > $(this).height()){
        //it's a landscape
        $(this).addClass("landscape");
    } else if ( $(this).width() < $(this).height()){
        //it's a portrait
        $(this).addClass("portrait");
    } else {
        $(this).addClass("canttell");
    }
});

I assumed that you were trying to find width and height of the image. So added $(this) in front of them.

Dave · Accepted Answer · 2014-05-17T22:26:42.207

1

Instead of an each loop, I'd use the onload event to allow the images to download first:

$(".eventimage img").onload = function() {
  var width = this.width;
  var height = this.height;
      if ( width > height){
        //it's a landscape
        $(this).addClass("landscape");
    } else if ( width < height){
        //it's a portrait
        $(this).addClass("portrait");
    } else {
        $(this).addClass("canttell");
    }
}

EDIT:

In some circumstances my sample code might be too simplistic and it might be necessary to preload your images in JS. To use this method the images may need to be preloaded first and then get their sizes by doing something like this:

var GetImageSize = function( src, callback ) {
  this.src = src;
  this.callback = callback;
  this.image = new Image();
  this.requestImageSize()
};

Here is someone else's example of how this is done:

http://jsfiddle.net/peterbenoit/MWVLY/

edited May 17 '14 at 22:26

answered May 17 '14 at 18:45

Dave

4,949
6
50
73

Genius, thanks dave. I'm just a old fashioned print designer trying to expand my knowledge. JS is the next level im determined to learn so thanks for taking the time to answer my question and make my day that little bit better. – user3648221 May 17 '14 at 18:54
Really cool of you to give me a better solution Dave. Should make me a bit to think in a more rounded way next time I create a script. Cheers. – user3648221 May 17 '14 at 18:57
Thanks, I haven't tested it competely, so it might need some syntax adjustments. Hope it works. – Dave May 17 '14 at 19:00
Works perfectly from what i can see in the browser inspector :) – user3648221 May 17 '14 at 19:12
If do run into any trouble let me know. I don't want to leave any misinformation on the SO 'wiki'. Thanks! – Dave May 17 '14 at 19:19

score 0 · Answer 3 · answered May 17 '14 at 18:34

According to your logic, change your code to:

$(".eventimage img").each(function(index, element) {
    if ($(element).width() > $(element).height()){
        //it's a landscape
        $(element).addClass("landscape");
    } else if ( $(element).width() < $(element).height()){
        //it's a portrait
        $(element).addClass("portrait");
    } else {
        $(element).addClass("canttell");
    }
});

Please refer to the documentation of jquery.each() : http://api.jquery.com/jquery.each/

jquery loops using .each to detect width and height

3 Answers3