How to show image by clicking bullet in javascript?

Question

How can i make bullet show image once it's clicked ? i have created click function to loop all the bullets,i'm not sure why its still not working did i declare kind of variable wrong ? or the code in function is wrong? please take a look at my code.

var slideShow = (function () {
    var slideImages = document.getElementsByClassName("slide");
    var slideBullets = document.getElementsByClassName("bullets");
    var current = 0;

    function reset() {        
        for (var i = 0; i < slideImages.length; i++) {
            slideImages[i].style.display = 'none';
        }
    };

    function showImages() {
        for (var i = 0; i < slideImages.length; i++) {
            slideImages[0].style.display = 'block';
        }
    };
    function showBulletsImages() {
        for (var i = 0; i < slideBullets.length; i++) {
            slideBullets[i].addEventListener("click", function () {
                reset();
                slideImages[i].style.display = 'block';
                current = i;
            });
        }
    };

    return {
        reset: reset(),
        showImages: showImages(),
        showBulletsImages: showBulletsImages()
    };
})();

#slideshow {
    position: relative;
    width: 100%;
    height: 100%;
}

#slide1 {
    background-image: url(https://preview.ibb.co/mV3TR7/1.jpg);
}

#slide2 {
    background-image: url(https://preview.ibb.co/bSCBeS/2.jpg);
}

#slide3 {
    background-image: url(https://preview.ibb.co/kgG9Yn/3.jpg);
}

.slide {
    background-repeat: no-repeat;
    background-position: center;
    background-size: 800px 400px;
    width: 800px;
    height: 400px;
    margin: auto;
    margin-top: 40px;
}

.slide-contain {
    position: absolute;
    left: 50%;
    bottom: 50%;
    transform: translate3d(-50%,-50%,0);
    text-align: center;
}

.slide-contain span {
    color: white;
}


/*bullets*/
#slidebullet {
    position: relative;
    top: -30px;
    text-align: center;
}

.bullets {
    display: inline-block;
    background-color: gray;
    width: 15px;
    height: 15px;
    border-radius: 10px;
    cursor: pointer;
    transition: background-color 0.6s ease;
}

<!DOCTYPE html>

<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
    <meta charset="utf-8" />
    <title></title>
    <link rel="stylesheet" type="text/css" href="index.css" />
</head>
<body>
  
    <div id="slideshow">

        <div id="slide1" class="slide">
            <div class="slide-contain">
                <span>Image One</span>
            </div>
        </div>

        <div id="slide2" class="slide">
            <div class="slide-contain">
                <span>Image Two</span>
            </div>
        </div>

        <div id="slide3" class="slide">
            <div class="slide-contain">
                <span>Image Three</span>
            </div>
        </div>

        <div id="slidebullet">
            <div id="bullet1" class="bullets"></div>
            <div id="bullet2" class="bullets"></div>
            <div id="bullet3" class="bullets"></div>
        </div>

        <div id="arrow-left" class="arrow"></div>
        <div id="arrow-right" class="arrow"></div>
    </div>
    <script src="jquery.js"></script>
    <script src="index.js"></script>
    <script>

    </script>
</body>
</html>

If any one would like to give me some advice or edit my code i will appreciate it thank you.

Best Place for code reviews is here https://codereview.stackexchange.com/ — winner_joiner, Jun 09 '18 at 05:22
Please change your question title, your question title says that you don't trust the SO community..... — Chirag Jain, Jun 09 '18 at 05:23
Okay you meant, you need help with your code since it doesn't work. Check out my answer below. It should solve your problem. — winner_joiner, Jun 09 '18 at 05:36

winner_joiner · Accepted Answer · 2018-06-09T16:23:07.047

The Problem in your code is, the scope of the loop variable i in the function showBulletsImages

It is better to use let instead of var to define index variables in for loops, because of such scope issues(like in your case).

Because if you use var, when the click function is called the variable i is 3 and not the value, that it had when the loop ran.

Run the code to see the result (if you are using an older browser it might not know the let keyword)

Here is some information on the let statement: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let

And here is a good answer on SO that explains the issue in more detail: https://stackoverflow.com/a/11444416/1679286

