3
\$\begingroup\$

I set out to make a simple dynamic resize method for images. Obviously, it's better to serve images that are pre-sized to avoid unnecessary server loads. But it was fun to make for a novice/beginner jQuery project and not really meant for a production project. For many of you, I'm sure it'll be rudimentary at best, but that's why I'm posting it here, to get some advice on ways to improve it.

The goal,

  1. Add wrappers dynamically (using the wrapInner method for content structure inside the collection of elements with class topic), add wrapper around images to constrain the content image sizes.
  2. dynamic width & height attributes on images - onLoad & onResize
  3. Dynamic query string on img.src (URL adds ?width=###&height=###) onLoad & resize (for demo purposes I'm using "placehold.it/###x###") this is only needed for content images in the DOM.
  4. Real width & height of image set to parent (figure) as max-width/height onLoad
  5. Debounce/throttle resize events to avoid excess events begin fired. (used a method displayed on css-tricks.com and can be found here)

The CSS and HTML is more to make the demo look nice but if you have some input for that as well, by all means. Like I said, I'm certainly not a jQuery developer, though I'm trying to learn as quickly as possible to augment my skill set. So if you have any advice or input on areas that can improve, I'd greatly appreciate it.

Fiddle: DEMO

$(function() {
    // demo fun
    $('.topic').each(function() {
        var $this = $(this),
            ifMedia = $this.find('.media');
        $this.wrapInner('<section class="content"/>');

        if (ifMedia.length) {
            $this.addClass('media-support');
        }
        if ($this.hasClass('media-support')) {
            $('.media-support:odd').addClass('inverse');
        }
    });
});

var sizeTimer;
$(window).on('load', function() {
    var img = new Image();
    $('img').each(function() {
        var $this = $(this),
            image = new Image(),
            _imgWidth = $(this).width(),
            _imgHeight = $(this).height(),
            _naturalWidth = this.naturalWidth,
            _naturalHeight = this.naturalHeight;
        image.src = $(this).attr("src");
        $this.attr({
            'width': +_imgWidth,
            'height': +_imgHeight,
            'src': '//placehold.it/' + _imgWidth + 'x' + _imgHeight
        }).wrap('<figure class="image" />').closest('.image').css({
            'max-width': _naturalWidth,
            'max-height': _naturalHeight
        });
    });
}).on('resize', function() {
    clearTimeout(sizeTimer);
    sizeTimer = setTimeout(function() {
        $('img').each(function() {
            var $this = $(this),
                _imgWidth = $(this).width(),
                _imgHeight = $(this).height();
            $this.attr({
                'width': _imgWidth,
                'height': _imgHeight,
                'src': '//placehold.it/' + _imgWidth + 'x' + _imgHeight
            });
        });
    }, 250);
}); 
\$\endgroup\$
3
\$\begingroup\$

I know that this code was posted more than three years ago and you have likely learned many things since then but I'd rather not see it be a zombie any longer... so here we go:

In general the code looks okay. There aren't too many repeated blocks. There are a few suggestions I have to tidy up the code.

  • Cache DOM references - instead of querying for collections like $('img') multiple times, do that once the DOM is loaded, store the collection in a variable and then reference the variable instead of re-querying.

    var images
    $(window).load(function() {
    
        images = $('img');
        images.each(function () {
    

    and later in the resize callback:

    .on('resize', function () {
    
        clearTimeout(sizeTimer);
    
        sizeTimer = setTimeout(function () {
    
            images.each(function () {
    
  • unused variable: img this variable doesn't appear to be used anywhere after it is created: var img = new Image();. Perhaps that is left from a side venture in the code...

  • Don't repeat code this may only be achievable with advanced function manipulation so I wouldn't expect you to do this in one of your first projects but the code that exists in the callback function on the resize event executed for each image could be abstracted out to a separate function:

    function setAttributesOnImage() {
    
        var $this = $(this),
            _imgWidth = $(this).width(),
            _imgHeight = $(this).height();
    
        $this.attr({
            'width' : _imgWidth,
            'height': _imgHeight,
            'src': '//placehold.it/' + _imgWidth + 'x' + _imgHeight
        });
    
    }
    

    Then that function can be used in the load callback, utilizing Function.prototype.apply():

    image.src = $(this).attr("src");
    setAttributesOnImage.apply(this);
    $this.wrap('<figure class="image" />').closest('.image').css({
    
        'max-width': _naturalWidth,
        'max-height': _naturalHeight
    
    });
    

    This means _imgWidth and _imgHeight don't have to be declared again in the load handler, since the call to setAttributesOnImage() handles that.

    And also in the resize callback:

      .on('resize', function () {
    
        clearTimeout(sizeTimer);
    
        sizeTimer = setTimeout(function () {
    
            images.each(setAttributesOnImage)
        }, 250);
    

    Also, that last timeout callback could be simplified, using Function.prototype.bind() to create a Partially applied function:

    sizeTimer = setTimeout(images.each.bind(images, setAttributesOnImage), 250);
    

Snippet

Expand the snippet below to see simplified code.

/* Goal:

 1. add wrapper around .topic contents (wrapInner) onDocReady
 2. add wrapper around images (wrap) onDocReady
 
 3. dynamic width & height attributes on images (attr) onLoad & resize
 4. dynamic queryString on img.src (URL adds ?width=###&height=###) onLoad & resize (for demo purposes I'm using "placehold.it") this is only needed for content images in the DOM.
 
 5. real width & height of image set to parent as max-width/height onLoad
 6. debounce/throttle resize events

*/

$(function () {
    
    $('.topic').each(function () {
        
        var $this = $(this),
            ifMedia = $this.find('.media');
        
        $this.wrapInner('<section class="content"/>');
        
        if (ifMedia.length) {
            $this.addClass('media-support');
        }
        if ($this.hasClass('media-support')){
            $('.media-support:odd').addClass('inverse');
        }
    });
        
});
function setAttributesOnImage() {

  var $this = $(this),
      _imgWidth = $(this).width(),
      _imgHeight = $(this).height();

  $this.attr({
    'width' : _imgWidth,
    'height': _imgHeight,
    'src': '//placehold.it/' + _imgWidth + 'x' + _imgHeight
  });

    }
var sizeTimer;
var images;
$(window).on('load', function () {
		images = $('img');
    
    images.each(function () {
        
        var $this = $(this),
    		image = new Image(),
            _naturalWidth = this.naturalWidth,
            _naturalHeight = this.naturalHeight;
        
        image.src = $(this).attr("src");
        setAttributesOnImage.apply(this);
        $this.wrap('<figure class="image" />').closest('.image').css({
            
            'max-width': _naturalWidth,
            'max-height': _naturalHeight
            
        });

    });
    
}).on('resize', function () {
    
    clearTimeout(sizeTimer);
    
    sizeTimer = setTimeout(images.each.bind(images, setAttributesOnImage), 250);

});
* {
    box-sizing:border-box;
}
html{background-color:#ecf0f1;}
body{max-width:1200px;color:#34495e;}
.content{max-width:800px;}
body,.content{margin:0 auto;}
h2{font-size:2rem;margin-bottom:15px;line-height:1;}
p{font-family:sans-serif;line-height:1.55}

img {
    display:block;
    max-width:100%;
    height:auto;
}
.topic:nth-of-type(odd) {
    background-color:#cde
}
.topic:nth-of-type(even) {
    background-color:white
}
.media{
    position:absolute;
    transform:translateX(-100%)
}
.content{padding:35px 25px}
@media (min-width:540px){
    .content {
        height:350px;
        padding:0;
    }
    .content:after {
        clear:both;
        content:'';
        display:table;
    }
    .image {
        background-position:center center;
        background-size:cover;
        background-repeat:no-repeat;
        width:100%;
        padding-bottom:30%;
    }
    .copy{
        padding-left:25px;
        padding-right:25px;
        position:relative;
        top:50%;
        transform:translateY(-50%);
    }
    .media-support .copy {
        width:50%;
        float:left;
    }
    .media-support.inverse .copy {float:right}

    .media-support .media {
        width:50%;
        transform:translateX(0);
        float:right;
        height:100%;
        position:relative;
    }
    .media-support.inverse .media {float:left}
    figure {
        margin:0;
    }
    .media .image, .media img, .media div {
        position:absolute;
        bottom:0;
    }
    .media-support:not(.inverse) .image,
    .media-support:not(.inverse) img{
        left:0
    }
    .media-support.inverse .image,
    .media-support.inverse img{
        right:0;
    }
    .media .undock {
        position:absolute;
        bottom:50%;
        transform:translateY(50%);
    }
    .media-support .media .image > img {
        width:100%;
        height:auto;
    }
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<article class="topic">
    <div class="copy">
         <h2>This is a section heading</h2>

        <p>Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Donec odio. Quisque volutpat mattis eros. Nullam malesuada erat ut turpis. Suspendisse urna nibh, viverra non, semper suscipit, posuere a, pede.</p>
    </div>
    <aside class="media">
        <img src="//placehold.it/350x125" alt>
    </aside>
</article>
<article class="topic">
    <div class="copy">
         <h2>This is a section heading</h2>

        <p>Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Donec odio. Quisque volutpat mattis eros. Nullam malesuada erat ut turpis. Suspendisse urna nibh, viverra non, semper suscipit, posuere a, pede.</p>
    </div>
</article>
<article class="topic">
    <div class="copy">
         <h2>This is a section heading</h2>

        <p>Donec nec justo eget felis facilisis fermentum. Aliquam porttitor mauris sit amet orci. Aenean dignissim pellentesque felis.</p>
        <p>Morbi in sem quis dui placerat ornare. Pellentesque odio nisi, euismod in, pharetra a, ultricies in, diam. Sed arcu. Cras consequat.</p>
    </div>
    <aside class="media">
        <div class="image undock" style="background-image:url(//placehold.it/350x125);max-width:350px;max-height:125px">&nbsp;</div>
    </aside>
</article>

\$\endgroup\$
  • \$\begingroup\$ Woh, talk about a blast from the past ha. I actually don't even use jQuery any longer and either use vanilla or typescript. That said, I like the improvements you've made, excellent suggestions. Thanks! I actually do love to see work from ages ago and cringe a little, reminds me of how far I've come. \$\endgroup\$ – darcher Dec 21 '18 at 17:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.