CSS Photo Gallery

Cleaning up the code

[mkingsle]

Hello Everyone:

I have some code for a photo gallery below that I'd appreciate anyone looking at. I should note that it does work, but I am looking for any input on whether or not it could be condensed (cleaned up any further).

The html code is:

<div class="gallerycontent">
<br /><br />

<div class="column last" id="small">
<ul id="gallerynav">
<li><div class="thumbnail"><img src="/image/thumbs/th_image_one.jpg" width="60" height="60" alt="image one"/><span><img src="/image/big/image_one.jpg" alt="image one big" /></span></div>
</li>
<li><div class="thumbnail"><img src="/image/thumbs/th_image_two.jpg" width="60" height="60" alt="image one"/><span><img src="/image/big/image_two.jpg" alt="image one big" /></span></div>
</li>
<li><div class="thumbnail"><img src="/image/thumbs/th_image_three.jpg" width="60" height="60" alt="image one"/><span><img src="/image/big/image_three.jpg" alt="image one big" /></span></div>
</li>
<li><div class="thumbnail"><img src="/image/thumbs/th_image_four.jpg" width="60" height="60" alt="image one"/><span><img src="/image/big/image_four.jpg" alt="image one big" /></span></div>
</li>
<li><div class="thumbnail"><img src="/image/thumbs/th_image_five.jpg" width="60" height="60" alt="image one"/><span><img src="/image/big/image_five.jpg" alt="image one big" /></span></div>
</li>
<li><div class="thumbnail"><img src="/image/thumbs/th_image_six.jpg" width="60" height="60" alt="image one"/><span><img src="/image/big/image_six.jpg" alt="image one big" /></span></div>
</li>
</ul>

</div>
<img style="padding-top:15px; padding-left:30px;" src="/image/big/image_one.jpg" alt="image one"/>
<!-- column first -->
</div>

The CSS code is as follows:

/* My Photo Gallery */

div.gallerycontent{
width:718px;
overflow: auto;
min-height: 100%;
height: 100%;
padding-bottom: 60px;
background:url(/image/photogalleryback.jpg) no-repeat;
margin-left:10px;
}

.column.last {
float:left;
width:150px;
}

#small {
width: 150px;
padding: 0;
}

#small ul#gallerynav {
width: 100px;
margin-top:-10px;
}

#small ul#gallerynav li {
width: 60px;
margin-top:10px;
list-style-type:none;
}

#small ul#gallerynav li img {
border: 2px solid #cbc5b2;
}

#small ul#gallerynav li img:hover {
border: 2px solid #7aaaec;
}

#small .thumbnail:hover{
background-color: transparent;
}

#small .thumbnail span{
position: absolute;
padding: 5px;
visibility: hidden;
color: black;
text-decoration: none;
}

#small .thumbnail span img{
border:none!important;
padding: 2px;
}

#small .thumbnail:hover span{
position:absolute;
visibility: visible;
top:601px;
left: 389px;
}

Again, I appreciate any input about this. I have a feeling that I have unnecessary code in there, but I am still and always will be a newby and don't know any better.

Thanks in advance.

Michael

[vincevincevince]

Why do you have both .column and .last, but only use each together?

[SuzyUK]

yes it looks like it can be cleaned and condensed, but I'm hesitant to start telling you about removing classes etc.. as the words "column and last" suggest to me there's more than one #gallerynav div per page

anyhow,if there's only one of these here's the HTML template I would suggest

<div id="gallery">
<div id="gallerynav">
<ul>
<li><img src="/image/thumbs/th_image_one.jpg" alt="image one"/><span><img src="/image/big/image_one.jpg" alt="image one big" /></span></li>
<li><img src="/image/thumbs/th_image_two.jpg" alt="image two"/><span><img src="/image/big/image_two.jpg" alt="image two big" /></span></li>
<li><img src="/image/thumbs/th_image_three.jpg" alt="image three"/><span><img src="/image/big/image_three.jpg" alt="image three big" /></span></li>
</ul>
</div>

