3
\$\begingroup\$

I just created a simple gallery with scroll. Everything works fine but I'm not sure how should I optimize this code:

document.addEventListener("DOMContentLoaded", function() {

    var smallImages = document.querySelectorAll('li img');
    var bigPhotoDiv = document.querySelector('.bigPhoto img');
    var scroller = document.querySelector('#scroller');
    var list = document.querySelector('ul');

    scroller.max = smallImages.length - 1;
    scroller.defaultValue = 0;
    smallImages[scroller.defaultValue].classList.add('centralTheBiggest');

    bigPhotoDiv.src = smallImages[0].src;
    bigPhotoDiv.classList.add('fullSize')

    scroller.addEventListener('input', function(event) {
        //changing the big photo in div
        var currentLink = smallImages[scroller.value].src;
        bigPhotoDiv.src = currentLink;

        //move the list with photos
        list.scrollLeft = scroller.value * 70; //poprawka

        //emphesasise choosen photo

        for (var i = 0; i < smallImages.length; i++) {
            smallImages[i].classList.remove('centralTheBiggest')
        }
        if (smallImages.index = scroller.value) {
            smallImages[scroller.value].classList.add('centralTheBiggest');
        }
    });
    var body = document.querySelector('body');

    //clicking on big photo display it on fullscreen
    bigPhotoDiv.addEventListener('click', function(event) {
        var fullScreen = document.createElement('div');
        fullScreen.classList.add('fullScreenDiv');

        var fullScreenImgContainer = document.createElement('div');
        fullScreenImgContainer.classList.add('fullScreenImgContainer');

        var fullScreenImg = document.createElement('img');
        fullScreenImg.src = smallImages[scroller.value].src;
        fullScreenImg.classList.add('fullScreenImg')

        fullScreenImgContainer.appendChild(fullScreenImg)
        fullScreen.appendChild(fullScreenImgContainer);
        body.appendChild(fullScreen);
        //adding buttons:
        var nextButton = document.createElement('button');
        nextButton.classList.add('nextButton');
        fullScreenImgContainer.appendChild(nextButton);

        var prevButton = document.createElement('button');
        prevButton.classList.add('prevButton');
        fullScreenImgContainer.appendChild(prevButton);

        var closeButton = document.createElement('button');
        closeButton.classList.add('closeButton');
        fullScreenImgContainer.appendChild(closeButton);

        closeButton.addEventListener('click', function(event) {
            fullScreen.parentNode.removeChild(fullScreen);
        });
        //adding events to new buttons
        nextButton.addEventListener('click', function(event) {
            scroller.value++
                fullScreenImg.src = smallImages[scroller.value].src;
            bigPhotoDiv.src = smallImages[scroller.value].src;
        });

        prevButton.addEventListener('click', function(event) {
            scroller.value--
                fullScreenImg.src = smallImages[scroller.value].src;
            bigPhotoDiv.src = smallImages[scroller.value].src;
        });
        //hiding and showing prev and next button
        fullScreenImgContainer.addEventListener('mouseover', function() {
            nextButton.style.display = "block";
            prevButton.style.display = "block";
        });
        fullScreenImgContainer.addEventListener('mouseout', function() {
            nextButton.style.display = "none";
            prevButton.style.display = "none";
        });
    });
});

Is it a good way to put so many Events inside another event? or should I create all possible elements (like nextButton, prevButton for example) outside the 'click' event? And what about hiding and showing those buttons? Is it a good idea to put it inside 'mouseover' event?

\$\endgroup\$
3
  • \$\begingroup\$ I'm not sure if this is going to improve things, but you could consider changing classes on the events instead of building & adding / removing entire elements. If you set everything hidden & ready in the dom, you would only need to toggle visibility instead of the heavy lifting. \$\endgroup\$ Commented Jun 4, 2016 at 12:00
  • \$\begingroup\$ I was considering this solution and I think sometimes there is no need to create those elements before user decides to display images on full screen. For example this gallery is just a part of a whole website and user is not interested in my gallery so he will scroll down through the website and he doesn't need elements like prevButton etc. \$\endgroup\$ Commented Jun 4, 2016 at 12:29
  • \$\begingroup\$ You can make one method to create the elements, another to toggle visibility. That way, first run = create, second run = visibility. If this is only about a few images though, I wouldn't sweat it because you're not likely to run into noticable performance boosts with that change. \$\endgroup\$ Commented Jun 4, 2016 at 13:11

0

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.