Why does one function work and the other one doesn't work? (Javascript)

Question

I'm practicing on one project and i'm trying to use module pattern because i want to avoid global variables as much as i can.

My autoslide and dropdown menu are work fine. But when i clicked Topics then submenu will show, and in the same time my images is sliding.

Why dose my submenu suddenly disappear ?

var slideShow = (function () {
    var slideImages = document.getElementsByClassName("slide");
    var leftSide = document.getElementById("arrow-left");
    var rightSide = document.getElementById("arrow-right");
    var slideBullets = document.getElementsByClassName("bullets");
    var current = 0;

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

    function showImages() {
        for (let i = 0; i < slideImages.length; i++) {
            slideImages[0].style.display = 'block';
            slideBullets[current].classList.add("clicked");
        }
    };

    function arrowSlide() {
        leftSide.addEventListener("click", function () {
            reset();
            if (current === 0) {
                current = slideImages.length;
            }
            slideImages[current - 1].style.display = 'block';       
            current--;
            slideBullets[current].classList.add("clicked");
        });

        rightSide.addEventListener("click", function () {
            reset();
            if (current === slideImages.length - 1) {
                current = - 1;
            }
            slideImages[current + 1].style.display = 'block';         
            current++;
            slideBullets[current].classList.add("clicked");
        });
    };

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

    function autoSlide() {
        setInterval(function () {
            rightSide.click();
            slideBullets[current].classList.add('clicked')
        }, 2000);
    };

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

var toggleMenu = (function () {
    var mainTopics = document.getElementById("maintopics");

    mainTopics.addEventListener("click", function (e) {
        e.preventDefault();
        e.stopImmediatePropagation();
        mainTopics.classList.toggle("show");
    });

    document.addEventListener("click", function () {
        mainTopics.classList.remove("show");
    });

    return {
        toggleMenu: toggleMenu()
    };
})();

body {
    margin: 0;
}

li, a{
    text-decoration: none;
    list-style-type: none;
    text-decoration-line: none;
    color: black;
}

/*main-menu*/
#mainmenu {
    position: relative;
}

#mainmenu ul {
    margin: 0;
    padding: 0;
}

#mainmenu li {
    display: inline-block;
}

#mainmenu a {
    display: block;
    width: 100px;
    padding: 10px;
    border: 1px solid;
    text-align: center;
}

/*sub-topics*/
#subtopics {
    position: absolute;
    display: none;
    margin-top: 10px;
    width: 100%;
    left: 0;
}

#maintopics.show #subtopics {
    display: block;
}

#subtopics ul {
    margin: 0;
    padding: 0;
}

#subtopics li {
    display: block;
}

#subTopics a {
    text-align: left;
}

/*columns*/
#column1, #column2, #column3 {
    position: relative;
    float: left;
    left: 125px;
    margin: 0px 5px 0px 0px;
}

/*hover underline*/
#mainmenu li:hover {
    text-decoration: underline;
}

/*slideshow*/
#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;
}

/*arrow*/
.arrow {
    position: absolute;
    cursor: pointer;
    top: 200px;
    width: 0;
    height: 0;
    border-style: solid;
}

.arrow:hover {
    background-color: #e0dede;
    transition: background-color 0.6s ease;
}


#arrow-left {
    position: absolute;
    border-width: 30px 40px 30px 0px;
    border-color: transparent gray transparent transparent;
    left: 0;
    margin-left: 300px;
}


#arrow-right {
    border-width: 30px 0px 30px 40px;
    border-color: transparent transparent transparent gray;
    right: 0;
    margin-right: 300px;
}

