плагин jQuery, который делает слайдер из <ul> список

Это один из моих самых первых плагинов jQuery. Короче говоря, он делает слайдер из списка <ul>, заполненного <li> элементы, содержащие <img> s. Просто следуйте этой ссылке JSFiddle, чтобы увидеть ее в действии .

Поэтому я хотел знать, были ли какие-то недостатки в моем коде. Я бы хотел, чтобы он был просмотрен любым, кто хотел бы сообщить мне о какой-либо серьезной проблеме.

Итак, вот HTML:

<ul id="slider">
    <li><img src="http://lorempixel.com/700/300/food/1" alt="slider_1"/></li>
    <li><img src="http://lorempixel.com/700/300/food/2" alt="slider_2"/></li>
    <li><img src="http://lorempixel.com/700/300/food/3" alt="slider_3"/></li>
    <li><img src="http://lorempixel.com/700/300/food/4" alt="slider_4"/></li>
    <li><img src="http://lorempixel.com/700/300/food/5" alt="slider_4"/></li>
</ul>

И код плагина jQuery:

;(function($) {

    $.fn.extend({
        sdSlider: function(options) {
            if (options && typeof(options) == 'object') {
                options = $.extend({}, $.sdSlider.defaults, options);
            }

            // be sure the plugin is attached to only one DOMElement
            if($(this).length == 1) {
                new $.sdSlider(this, options);
            } else if($(this).length > 1) {
                console.error($.messages.severalElements);
            } else if($(this).length == 0) {
                console.error($.messages.zeroElement);
            }

            return this;
        }
    });

    // error messages to be displayed in the console
    $.messages = {
        imgAndLiMismatch: '[jQuery.sdSlider] Error: the number of list items and the number of <img src> mismatch.',
        severalElements: '[jQuery.sdSlider] Error: several DOM element have been detected. Are you sure you\'re using the right selector?',
        noImgFound: '[jQuery.sdSlider] Error: no <img> tag was found within the <li>.',
        zeroElement: '[jQuery.sdSlider] Error: couldn\'t find the slider. It seems you have targetted no element.'
    };


    $.sdSlider = function(el, option) {
        $(window).on('load', function() {

            var options  = option || $.sdSlider.defaults
            , $li = $(el).find('> li')
            , $img = $li.find('> img')
            , imgSrcs = []
            , imgHeights = []
            , imgWidths = []
            , $wrapper
            , i
            , index;

            if(!$img.length) {
                console.error($.messages.noImgFound);
                return;
            }

            // mark the first slide as active
            $(el).find('> li:first-child').addClass('active');

            // add border if asked to
            if(options.border && typeof(options.border) === 'object') {
                $img.css('border', options.border.width + 'px solid ' + options.border.color);
            }


            $img.each(function(i) {
                $(this).attr('data-image', i);
                imgSrcs.push($(this).attr('src'));
                imgHeights.push($(this).outerHeight());
                imgWidths.push($(this).outerWidth());
            });

            // if each li has an img
            if(imgSrcs.length === $li.length) {

                maxHeight = Math.max.apply(Math, imgHeights);
                maxWidth = Math.max.apply(Math, imgWidths);

                // build the wrapper div
                $wrapper = $('<div>').attr('id', 'sdSlider-wrapper').css({
                    width: maxWidth,
                    height: maxHeight
                });
                $(el).wrap($wrapper);

                // add arrows if asked to
                if(options.arrows) {
                    $.sdSlider.addArrows(el, options, $wrapper);
                }

                // add dots controls if asked to
                if(options.controls) {
                    $controlWrapper = $.sdSlider.addControls(el, options);
                    // on click on the dots
                    $controlWrapper.find('> span').on('click', function(e) {
                        // make slider slide
                        i = $(this).index();
                        $.sdSlider.appear(el, options, i);
                    });
                }

                // autostarting the slider if asked to   
                if(options.autoStart.active) {
                    window.setInterval(function() {
                        index = $(el).find('> li.active').index();
                        if(index + 1 < $img.length) {
                            $.sdSlider.appear(el, options, index + 1);
                        } else {
                            $.sdSlider.appear(el, options, index - $img.length + 1);
                        }
                    }, options.autoStart.delay);
                }



            } else {
                console.error($.messages.imgAndLiMismatch);
            }

            return;
        });
    };

    // function to add the dots control on the bottom of the slider
    $.sdSlider.addControls = function(el, options) {

        var $li = $(el).find('> li')
        , $controlWrapper = $('<div>')
                                .addClass('control-wrapper')
                                .css({
                                    'text-align': 'center',
                                    'bottom': '35px',
                                    'background-color': 'rgba(170, 170, 170, 0.5)'
                                })
        , $controls;

        $li.each(function(i) {
            $controls = $('<span>').attr('data-image', i);
            if(!i) {
                $controls.addClass('active');
            }
            $controlWrapper.append($controls);
        });

        $(el).after($controlWrapper);

        return $controlWrapper;
    }

    // function to add the controlling left and right arrows on the sides
    $.sdSlider.addArrows = function(el, options, $wrapper) {

        var classes = 'sdArrow fa-4x fa'
            , $left = $('<i />').addClass(classes + ' sdArrow-left fa-arrow-circle-left disabled').css('left', 0)
            , $right = $('<i />').addClass(classes + ' sdArrow-right fa-arrow-circle-right').css('right', 0)
            , $img = $(el).find('> li').find('> img')
            , index;
        ;

        $(el).after($left).before($right);


        // if right arrow is clicked
        $right.on('click', function() {
            index = $(el).find('> li.active').index();
            if(index + 1 < $img.length) { // if this is not the end of the slider
                // make slider go right
                $.sdSlider.appear(el, options, index + 1);
            }
        });

        // if left arrow is clicked
        $left.on('click', function() {
            index = $(el).find('> li.active').index();
            if(index - 1 >= 0) { // if this is not the beginning of the slider
                // make slider go left
                $.sdSlider.appear(el, options, index - 1);
            }
        });




        return;
    };

    // function to make the slider slide (where 'i' is the index to go to)
    $.sdSlider.appear = function(el, options, i) {
        var activeImgIndex = $(el).find('> li.active').index()
            , animation = {}
            , gap = 0
            , $li = $(el).find('> li')
            , $img = $li.find('> img')
            , $left = $(el).parent('div').find('i.sdArrow-left')
            , $right = $(el).parent('div').find('i.sdArrow-right');

        // if the slider is not currently sliding
        if(!$li.is(':animated')) {
            $li.removeClass('active').eq(i).addClass('active');
            if(activeImgIndex < i) { // going right
                gap = i - activeImgIndex;
                animation['left'] = '-=' + ($li.find('> img').eq(i).outerWidth() * gap) + 'px';
            } else { // going left
                gap = activeImgIndex - i;
                animation['left'] = '+=' + ($li.find('> img').eq(i).outerWidth() * gap) + 'px';

            }

            // slider animation
            $li.each(function() {
                $(this).animate(animation, {
                    duration: options.duration
                });
            });

            // add disabled class to arrows if needed to
            if(options.arrows) {
                if(i + 1 === $img.length) {
                    $right.addClass('disabled');
                } else {
                    $right.removeClass('disabled');
                }

                if(i === 0) {
                    $left.addClass('disabled');
                } else {
                    $left.removeClass('disabled');
                }
            }

            // add active class to corresponding dot
            $('.control-wrapper')
                .find('> span')
                .removeClass('active')
                .eq(i)
                .addClass('active')
            ;
        }

        return;
    };

    // plugin default options
    $.sdSlider.defaults = {
        autoStart: {
            active: false,
            delay: 1000
        },
        border: {
            color: '#000',
            width: 0
        },
        controls: true,
        arrows: true,
        duration: 1000
    };

})(jQuery);

