HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Responsive image code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
responsivecodeimage

Problem

I am a newbie jQuery developer and i have created a responsive image solution where the HTML custom data attributes are used for loading images according to size like desktop, laptop, iPad etc...

Here is a live demo of the project.

I hope you like the project. Please tell me how to improve this.

```
// Mezzaraine Beta Version

(function ($) {
jQuery.fn.extend({

mezzaraine: function () {

return this.each(function () {
// Optimisation: store the references outside the event handler:
var $img = $(this); //Got this is a variable that hold the img id or class tag.
var $window = $(window),
currentWindowWidth, // width of current Window
flagIs,
flagWas = '';
function res() {
currentWindowWidth = $window.width();

//set flag string .
if (currentWindowWidth = 641 && currentWindowWidth = 769 && currentWindowWidth = 1026 && currentWindowWidth = 1500 && currentWindowWidth <= 2800) {
flagIs = "large-desktop";
}

if (flagIs !== flagWas) {

switch (flagIs) {
case 'mobileLowEnd':
$('img').each(function () {

if (!($(this).attr('data-mobLow') === undefined)) {
$img.attr("src", $img.attr("data-mobLow"));
}

// Finally Without A Data Attribute Set the Original to Src//
else {
$img.attr("src", $img.attr("src"));

}
});
break;

case 'mobileHighEnd':
$('img').

Solution

There's a lot of duplication in this code.

Instead of this:

if (currentWindowWidth = 641 && currentWindowWidth = 769 && currentWindowWidth <= 1025) {
    flagIs = "tablet";
}


This is the same, and if someday you'll want to adjust the thresholds, it will be a lot easier:

if (currentWindowWidth <= 640) {
    flagIs = "mobileLowEnd";
} else if (currentWindowWidth <= 768) {
    flagIs = "mobileHighEnd";
} else if (currentWindowWidth <= 1025) {
    flagIs = "tablet";
}


In fact, it would be best to make a list of width-class pairs, and use a loop to find the first match instead of repetitive else-ifs.

Here's another example of duplicated logic:

if (!($(this).attr('data-tab') === undefined)) {
    $img.attr("src", $img.attr("data-tab"));
}

else if (!($(this).attr('data-mobHigh') === undefined)) {
    $img.attr("src", $img.attr("data-mobHigh"));
}

else if (!($(this).attr('data-mobLow') === undefined)) {
    $img.attr("src", $img.attr("data-mobLow"));
}


The checking and setting of attributes can be simplified,
using a list and a loop,
something like this:

var names = ['data-tab', 'data-mobHigh', 'data-mobLow'];
for (var i in names) {
    var name = names[i];
    if (!($(this).attr(name) === undefined)) {
        $img.attr("src", $img.attr(name));
        break;
    }
}


As a next step, you could turn this into a function and reuse in all the mobile, tablet, laptop, and other conditional branches with a different names array parameter.

I don't know if I'm missing something but this looks utterly pointless:

$img.attr("src", $img.attr("src"));


Finally, although I like to have a comfortable amount whitespace in code,
this code uses a bit too much vertical spacing even for my taste.

Code Snippets

if (currentWindowWidth <= 640) {
    flagIs = "mobileLowEnd";
}

else if (currentWindowWidth >= 641 && currentWindowWidth <= 768) {
    flagIs = "mobileHighEnd";
}

else if (currentWindowWidth >= 769 && currentWindowWidth <= 1025) {
    flagIs = "tablet";
}
if (currentWindowWidth <= 640) {
    flagIs = "mobileLowEnd";
} else if (currentWindowWidth <= 768) {
    flagIs = "mobileHighEnd";
} else if (currentWindowWidth <= 1025) {
    flagIs = "tablet";
}
if (!($(this).attr('data-tab') === undefined)) {
    $img.attr("src", $img.attr("data-tab"));
}

else if (!($(this).attr('data-mobHigh') === undefined)) {
    $img.attr("src", $img.attr("data-mobHigh"));
}

else if (!($(this).attr('data-mobLow') === undefined)) {
    $img.attr("src", $img.attr("data-mobLow"));
}
var names = ['data-tab', 'data-mobHigh', 'data-mobLow'];
for (var i in names) {
    var name = names[i];
    if (!($(this).attr(name) === undefined)) {
        $img.attr("src", $img.attr(name));
        break;
    }
}
$img.attr("src", $img.attr("src"));

Context

StackExchange Code Review Q#71382, answer score: 2

Revisions (0)

No revisions yet.