/*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;
}

.clicked {
    background-color: #ff0000;
}

<!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>
    <header></header>
    <nav>
        <div id="mainmenu">
            <ul>
                <li><a href="">Logo</a></li>
                <li><a href="">Home</a></li>
                <li id="maintopics"><a href="">Topics</a>
                    <div id="subtopics">
                        <div id="column1" class="columns">
                            <ul>
                                <li><a href="">example1</a></li>
                                <li><a href="">example2</a></li>
                            </ul>
                        </div>
                    </div>
                </li>
            </ul>
        </div>
    </nav>
    <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>

also your ` leftSide.addEventListener("click"` is inside a function that you may be calling each time and this attaches click event everytime you call `arrowSlide` — Muhammad Usman, Jun 19 '18 at 07:03
@GeorgeBailey So what is the point ? Can you more clarify for me please? — Saranyuquestion, Jun 19 '18 at 07:06
adding event listeners just one time will do it. You don't have to do it again and again — Muhammad Usman, Jun 19 '18 at 07:07
The title ... please edit the title to describe the issue at hands. — Teemu, Jun 19 '18 at 07:15
@GeorgeBailey you mean i should only code one addEventListener in one function ? — Saranyuquestion, Jun 19 '18 at 07:16

Mihai T · Accepted Answer · 2018-06-19T14:07:12.733

The fact is that you hide the mainTopics when click on the documment.

You have a click on the document when you trigger the click on the arrow-right here : rightSide.click(); . ( arrow-right is, like everything else, part of the document. )

This triggers a click event on the document so it hides the mainTopics

One solution would be to use event.target inside the document click event, and condition it NOT to be the arrow-right element ( or the left-arrow if you want, it's easy to modify )

  if (e.target.id != 'arrow-right') {
    mainTopics.classList.remove("show");
  }

As a side Note: Depending on the conditions you have, you can write this if statement in 2 more different ways

e.target.id != 'arrow-right' && mainTopics.classList.remove("show")

or

e.target.id != 'arrow-right' ? mainTopics.classList.remove("show") : ""

See code snippet below. Let me know if it works.

var slideShow = (function () {
    var slideImages = document.getElementsByClassName("slide");
    var leftSide = document.getElementById("arrow-left");
    var rightSide = document.getElementById("arrow-right");
    var slideBullets = document.getElementsByClassName("bullets");
    var current = 0;

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

    function showImages() {
        for (let i = 0; i < slideImages.length; i++) {
            slideImages[0].style.display = 'block';
            slideBullets[current].classList.add("clicked");
        }
    };

    function arrowSlide() {
        leftSide.addEventListener("click", function () {
            reset();
            if (current === 0) {
                current = slideImages.length;
            }
            slideImages[current - 1].style.display = 'block';       
            current--;
            slideBullets[current].classList.add("clicked");
        });

        rightSide.addEventListener("click", function () {
            reset();
            if (current === slideImages.length - 1) {
                current = - 1;
            }
            slideImages[current + 1].style.display = 'block';         
            current++;
            slideBullets[current].classList.add("clicked");
        });
    };

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

    function autoSlide() {
        setInterval(function () {
            rightSide.click();
            slideBullets[current].classList.add('clicked')
        }, 2000);
    };

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

var toggleMenu = (function () {
    var mainTopics = document.getElementById("maintopics");

    mainTopics.addEventListener("click", function (e) {
        e.preventDefault();
        e.stopImmediatePropagation();
        mainTopics.classList.toggle("show");
    });

    document.addEventListener("click", function (e) {
      if (e.target.id != 'arrow-right') {
        mainTopics.classList.remove("show");
      }
    });

    return {
        toggleMenu: toggleMenu()
    };
})();

body {
    margin: 0;
}

li, a{
    text-decoration: none;
    list-style-type: none;
    text-decoration-line: none;
    color: black;
}

/*main-menu*/
#mainmenu {
    position: relative;
}

#mainmenu ul {
    margin: 0;
    padding: 0;
}

#mainmenu li {
    display: inline-block;
}

#mainmenu a {
    display: block;
    width: 100px;
    padding: 10px;
    border: 1px solid;
    text-align: center;
}

/*sub-topics*/
#subtopics {
    position: absolute;
    display: none;
    margin-top: 10px;
    width: 100%;
    left: 0;
}

#maintopics.show #subtopics {
    display: block;
}

#subtopics ul {
    margin: 0;
    padding: 0;
}

#subtopics li {
    display: block;
}

#subTopics a {
    text-align: left;
}

/*columns*/
#column1, #column2, #column3 {
    position: relative;
    float: left;
    left: 125px;
    margin: 0px 5px 0px 0px;
}

/*hover underline*/
#mainmenu li:hover {
    text-decoration: underline;
}

/*slideshow*/
#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;
}

/*arrow*/
.arrow {
    position: absolute;
    cursor: pointer;
    top: 200px;
    width: 0;
    height: 0;
    border-style: solid;
}

.arrow:hover {
    background-color: #e0dede;
    transition: background-color 0.6s ease;
}


#arrow-left {
    position: absolute;
    border-width: 30px 40px 30px 0px;
    border-color: transparent gray transparent transparent;
    left: 0;
    margin-left: 300px;
}


#arrow-right {
    border-width: 30px 0px 30px 40px;
    border-color: transparent transparent transparent gray;
    right: 0;
    margin-right: 300px;
}

/*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;
}

.clicked {
    background-color: #ff0000;
}

<!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>
    <header></header>
    <nav>
        <div id="mainmenu">
            <ul>
                <li><a href="">Logo</a></li>
                <li><a href="">Home</a></li>
                <li id="maintopics"><a href="">Topics</a>
                    <div id="subtopics">
                        <div id="column1" class="columns">
                            <ul>
                                <li><a href="">example1</a></li>
                                <li><a href="">example2</a></li>
                            </ul>
                        </div>
                    </div>
                </li>
            </ul>
        </div>
    </nav>
    <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>

Your explanation is the best, makes me more understand i wish i can be professional like you, i really appreciate your help thank you very much. — Saranyuquestion, Jun 19 '18 at 10:38

score 0 · Answer 2 · answered Jun 19 '18 at 07:09

0

For mainTopics you have attached click event at two DOM element.

mainTopics
Document

When you click, show class is added due to mainTopics DOM click event

Due event bubbling , document capture the click event which remove the show class.

So,at the same time show class added and removed.

mainTopics.addEventListener("click", function (e) {
        e.preventDefault();
        e.stopImmediatePropagation();
        mainTopics.classList.toggle("show");
    });

    document.addEventListener("click", function () {
        mainTopics.classList.remove("show");
    });

answered Jun 19 '18 at 07:09

RIYAJ KHAN

15,032
5
31
53

Seems like there's a long way to go, i don't understand that much, Does it means should i only use one addEventListener ? – Saranyuquestion Jun 19 '18 at 07:15
@Saranyuquestion no issue on number of listener.But make sure eventlistener not causing reversing the value of other changes.Generally avoid listner on `Document` directly.Instead attach it to particular named dom only. – RIYAJ KHAN Jun 19 '18 at 07:23

Why does one function work and the other one doesn't work? (Javascript)

2 Answers2