HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Quality of custom slider

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
customqualityslider

Problem

Should I be using this more, instead of slider? Am I making any serious no-no's here, and is there a better design I should implement? I'm up for any pointers :)

slider = {
                selector : '#slider',
                init : function() {
                        $('#slider').hover(function(e){
                            slider.toggle();
                        });

                    $(document).bind('click', function(e){
                        slider.lock.unlock();
                        slider.state.collapse();
                    });

                    $('#slider').bind('click', function(e){
                        e.stopPropagation();
                        slider.lock.lock();
                    })
                },
                toggle : function() {
                    if (!this.lock.isLocked) {
                        this.state.toggle();
                    }
                },
                lock : {
                    isLocked   : false,
                    lock : function() {
                        this.isLocked = true;
                    },
                    unlock : function() {
                        this.isLocked = false;
                    }
                },
                state : {
                    isExtended : false,
                    toggle : function() {
                        if (this.isExtended===true) {
                            this.collapse();
                        } else {
                            this.extend();
                        }
                    },
                    extend : function() {
                        this.isExtended = true;
                        $('#slider').stop().animate({'left':'-5px'}, 'slow');
                    },
                    collapse : function() {
                        this.isExtended = false;
                        $('#slider').stop().animate({'left':'-210px'}, 'slow');
                    }
                }
            };

Solution

-
You never use slider.selector, so what's the point?

-
jQuery instances should be "cached", otherwise you are creating new jQuery object each time you call $() function.

-
This code permits only one slider per page. What if you need more? Will you copy-paste the whole thing?

-
This is not configurable at all. You have to change the objects source, just to change the way animation works.

-
jQuery is not really necessary here. You actually need it only for animations, which is not jQuery's strong point.

  • Slider object ends up in global scope.

Context

StackExchange Code Review Q#6213, answer score: 7

Revisions (0)

No revisions yet.