addClass on set of images based on dimensions

So I’ve got the jquery-ui slider working on adjusting the size of my thumbnails, BUT the problem is if the image is portrait, it overflows out of the container…*jquery to the rescue, right? Not quite.

Below is the js and basic css that i’m using. Shouldn’t this be applying a different class on each img element based on the height/width values? They’re all getting the same class.

Am I needing to do some sort of loop function for this instead?


	$("#thumbnail_size").slider({
		range: "min",
		min: 125,
		max: 250,
		value: 187,
		slide: function (event, ui) {
				var image_width   = $(".project_file_container").find('img').width();
				var image_height  = $(".project_file_container").find('img').height();
				$(".project_file_container").css("width", ui.value + "px");
				$(".project_file_container").css("height", ui.value + "px");
				if (image_width > image_height) {
					$(".project_file_container").find('img').addClass('wide');
				} else {
					$(".project_file_container").find('img').addClass('tall');
				}
			}
	});



.project_file_container img.wide {width:100%;height:auto;}
.project_file_container img.tall {width:auto;height:100%;}

No, don’t do that! PNGs are enormous anyway (stick with JPEG for photos), and then you run into problems with IE.

There was a typo in what I posted… it should be if ($(this).width() > $(this).height()) - I forgot the () after height.

If this still doesn’t work, then you should learn to debug JS by yourself. For example, with Firebug, you can see what width() and height() are returning:

$("#thumbnail_size").slider({
        range: "min",
        min: 125,
        max: 250,
        value: 187,
        slide: function (event, ui) {
                var container = $(".project_file_container"),
                     images = container.find('img'),
                     image_width   = images.width(),
                     image_height  = images.height();
                container.css({width: ui.value + "px", height: ui.value + "px"});
                images.each(function() {
                  console.log($(this).width(), $(this).height()); // debug
                  if ($(this).width() > $(this).height) $(this).addClass('wide');
                  else $(this).addClass('tall');
                });
            }
    });

Based on what you get back, you can progress from there.

No, that’s not right. container.image_width doesn’t mean anything. That’s saying that the container object has an “image_width” property, which is not true. Also you already have the set of images in the “images” variable.

You have to learn to think in terms of “is this returning a jQuery object?” and life will become a lot easier and a lot of how jQuery works will begin to make sense and fit together. A jQuery object is just a collection of matched DOM elements (matched by whatever set of rules you’ve provided).

This is what you want:

$("#thumbnail_size").slider({
        range: "min",
        min: 125,
        max: 250,
        value: 187,
        slide: function (event, ui) {
                var container = $(".project_file_container"),
                     images = container.find('img'),
                     image_width   = images.width(),
                     image_height  = images.height();
                container.css({width: ui.value + "px", height: ui.value + "px"});
                images.each(function() {
                  if ($(this).width() > $(this).height) $(this).addClass('wide');
                  else $(this).addClass('tall');
                });
            }
    });

I’m still not sure you’re going about this the right way though. Do you have a live version of it to play with anywhere?

It’s working!

Thanks a LOT for the help. I know, I know… i have a long way to go as far as general use for js. But I’ll get there… eventually.

Before your reply, I had adapted this
http://thejudens.com/eric/2009/07/jquery-image-resize/

your solution is much more concise though.

Once again… thanks.

Here’s the basic html


<div class="listing_box whole">
	<h2>Project Files</h2>
	<div class="content" id="project_files">
		<div class="project_file_container"><!-- image container, repeated x times for file in project -->
			<span class="file_preview_container">
				<a href="/project/checkout_file/16" class="document jpg"><label>jpg</label></a>
				<img alt="Thumb_1190263_11772578" class="file_preview" id="project_file_16" src="/images/project_thumbnails/1/thumb_1190263_11772578.jpg?1276189812" />
				<span class="file_title">
					created by: first name last name<br />
					on: 06/10/10 at 12:10PM<br />
					filesize: 1.7 MB		
				</span>
			</span>
			<a href="/project/download_file/16">a test file</a>
		</div>
	</div>
	<br class="clear" />
</div>

And here’s how it would look

So, right now… i’ve got the slider resizing the width of the .project_file_container div, with the image taking on 100% of it’s width. That’s great for landscape images, but it’s the mixing of the portrait images that sucks. I’m creating the thumbnails with imageMagick with a max res of 250x250 while retaining aspect ratio. The portrait thumbs go beyond their intended resolution when they go to a 250px width.

I’ll try your suggestion and see how that works, thanks.

unfortunately no… this whole thing is a project i’ve made up to learn ruby/rails/jquery and only exists on my local machine right now.

With your update, they’re all getting ‘tall’ applied. Which obviously still isn’t working.

I don’t think creating pngs with transparency is the ‘cool’ answer here, but I’m wondering if that’s the smarter solution. All images would be square and thus easily scalable.

The width() and height() functions measure the dimensions of the first element in the set. The set is what is before the .width() or .height(). So they are all getting the same class because the comparison is being done between the dimensions of the first image in the collection only.

Yes, you need to loop through the images each time and then apply your styles (use each()). However, I’m not sure you need to do this at all. Perhaps it could all be done completely automatically using CSS. I’d have to see a live version or at least some HTML to go with this code and a picture/good description of what it should look like.

Your jQuery needs some simple improvements. In that short piece of code, you’re forcing jQuery to find all elements with the class “.project_file_container” six times, and make a list of images within four times. This repetitive procedure can be solved simply by storing the results in a variable:

$("#thumbnail_size").slider({
        range: "min",
        min: 125,
        max: 250,
        value: 187,
        slide: function (event, ui) {
                var container = $(".project_file_container"),
                     images = container.find('img'),
                     image_width   = images.width(),
                     image_height  = images.height();
                container.css({width: ui.value + "px", height: ui.value + "px"});
                // now loop through images and add/remove classes
            }
    });

this is what should go after the ‘loop through images’ comment right?


$(".project_file_container img").each(
     if (container.image_width > container.image_height) {
          $(this).addClass('wide');
     } else {
          $(this).addClass('tall');
     }
);

It’s breaking the entire page.