1
\$\begingroup\$

Problem: Make a slider, located at the bottom of the page. The slider should be dynamically loaded only when a div (#slider) becomes visible on the screen. Images do not have to load all at once. Load the first two images, then load the third when we switched to the second, and so on.

I would like other users to look look at my code and tell me what I'm doing wrong and what could be done better (if you can - in more detail).

$(function () {
    var countImg = 5,
        indexImg = 1,
        indexImgLoad = 2,
        pathImg = '/img/slider/',
        formatImg = 'jpg',
        controlFlag = 1,
        elWrap = $('#slider'),
        el =  elWrap.find('img');
    function create () {
        Img = new Image(); 
        Img.src= pathImg+indexImg+'.'+formatImg;                        
        $(Img).appendTo($("#slides"));
        $('<span class="next"></span><span class="prev"></span>').appendTo($("#slider"));
        navigation();
        check();
    }
    function change () {
        el.fadeOut(500);
        el.filter(':nth-child('+indexImg+')').fadeIn(500);
    }
    function newimg() {
        nextImg = new Image(); 
        nextImg.src= pathImg+indexImgLoad+'.'+formatImg;                        
        $(nextImg).appendTo($("#slides"));
    }
    function check() {
        if (controlFlag && indexImgLoad <= countImg) {
            newimg();        
        }
        if (controlFlag){
            el =  elWrap.find('img');
        }
        if (el.length == countImg){    
            controlFlag = 0;
        }
    }
    function navigation(){
        $('span.next').click(function() {
            indexImg++;
            indexImgLoad++;
            if(indexImg > countImg) {
                indexImg = 1;
            }
            check();
            change();
        });
        $('span.prev').click(function() {
            indexImg--;                    
            if(indexImg < 1) {
                indexImg = countImg;
            }
            indexImgLoad++;
            check();
            change ();
        });    
    }
    var loadSlider = 1;
    window.onscroll = function() {                
          var pageY = window.pageYOffset || document.documentElement.scrollTop,
        heightScreen = window.innerHeight + pageY;
        sliderTopY = $("#slides").offset().top;
        if ((heightScreen > (sliderTopY - sliderTopY * 0.05)) && loadSlider) {
            loadSlider = 0;
            create();
        } 
    }
});
.slider_wrap {
        margin: 0px auto 0;
        width: 735px;
        height: 200px;
        position: relative;
        overflow: hidden;
    }
    .slider_wrap img {
        width: 645px;
        height: auto;
        display: none;
        position: absolute;
        top: 0;
        left: 45px;    
    }
    .slider_wrap img:first-child {
        display: block;
    }
    .slider_wrap span {
        width: 45px;
        height: 200px;
        display: block;
        position: absolute;
        cursor: pointer;
        background: url(/img/rotator_buttons.png) no-repeat;
        opacity: 0.7;
    }
    .slider_wrap span.next {
        right: 0;
        background-position: -45px 0;
    }
    .slider_wrap span.next:hover, span.prev:hover {
        opacity: 1.0;
    }
    .slider_wrap span.prev {
        background-position: 0 0;
    }
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div style="height:1800px;">div: height = 1800px</div>
    <div id="slider" class="slider_wrap">
        <div id="slides">
        </div>            
    </div>

\$\endgroup\$
  • \$\begingroup\$ (quote)The slider should be dynamically loaded only when an unicorn becomes visible on the screen.(/quote) \$\endgroup\$ – Mathieu Guindon Oct 24 '13 at 23:26
  • \$\begingroup\$ Sorry :) There was: div id=slider \$\endgroup\$ – M03G Oct 25 '13 at 6:27
1
\$\begingroup\$

Few things:

  1. You can create a plugin out of it (create divs #slider and #slides - don't use id, but just reference - and add class slider_wrap in code)
  2. You don't put var before nextImg = new Image();
  3. Instead of new Image you can use jQuery:

    $('<img/>').attr('src', path).appendTo($("#slides"));

  4. Instead of $('span.next') use elWrap.find('span.next') because it will not work if span.next is somewhere in the html.

  5. Is always better ot use jQuery not native function because they may be different from browser to browser. So use $(window).scroll(function() { }) $(window).innerHeight() and $(window).scrollTop();
  6. Inside newimg you call $("#slides") you can call that before a function and store it's value in variable.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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