Код для запуска плагина таков:

jQuery(function($) {

    $('#slider').sdSlider({
        autoStart: { // an object containing:
            active: false, // a boolean indicating whether to start the slider automatically
            delay: 1000 // and a integer specifying the delay between each slide
        },
        border: { // an object containing:
            color: '#f00', // the color of each image border as a string 
            width: 0 // the width in px of each image border as an integer
        },
        controls: true, // a boolean to specify whether to display the dots controlling each slide
        arrows: true, // a boolean to specify whether to display the left and right arrows
        duration: 500 // the duration in ms of each animation as a integer
    });

});

Вы можете увидеть более полное README в моей учетной записи Github .

Я также хотел бы знать, не является ли мой код не кросс-браузером. Пожалуйста, не стесняйтесь оставлять подробные сведения об этой точке и о том, как ее можно исправить.

Мои общие проблемы касаются следующих рекомендаций, чтобы написать хороший плагин jQuery, поэтому я ищу любые предложения о том, как улучшить этот код.

11 голосов | спросил D4V1D 1 PM00000060000000531 2015, 18:01:05

2 ответа


5

Одна вещь, которая выпрыгивает из меня, блокирует ваш плагин одним элементом:

$.fn.extend({
    sdSlider: function(options) {
        if (options && typeof(options) == 'object') {
            options = $.extend({}, $.sdSlider.defaults, options);
        }

        // be sure the plugin is attached to only one DOMElement
        if($(this).length == 1) {
            new $.sdSlider(this, options);
        } else if($(this).length > 1) {
            console.error($.messages.severalElements);
        } else if($(this).length == 0) {
            console.error($.messages.zeroElement);
        }

        return this;
    }
});