<div id="slideshow">
<img src="/image/big/image_one.jpg" alt="image one"/>
</div>
</div>

There is now only 3 x ID's & no classes but everything inside #gallery div can be targeted using just these three (more semantic?) names/ID's

How best to optimise the CSS will depend on if/how you reset the HTML, but the main thing is to try to use a consistent method of targeting the inner elements... e.g. in your code:

.column.last {
float:left;
width:150px;
}

#small {
width: 150px;
padding: 0;
}

This is targeting exactly the same div, though that's not immediately obvious unless you have the HTML to hand is it ;) - but it does makes the width declaration on both, unnecessary, it only needs to be on one. As an aside the

.column.last

is an advanced selector, it's targeting only an element with both of those class names, which is OK as far as your existing code goes but that advanced selector is not supported by IE less than 7 so the float part of it won't be working.

Anyway what I mean by consistent is for example to always to use #small (using your code as a base) in the specificity of the selectors - not only is it obvious to you that this whole section targets that #small div, but it will help avoid any specificity conflicts that may arise. ID's are more specific than classes so mixing the two can override the cascade.. there are times when this is useful, but that's another story..

note I changed .gallerycontainer & #small to #gallery and #gallerynav as it seemed to make more sense as to what each div was actually doing.

CSS to go with the HTML I just posted, it seems longer because of the comments and test colors, and I've also removed a lot of the HTML attributes to CSS so the CSS is maybe cleaner rather than condensed, though I think stripped of the comments & my added extra coloring it's possibly condensed too.

html, body {padding: 0; margin: 0; height: 100%;}

#gallery {
width: 718px;
height: 100%;
min-height: 100%;
background: #ffc; /* test color */
position: relative; /* so AP spans take their postion from here */
}

#gallery img {display: block;}

#gallerynav {
float:left;
width: 150px;
margin: 40px 0 0 0; /* top gap instead of the <br's> */
background: #dad; /* purple */
}

#gallerynav ul {
width: 100px;
margin: 0;
padding: 1px 0; /* contain collapsing margins from li's */
list-style-type:none;
background: #abc; /* blue for list */
}

#gallerynav li {
margin: 10px 0; /* space out the li's which hold the images */
}

#gallerynav li img {
border: 2px solid #cbc5b2;
width: 60px; /* width and height can be removed from HTML to here if they are all the same */
height: 60px;
margin: 0 auto; /* to center little image in the li */
}

#gallerynav li img:hover {
border: 2px solid #7aaaec;
}

/* the following combines the fact that all the big images no matter their state or where they are have the same properties - this may not be the actual case, but the three selectors are how to target them */
#gallerynav li span img,
#gallerynav li span img:hover,
#slideshow img {
border: 0;
margin: 0;
width: 350px; /* again only put the width and height in here if the large images have .. */
height: 350px; /* .. the same dimensions, otherwise put dimensions in the HTML */
}

#gallerynav li span{
position: absolute;
top: 40px;
left: 150px; /* same as width of #slideshow div shown below */
visibility: hidden;
}

#gallerynav li:hover span {
/* no need to redeclare the properties again just the visibility overrule */
visibility: visible;
}

#slideshow {
background: #ffe;
margin-left: 150px; /* width of left #gallerynav or more */
padding-top: 40px; /* same as top position of absolute span if you want the pop-up images over the top */
}

#slideshow img {
/* this rule is combined with the span image one above but would be best here if the properties are actually different */
}

hope that makes sense, basically reading down the CSS you're cascading down as if you were going through the HTML, making a clearer order than before and the selectors should speak for themselves.

-Suzy

[mkingsle]

Thank you so much for the reply and your input. I am going to test this today and I'll post my results. It looks good though.

Once again, I really appreciate the help.

Michael