patternjavascriptMinor
Responsive image code
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').
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:
This is the same, and if someday you'll want to adjust the thresholds, it will be a lot easier:
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:
The checking and setting of attributes can be simplified,
using a list and a loop,
something like this:
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
I don't know if I'm missing something but this looks utterly pointless:
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.
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.