Большинство jQuery с радостью принимают несколько элементов, поэтому я предлагаю следующее изменение:

$.fn.extend({
    sdSlider: function(options) {
        if (options && typeof(options) == 'object') {
            options = $.extend({}, $.sdSlider.defaults, options);
        }

        if($(this).length == 0) {
            console.error($.messages.zeroElement);
        }

        return this.each(function(index, el){
            new $.sdSlider(el, options);
        });
    }
});

вместе с заменой:

$wrapper = $('<div>').attr('id', 'sdSlider-wrapper').css({...})

с:

$wrapper = $('<div>').addClass('sdSlider-wrapper').css({...})

Также как обновление любого релевантного css.

jsFiddle

ответил Shaun H 3 PM000000100000000831 2015, 22:49:08
4

Я увидел что-то, что могло бы сделать ваш код более сухим ...

        // be sure the plugin is attached to only one DOMElement
        if($(this).length == 1) {
            new $.sdSlider(this, options);
        } else if($(this).length > 1) {
            console.error($.messages.severalElements);
        } else if($(this).length == 0) {
            console.error($.messages.zeroElement);
        }

вы должны захватить $(this).length и сделать из него переменную. это было бы настолько чище, как это.

var objectLength = $(this).length;
if(objectLength == 1) {
    new $.sdSlider(this, options);
} else if(objectLength > 1) {
    console.error($.messages.severalElements);
} else if(objectLength == 0) {
    console.error($.messages.zeroElement);
}

, но вы, вероятно, можете придумать лучшее имя для переменной.


Вот место, которое вы, вероятно, должны делать наоборот.

if(options.controls) {
    $controlWrapper = $.sdSlider.addControls(el, options);
    // on click on the dots
    $controlWrapper.find('> span').on('click', function(e) {
        // make slider slide
        i = $(this).index();
        $.sdSlider.appear(el, options, i);
    });
}

возьмите i и

if(options.controls) {
    $controlWrapper = $.sdSlider.addControls(el, options);
    // on click on the dots
    $controlWrapper.find('> span').on('click', function(e) {
        // make slider slide
        $.sdSlider.appear(el, options, $(this).index());
    });
}

У вас есть пара мест в коде, где у вас несколько новых строк один за другим.

Не делай этого, ты можешь положить туда код и сломать вещи.

Взгляните сюда ...

        // if each li has an img
        if(imgSrcs.length === $li.length) {

            maxHeight = Math.max.apply(Math, imgHeights);
            maxWidth = Math.max.apply(Math, imgWidths);

            // build the wrapper div
            $wrapper = $('<div>').attr('id', 'sdSlider-wrapper').css({
                width: maxWidth,
                height: maxHeight
            });
            $(el).wrap($wrapper);

            // add arrows if asked to
            if(options.arrows) {
                $.sdSlider.addArrows(el, options, $wrapper);
            }

            // add dots controls if asked to
            if(options.controls) {
                $controlWrapper = $.sdSlider.addControls(el, options);
                // on click on the dots
                $controlWrapper.find('> span').on('click', function(e) {
                    // make slider slide
                    i = $(this).index();
                    $.sdSlider.appear(el, options, i);
                });
            }

            // autostarting the slider if asked to   
            if(options.autoStart.active) {
                window.setInterval(function() {
                    index = $(el).find('> li.active').index();
                    if(index + 1 < $img.length) {
                        $.sdSlider.appear(el, options, index + 1);
                    } else {
                        $.sdSlider.appear(el, options, index - $img.length + 1);
                    }
                }, options.autoStart.delay);
            }



        } else {
            console.error($.messages.imgAndLiMismatch);
        }

Прямо перед этим инструкция else. Если кто-то помещает что-то там, что, по их мнению, будет после утверждения if, или даже добавит другое выражение, чтобы не видеть уже существующий оператор else.

Я знаю, что это маловероятно, но это случается, опасайтесь слишком большого пробела в вашем коде.


Вы не должны добавлять CSS к объекту с JavaScript, он не поддерживается.

    , $controlWrapper = $('<div>')
                            .addClass('control-wrapper')
                            .css({
                                'text-align': 'center',
                                'bottom': '35px',
                                'background-color': 'rgba(170, 170, 170, 0.5)'
                            })

Вы уже добавляете класс в этот div, почему бы не использовать таблицу стилей, чтобы добавить стиль, так что вам не нужно просматривать код, чтобы изменить код background-color или что-то в этом роде.


Я хотел кое-что сказать о комментариях, но я не мог найти ничего плохого, чтобы сказать о них. Сначала я подумал, что, возможно, они будут посторонними, но они не были, они были информативными и прямолинейными.

Они казались мне хорошими комментариями.

ответил Malachi 10 PM00000050000002331 2015, 17:16:23

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132