var slideShow = (function () {
    var slideImages = document.getElementsByClassName("slide");
    var slideBullets = document.getElementsByClassName("bullets");
    var current = 0;

    function reset() {        
        for (let i = 0; i < slideImages.length; i++) {
            slideImages[i].style.display = 'none';
        }
    };

    function showImages() {
        for (let i = 0; i < slideImages.length; i++) {
            slideImages[0].style.display = 'block';
        }
    };
    function showBulletsImages() {
        for (let i = 0; i < slideBullets.length; i++) {
            slideBullets[i].addEventListener("click", function () {
                reset();
                slideImages[i].style.display = 'block';
                current = i;
            });
        }
    };

    return {
        reset: reset(),
        showImages: showImages(),
        showBulletsImages: showBulletsImages()
    };
})();

#slideshow {
    position: relative;
    width: 100%;
    height: 100%;
}

#slide1 {
    background-image: url(https://preview.ibb.co/mV3TR7/1.jpg);
}

#slide2 {
    background-image: url(https://preview.ibb.co/bSCBeS/2.jpg);
}

#slide3 {
    background-image: url(https://preview.ibb.co/kgG9Yn/3.jpg);
}

.slide {
    background-repeat: no-repeat;
    background-position: center;
    background-size: 800px 400px;
    width: 800px;
    height: 400px;
    margin: auto;
    margin-top: 40px;
}

.slide-contain {
    position: absolute;
    left: 50%;
    bottom: 50%;
    transform: translate3d(-50%,-50%,0);
    text-align: center;
}

.slide-contain span {
    color: white;
}


/*bullets*/
#slidebullet {
    position: relative;
    top: -30px;
    text-align: center;
}

.bullets {
    display: inline-block;
    background-color: gray;
    width: 15px;
    height: 15px;
    border-radius: 10px;
    cursor: pointer;
    transition: background-color 0.6s ease;
}

<!DOCTYPE html>

<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
    <meta charset="utf-8" />
    <title></title>
    <link rel="stylesheet" type="text/css" href="index.css" />
</head>
<body>
  
    <div id="slideshow">

        <div id="slide1" class="slide">
            <div class="slide-contain">
                <span>Image One</span>
            </div>
        </div>

        <div id="slide2" class="slide">
            <div class="slide-contain">
                <span>Image Two</span>
            </div>
        </div>

        <div id="slide3" class="slide">
            <div class="slide-contain">
                <span>Image Three</span>
            </div>
        </div>

        <div id="slidebullet">
            <div id="bullet1" class="bullets"></div>
            <div id="bullet2" class="bullets"></div>
            <div id="bullet3" class="bullets"></div>
        </div>

        <div id="arrow-left" class="arrow"></div>
        <div id="arrow-right" class="arrow"></div>
    </div>
    <script src="jquery.js"></script>
    <script src="index.js"></script>
    <script>

    </script>
</body>
</html>

Brief Info

This function

    function showBulletsImages() {
        for (var i = 0; i < slideBullets.length; i++) {
            slideBullets[i].addEventListener("click", function () {
                reset();
                slideImages[i].style.display = 'block';
                current = i;
            });
        }
    }

will be interpreted as

   function showBulletsImages() {
        var i;
        for (i = 0; i < slideBullets.length; i++) {
            slideBullets[i].addEventListener("click", function () {
                /* the i variable that is used in this function, can be 
                   altered from outside of the function, so with every loop
                   iteration the i variable will increase. So at the end 
                   of the for loop, the i variable will be set in all click
                   functions to the last value of i, which will be 3, since
                   i < slideBullets.length  will end to loop, when i = 3;
                */
                reset();
                slideImages[i].style.display = 'block';
                current = i;
            });
        }
    }

On the other hand

    function showBulletsImages() {
        for (let i = 0; i < slideBullets.length; i++) {
            /* 
               here the i variable is only valid in the for-block,
               and is unique for each iteration, not like in the var
               case, SO the i, won't be alter by other iterations.
            */
            slideBullets[i].addEventListener("click", function () {
                reset();
                slideImages[i].style.display = 'block';
                current = i;
            });
        }
    }

One more question about what you have said "Because if you use var, when the click function is called the variable i is 3 and not the value, that it had when the loop ran." May you more clarify for me please ? how do you know when the click function is called then variable i is 3 ? @winner_joiner — Saranyusocial, Jun 09 '18 at 14:38
@Saranyusocial I updated my answer with some more explaination — winner_joiner, Jun 09 '18 at 16:27
That's very clear and useful for me i appreciate it thanks for your explanation. — Saranyusocial, Jun 10 '18 at 03:22

How to show image by clicking bullet in javascript?

1 Answers1