Add attribute based on image dimensions

Question

I'm working for a client who's using a third-party platform which only allows me to override CSS and partially edit the HTML templates. I can also make changes by adding Javascript, and in this instance I need to append classes and IDs based on the dimensions of thumbnail images, represented as images within list-items. I've started a fiddle, replacing the images with a div.

http://jsfiddle.net/dbudell/SYTsP/4/

Here's the HTML:

<ul>
  <li><div class="item item1"></div></li>
  <li><div class="item item2"></div></li>
</ul>

And the CSS. As you can see, I've set .item1 to "portrait" dimensions and .item2 to "landscape dimensions. I want to append to IDs based on these dimensions:

li {
   display:inline-block;
   margin:5px;
}

.item1, .item2 {
  border:1px solid #333;
}

.item1 {
   width:100px;
   height:200px;
}

.item2 {
    width:200px;
    height:100px;
}

#fill-portrait {
  background:#333;
}

#fill-landscape {
  background:#888;
}

And here's the JQuery code, which seems to be syntactically correct but is yielding no results. Not sure what the problem is.

var itemWidth = $('.item').width();
var itemHeight = $('.item').height();

if (itemWidth > itemheight) {
  $('.item', this).addAttr('id','fill-landscape');
} else {
  $('.item', this).addAttr('id','fill-portrait');
}

Again, the link to the fiddle is http://jsfiddle.net/dbudell/SYTsP/4/.

There's a typo in the source code: `itemheight`=>`itemHeight`. After you fix that, see Joeltine's answer. — John Dvorak, Jan 11 '13 at 23:07
Why do you want to append the IDs when you already have the classes? — John Dvorak, Jan 11 '13 at 23:11
Are you planning to apply `#fill-landscape` and `#fill-portrait` to multiple elements? — Alexander, Jan 11 '13 at 23:14
@Alexander: +1 Very good point, I have noticed that possibility after posting my answer too and have added that as a note to the end of it to ensure OP is aware of the issues duplicate id values will cause and that using classes instead would be better if that is the case as well as a DEMO. — Nope, Jan 11 '13 at 23:28

Nope · Answer 1 · 2013-01-11T23:33:29.020

There is several things that don't look right:

typo in your current code itemheight should be itemHeight
addAttr("id, "value") should be attr("id", "value")
this in your selector $("li", this) is refering to the window which cannot be used in a selector like this as $("li", this).length = 0
you are only accessing the first li you come across and not all li elements in the ul

DEMO - Combining all of the above

I used your existing HTML and CSS in the above DEMO and the following script:

var $listItems = $("ul>li");

$listItems.each(function () {
  var $item = $(this);
  var itemWidth = $item.width();
  var itemHeight = $item.height();

  if (itemWidth > itemHeight) {
    $item.attr('id', 'fill-landscape');
  } else {
    $item.attr('id', 'fill-portrait');
  }
});

Side-note on id vs class
If you are planning on having several of those elements using the same id values you might want to switch those to classes instead, i.e: .addClass("fill-landscape") instead of attr("id", "fill-landscape"). id values have to be unique. It would be invalid HTML and jQuery selectors also only match the first matching id and won't return a collection. Off course this would mean you have to update your CSS as well from #fill-landscape to .fill-landscape.

DEMO - Using classes instead of ids just in-case

The above DEMO used the following changed CSS (#fill-x is now .fill-x):

.fill-portrait {
  background:#333;
}
.fill-landscape {
  background:#888;
}

Changed Script (using .addClass() instead of .attr()):

var $listItems = $("ul>li");

$listItems.each(function () {
  var $item = $(this);
  var itemWidth = $item.width();
  var itemHeight = $item.height();

  if (itemWidth > itemHeight) {
    $item.addClass('fill-landscape');
  } else {
    $item.addClass('fill-portrait');
  }
});

joeltine · Answer 2 · 2013-01-11T23:17:07.560

0

You need to select the individual elements you're trying to get the dimensions of. Otherwise, you're $('li') selector is selecting every "li" element on the page. For example:

$('.item1').height();

Then you can add an id as follows:

$('.item1').attr('id', 'blah');

An explicit example http://jsfiddle.net/SYTsP/7/:

var $item1 = $('.item1');
var $item2 = $('.item2');

setId($item1);
setId($item2);

function setId($item){
  var itemWidth = $item.width();
  var itemHeight = $item.height();

  if (itemWidth > itemHeight) {
    $item.parent('li').attr('id','fill-landscape');
  } else {
    $item.parent('li').attr('id','fill-portrait');
  }
}

Now with more iteration!!

http://jsfiddle.net/SYTsP/9/

edited Jan 11 '13 at 23:17

answered Jan 11 '13 at 23:03

joeltine

1,610
17
23

hm I added a general class "item" and tried to add that, however it doesn't seem to be yielding positive results :( thanks though – Daniel Bogre Udell Jan 11 '13 at 23:07
The code sample should show how to iterate over the matched elements. Yours only attaches an ID to the elements of a specific class. – John Dvorak Jan 11 '13 at 23:10
Once again, this doesn't iterate over the elements (as you claim is neccessary). This only executes the same code over two distinct classes (which _still_ may not be one-element). – John Dvorak Jan 11 '13 at 23:13

Add attribute based on image dimensions

2 Answers2