# Cleaning up a rotating image gallery

I have created a nice little image gallery for the web. I set out wanting 3 things. 1. The whole gallery had a basic level responsiveness to it. 2. The gallery would take images of any size, and display them without cropping. 3. The gallery would run automatically. If you look here(JSFiddle) I have done just that. The automatic slide show is just a small bit of jQuery.

setInterval(function () {
$(".image_background").last().animate({ left: '-=1000' }, 1000, 'swing', function() { var top_element=$(this).detach();
top_element.css({"left": 0});
\$(".image_gallery").prepend(top_element);
})}, 2000);


I am quite happy with it. It's the css bit that I am not so sure about.

.main_image_gallery{
display: inline-block;
position: absolute;
height: 50%;
width: 30%;
z-index: 2;
top: 30%;
left: 3%;
background-color: #4e9ba1;
box-sizing: border-box;
overflow: hidden;
}
.image_background{
background-color: #4e9ba1;
display: inline-block;
position: absolute;
height: 100%;
width: 100%;
}
.gallery_image{
margin: auto;
position: absolute;
top: 0; left: 0; bottom: 0; right: 0;
max-width: 100%;
max-height: 100%;
}


For reference, here's the HTML

<div class="main_image_gallery image_gallery">
<div class="image_background">
<img class="gallery_image" src="http://www.vectortemplates.com/raster/batman-logo-big.gif" ></img>
</div>
<div class="image_background">
<img class="gallery_image" src="http://www.vectortemplates.com/raster/batman-logo.gif" ></img>
</div>
<div class="image_background">
<img class="gallery_image" src="http://th09.deviantart.net/fs70/PRE/f/2011/290/8/e/batman_logo_circleless_by_machsabre-d4d6s56.png" ></img>
</div>
</div>


I feel like there is a simpler way to do it. Any thoughts on how I can clean it up, maybe improve the responsive aspects?

For the css part, I started from the updated fiddle @cimmanon provided. I basically agree with all the things he added. I also went ahead and turned your markup into a ul as he (or she) correctly suggested. I would like to provide my own insights as well:

• You are closing your img tags, however they can never contain any content so the closing tags are redundant (and perhaps even invalid). You can either make them self-closing (<img ... />) or just omit them in html5.

• You are using a lot of different classes in your markup. There is no real need for that; all it does is make your code longer and harder to read. I would go for selectors like .image-gallery img instead of .gallery_image. I admit that in theory this is slightly less performant, but I doubt you'll be able to see, or even measure, the difference. And it makes your markup a lot easier to maintain (less code when adding slides, and less chance of making typos in all those class names).

• I would definitely not animate the left property, but go for a transform: translate3d in stead. It is a lot better for performance as explained here.

• I would use css transitions, instead of the jQuery animate. It is again better for performance, and it I believe it makes your code a lot better structured. If you decide to change anything in your animation, you just have to update your css and you don't have to start digging into your javascript. I'm a big fan of only using Javascript for thing that can't be done through css.

I went ahead and applied my suggestions to the fiddle, which you can find here: http://jsfiddle.net/eL0vaL44/3/

• Self-closing the img tag is just optional in HTML5, not invalid. – cimmanon Aug 10 '14 at 22:57
• yeah but it future proofs the markup to be polyglot markup – albert Aug 11 '14 at 0:04
• @cimmanon, to be clear, I was referring to the </img> tag. I looked it up (w3.org/TR/html-markup/syntax.html#void-element) and the spec clearly says Void elements only have a start tag; end tags must not be specified for void elements. – Pevara Aug 11 '14 at 9:30
• Any chance you know a better way to position the gallery, other than with padding in 'before'? – EasilyBaffled Aug 12 '14 at 12:00

Your markup looks pretty good. You're using just enough elements to get the desired effect and no more. Semantically speaking, a list might be more appropriate here and would give you the same structure.

It's unclear why your .main_image_gallery element is absolutely positioned. If it's because you want to keep the child elements contained within, you should be using position: relative instead. If it's so you can have the container sized relative to the parent element (in this case, the body element), there's a different technique you can use:

https://stackoverflow.com/questions/12121090/responsively-change-div-size-keeping-aspect-ratio

.main_image_gallery{
width: 50%;
background-color: #4e9ba1;
box-sizing: border-box;
overflow: hidden;
position: relative;
}
.main_image_gallery:before {
content: '';
display: block;
}
.image_background{
background-color: #4e9ba1;
position: absolute;
top: 0;
height: 100%;
width: 100%;
}


http://jsfiddle.net/eL0vaL44/2/

You don't need to specify display: inline-block if you're absolute positioning an element. Absolutely positioning an element will cause it to shrink to its contents.

• The method you got from stackoverflow, forces the hight to be much taller than I would like. It makes the gallery taller than it is wide. – EasilyBaffled Aug 10 '14 at 21:25
• I chose an arbitrary value: with a padding of 150%, the height will always be 50% taller than it is wide. If you want it smaller, just reduce it to whatever is appropriate. – cimmanon Aug 10 '14 at 22:52
• But reducing the padding, will reduce the position height. – EasilyBaffled Aug 10 '14 at 23:16