3
\$\begingroup\$

This uses HTML5 data attributes to load the image according to the screen width. For example, for laptops. it loads only laptop-sized images. saving bandwidth and optimizing speed.

Live example

    //---------------------------Setting ViewPort Widths-----------------------//
var sSize=768; // High End Mobile Phones
var mSize=1024; // Tables like Ipad Or Something..
var lSize=1336; // Laptop 15 Inch
var xlSize=1500;  // Desktop  22 Inch And Above


//------------------------------ Grabbing Images in Images Array ------------//
var images = document.getElementsByTagName('img'); 



//--------------------------------Checking Current Resolution Function -----------///
function checkRes()
{
var width = document.documentElement.clientWidth; // grab the width of the screen //
var currentFlag=""; // To Check the Final Resolution



// --------------------- Paths of Img  To Grab ---------------------------//

var  xlImgPath='data-xl';
var  lImgPath='data-l';
var  mlmgPath='data-m';
var  slmgPath='data-s';




//------------------------------  Set CurrentFlag to the Current Browser Size /////////

if (width>=xlSize) {
    currentFlag="xl";
}
else if (width>=lSize   ) 
{
    currentFlag="l";
}
else if (width>=mSize   ) 
{
    currentFlag="m";

}
else if (width<=sSize   ) 
{
    currentFlag="s";

}







    //-------------------------------  Cycle through all the Images And Attach the Proper Src ---------//////////
    for (var i = 0; i < images.length; i++)
    {   
        var thisImage=images[i];  // Grab Current Image //      
        var defaultImgSource =images[i].getAttribute('src'); // grabs the Current image src (The Default Image)

//-------------------------------------- Switch Case Accoring to Current Size ------------------

switch(currentFlag){


    case 'xl':
                if (thisImage.getAttribute(xlImgPath)!==null)  // If Desktop Image has been defined..
                { 

                thisImage.src=thisImage.getAttribute(xlImgPath);   // Set The path of Image to  Desktop Path
            }
            else
            {

                thisImage.src=defaultImgSource; // Else Set the Fefaut src image 

            }

            break;
            case 'l':
            if (thisImage.getAttribute(lImgPath)!==null)  
            { 

                thisImage.src= thisImage.getAttribute(lImgPath);   
            }
            else
            {

                thisImage.src=defaultImgSource; 

            }
            break;

            case 'm':
            if (thisImage.getAttribute(mlmgPath)!==null)  
            { 
                thisImage.src=thisImage.getAttribute(mlmgPath);  
            }
            else
            {

                thisImage.src=defaultImgSource; 

            }
            break;

            case 's':
            if (thisImage.getAttribute(slmgPath)!==null)  
            { 

                thisImage.src=thisImage.getAttribute(slmgPath);     
            }

            else
            {

                thisImage.src=defaultImgSource; 

            }
            break;
        }


    }
};


//---------------------------------Run The Function On Page Load ---------------------
checkRes();

HTML

        <img 
src="http://s30.postimg.org/rjm6iy2kh/default.jpg"  
data-s="http://s4.postimg.org/4gmjg2m7h/image.jpg"
data-m="http://s4.postimg.org/4gmjg2m7h/image.jpg" 
data-l="http://s4.postimg.org/4gmjg2m7h/image.jpg" 
data-xl="http://s4.postimg.org/4gmjg2m7h/image.jpg" >
\$\endgroup\$

3 Answers 3

4
\$\begingroup\$

Please avoid comments like these (Bad)

 //------------------------------ Grabbing Images in Images Array ------------//
    var images = document.getElementsByTagName('img');

Instead use this: (Good)

// Grabbing Images in Images Array 
var images = document.getElementsByTagName('img');

The current image images[i] should be stored in a simple variable named image instead of thisImage. Please use meaningful and simple names for all variables

Create a function which sets the image source as most of the code is common inside all switches

function setImageSource(image, path, defaultImgSource) {
        // If Desktop Image has been defined..
        if (image.getAttribute(path)) {
            // Set The path of Image to  Desktop Path
            image.src = image.getAttribute(path); 
        } else {
            // Set the Default src image 
            image.src = defaultImgSource; 
        }
    }

The actual switch block can be modified as below:

// Cycle through all the Images And Attach the Proper Src 
for (var i = 0; i < images.length; i++) {
    var image = images[i]; // Grab Current Image //    
    // grabs the Current image src (The Default Image)  
    var defaultImgSource = image.getAttribute('src'); 


    switch (currentFlag) {

        case 'xl':
            setImageSource(image, xlImgPath, defaultImgSource);
            break;
        case 'l':
            setImageSource(image, lImgPath, defaultImgSource);

            break;

        case 'm':
            setImageSource(image, mlmgPath, defaultImgSource)
            break;

        case 's':
            setImageSource(image, slmgPath, defaultImgSource)
            break;
    }

}

We even can even avoid switch completely by dynamic variables With this your code will be reduced a lot. This refers to global context in this

setImageSource(image, this[currentFlag + "ImgPath"], defaultImgSource);
\$\endgroup\$
0
4
\$\begingroup\$

Your code has really, really poor indentation.

Luckily for you, you are using JSFiddle to write and run your JavaScript code. That means that, if you look up at the top of the screen, you can click the TidyUp button, which will go through and properly indent and space out all of your code.

Then, as practice, look at the difference between the TideUped code and the original code; what has been indented and what stayed the same? Locate patterns so in the future, you could write your code with correct indentation from the start.


//---------------------------------Run The Function On Page Load ---------------------
checkRes();

