2

Hello,

I am trying to add classes to each image depending to its proportions (landscape/portrait/square). This code works in JSfiddle but not on the website (locally or on server):

$(document).ready(function()
{
    $('img').addClass('wide');
    $.each($('img'), function()
    {
        var image = $(this);

        if (image.height() > image.width()) {
            image.removeClass('wide');
            image.addClass('tall');
        } else if (image.height == image.width()) {
            image.removeClass('wide');
            image.addClass('square');
        }
    });
});​

Here is my JSfiddle: http://jsfiddle.net/LYxv6/ .

Reporter
  • 3,897
  • 5
  • 33
  • 47
user1645438
  • 23
  • 4

6 Answers6

5

The reason your code works on jsfiddle is because the site has (by default) wrapped your code in an onload handler. This handler is called once all the images have loaded, unlike $(document).ready() which only waits until the DOM structure is complete.

The function to change the classes is trivial if you take advantage of the version of addClass that takes a function parameter instead of a string:

$(window).on('load', function() {
    $('img').addClass(function() {
        if (this.height === this.width) {
            return 'square';
        } else if (this.height > this.width) {
            return 'tall';
        } else {
            return 'wide';
        }
    });
});

In the callback this is the <img> element itself, and you can directly access its .width and .height properties without having to use jQuery.

This should be far more efficient than repeatedly calling jQuery functions inside a .each() loop.

Alnitak
  • 334,560
  • 70
  • 407
  • 495
1

You should run this script on

$(window).load()

...otherwise(on ready) it may happen that the images are not loaded yet and the size is still unknown when you access them.

You also forgot some parentheses here:

else if (image.height == image.width()) {

should be

else if (image.height() == image.width()) {
Dr.Molle
  • 116,463
  • 16
  • 195
  • 201
1

Changed to $('img').each and missing paranthesis at image.height() (this is a function)

$(window).load(function() {

$('img').addClass('wide');

$('img').each(function() {
    var image = $(this);

    if (image.height() > image.width()) {
        image.removeClass('wide');
        image.addClass('tall');
    }
    else if (image.height() == image.width()) {
        image.removeClass('wide');
        image.addClass('square');
    }

});

});
jtheman
  • 7,421
  • 3
  • 28
  • 39
0

Try to use variables in if else if statements:

Here is jsFiddle.

    var biggerVal = image.height() > image.width();
    var equalVal = image.height() == image.width();

    if ( biggerVal ) {
        image.removeClass('wide');
        image.addClass('tall');
    } else if ( equalVal ) {
        image.removeClass('wide');
        image.addClass('square');
    }

Same problem on this question: How to build simple sticky navigation with jQuery?

Community
  • 1
  • 1
Barlas Apaydin
  • 7,233
  • 11
  • 55
  • 86
  • there's not _much_ point in this case declaring a variable that only gets used once... In fact, you should be caching `.height()` and `.width()`, since those are both making function calls. – Alnitak Sep 04 '12 at 08:15
  • @Alnitak When i saw "*This code works in JSfiddle but not on the website (locally or on server):*" on the question, i suspect from not declaring variable because in long jQuery codes, this happened me before. Working on jsFiddle but same thing not working on site. – Barlas Apaydin Sep 04 '12 at 08:32
  • 1
    The main reason code tends to work on jsfiddle but not elsewhere is because jsfiddle _defaults_ to putting an 'onload' wrapper around your code, as is the case with the OP's fiddle. Moving stuff into variables will have _absolutely nothing to do with it_! – Alnitak Sep 04 '12 at 08:35
  • @Alnitak Yeah you are right on this example, but it is always better to use variables in if else declarations in case not to create a jQuery conflict. – Barlas Apaydin Sep 04 '12 at 10:25
  • I have no idea what you think you're talking about. – Alnitak Sep 04 '12 at 10:28
  • @Alnitak Read the first comment please: http://stackoverflow.com/questions/11872381/scrolltop-and-then-div-fixed/11872508#11872508 – Barlas Apaydin Sep 04 '12 at 10:35
  • I can assure you that whatever fixed that code, putting that expression inside a variable was not it. – Alnitak Sep 04 '12 at 10:46
  • @Alnitak you are missing the point: **This occurs when it is inside a long jQuery function or used with some plugins. What i am talking about is not occurs because of document.ready or anythingelse because when defined var in if declaration, it starts to work.** It is like a conflict and this has really tiny chance to happen. I am sorry i can't prove it, but it happened me once. **Thanks for your interest.** – Barlas Apaydin Sep 04 '12 at 12:14
0

On your server on local system, make an .html file.

Change it to document load, put in missing braces for one of the width parts - and make sure the .html file looks like the below:

    <!DOCTYPE html>
    <html>
    <head>
    <meta charset=utf-8 />
    <title></title>
    <script>
       $(document).load(function() {

    $('img').addClass('wide');

    $.each($('img'), function() {
        var image = $(this);

        if (image.height() > image.width()) {
            image.removeClass('wide');
            image.addClass('tall');
        }
        else if (image.height() == image.width()) {
            image.removeClass('wide');
            image.addClass('square');
        }

    });

});​
    </script>
    <style>
    img{
height:250px;
}
.wide{
border:10px solid #aed;

}
.tall{
border:10px dashed #dea;

}
.square{
border:10px dotted #ade;

}​
    </style>
    </head>
    <body>
    <img src="http://24.media.tumblr.com/tumblr_m9t0dwyu0P1qjfsubo1_500.jpg"/>
    <img src="http://24.media.tumblr.com/tumblr_m9iqv8oQl01qcpwsso1_500.jpg "/>
    <img src="http://25.media.tumblr.com/tumblr_m9n4kyf8aA1r1thfzo5_1280.jpg"/>  
    </body>
    </html>
​

You may replace <style></style> and <script></script> with <link rel="stylesheet" type="text/css" href="yourstyle.css"/> and <script type ="text/javascript" src="jquerywhatever.js"></script><script type ="text/javascript" src="YOURSCRIPT.js"></script>

sajawikio
  • 1,516
  • 7
  • 12
0

I would do something like this:

(function ($) {
    $.fn.proportions = function (options) {
        options = $.extend(options, {
            wideClass : 'wide',
            tallClass : 'tall',
            squareClass : 'square'
        });
        addProportions = function () {
            img = $(this);
            var ratio = img.width() / img.height();
            if (ratio > 1) {
                return img.addClass(options.wideClass);
            }
            if (ratio === 1) {
                return img.addClass(options.squareClass);
            }
            return img.addClass(options.tallClass);
        };
        return this.each(function () {
            if ($(this).width() !== undefined) {
                addProportions.call(this);
            } else {
                $(this).on('load', addProportions);
            }
        });
    };
}(jQuery));

jQuery(function ($) {
    jQuery('img').proportions();
});​

Putting it in a plugin makes it re-usable. And instead of using window.load as jsfiddle is doing, I've attached the function to each image's load event, so that the class is added immediately.

Nathan MacInnes
  • 11,033
  • 4
  • 35
  • 50