I'm not sure if this comment is entirely true. If not, it's not entirely clean. When I see that code, I don't think,

Oh, he's obviously trying to get that function to run when the webpage loads.

I think that you are just running a function right after you've created it. I recommend setting this to the onload of the webpage, just to make things more clear and to be absolutely 100% sure the function will run after everything has loaded.

Here is what I mean:

window.onload = checkRes;

Then, when the window is loaded, checkRes will be called.


I may be missing something, but this seems a little redundant:

thisImage.src = defaultImgSource; // Else Set the Fefaut src image 

Isn't defaultImgSource already the image source, which is what thisImage.src is? I think you can safely omit this line and the else conditional.


thisImage.src = thisImage.getAttribute(xlImgPath); // Set The path of Image to  Desktop Path

Why are you using thisImage.src here? You already set defaultImgSource to the image's source.


Consider using an object to store all the information you need for each size, such as the size itself and the attribute that will hold the new image.

Here is what I came up with:

var sizes = {
    "xl": {
        dataAttribute: "data-xl",
        size: 1500
    },
    ...
}

Now, rather than having a bunch of spread out variables, everything is together in one neat place.

However, just having this object isn't enough; let's take this one step further by simplifying that bulky switch statement.

Now that we have this brand new object, we can iterate through that to find and compare against different values.

To start, lets find the object's type:

for(var flag in sizes) {
    if(sizes.hasOwnProperty(flag)) {
        if(currentFlag === flag) {
            ...
        }
    }
}

That last nested conditional is basically doing what you are doing here:

switch(currentFlag) {
    case "xl":
        ...
    ...
}

Only, rather than hard-coding the tags into the code, this is just using the tags from the object that we already created.

Now, all that's left is to check the data property and set the new image.

Here is what that looks like:

var dataAttribute;
for(var flag in sizes) {
    if(sizes.hasOwnProperty(flag)) {
        dataAttribute = thisImage.getAttribute(sizes[flag].dataAttribute)
        if(currentFlag === flag) {
            if(dataAttribute !== null) {
                thisImage.src = dataAttribute;
            }
        }
    }
}

Alright, let's break this down:

dataAttribute = thisImage.getAttribute(sizes[flag].dataAttribute)

This line is getting the data attribute of the element ("data-xl", "data-s" ...) and setting it to dataAttribute.

The reason why dataAttribute is set in the conditional is so that the method .getAttribute does not have to be called again if the conditional passes and the new src attribute is being set.


I recommend creating a new function called setScreenSizeFlag that does what this function does:

if (width >= xlSize) {
    currentFlag = "xl";
} else if (width >= lSize) {
    currentFlag = "l";
} else if (width >= mSize) {
    currentFlag = "m";

} else if (width <= sSize) {
    currentFlag = "s";

}

The reason: this conditional is making this function do more than it should.


Putting it all together:

var sizes = {
    "xl": {
        dataAttribute: "data-xl",
        size: 1500
    },
    "l": {
        dataAttribute: "data-l",
        size: 1336
    },
    "m": {
        dataAttribute: "data-m",
        size: 1024
    },
    "s": {
        dataAttribute: "data-s",
        size: 768
    }
}

//------------------------------ Grabbing Images in Images Array ------------//
var images = document.getElementsByTagName('img');

function setScreenSizeFlag() {
    var width = document.documentElement.clientWidth; // grab the width of the screen //
    var currentFlag;
    if (width >= xlSize) {
        currentFlag = "xl";
    } else if (width >= lSize) {
        currentFlag = "l";
    } else if (width >= mSize) {
        currentFlag = "m";

    } else if (width <= sSize) {
        currentFlag = "s";
    }
    return currentFlag
}

//--------------------------------Checking Current Resolution Function -----------///
function checkRes() {
    //------------------------------  Set CurrentFlag to the Current Browser Size /////////

    var currentFlag = setScreenSizeFlag();


    //-------------------------------  Cycle through all the Images And Attach the Proper Src ---------//////////
    for (var i = 0; i < images.length; i++) {
        var thisImage = images[i]; // Grab Current Image //     
        var defaultImgSource = images[i].getAttribute('src'); // grabs the Current image src (The Default Image)

        //-------------------------------------- Switch Case Accoring to Current Size ------------------

        var dataAttribute;
        for(var flag in sizes) {
            if(sizes.hasOwnProperty(flag)) {
                if(currentFlag === flag) {
                    dataAttribute = thisImage.getAttribute(sizes[flag].dataAttribute);
                    if( dataAttribute !== null) {
                        thisImage.src = dataAttribute;
                    }
                }
            }
        }
    }
};


//---------------------------------Run The Function On Page Load ---------------------
window.onload = checkRes;
\$\endgroup\$
0
4
\$\begingroup\$

I have a few comments, which in no way substitute a full review.

Your indentation is inconsistent which makes your code unnecessarily hard to read.

Your comments are also inconsistent. I'm not going to prescribe you a commenting standard, but pick a style you like and stick with it.

Now we're talking about comments, the following is redundant:

var thisImage=images[i];  // Grab Current Image // 

It's still clear what you're doing without the comment. It's commenting the 'what' of your code while you should only be commenting the 'why'. The 'what' is already obvious in this case, as it should be. You may be interested in reading Writing Readable Code, about readable code and what part comments play in this. Comments can be good, but they are often not necessary.

You're only using the width of the resolution. While most desktop screens use standard ratios, you seem to target mobile users. Mobile devices don't necessarily stick to standard ratios. For width your screen may be a small, but for height it could be a medium.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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