5
\$\begingroup\$

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');
                    }
                }
            };
\$\endgroup\$
7
\$\begingroup\$
  1. You never use slider.selector, so what's the point?

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

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

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

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

  6. Slider object ends up in global scope.
\$\endgroup\$
0
\$\begingroup\$

All your slider calls are inside the jQuery callbacks, so they use their own context. For example, in this callback:

$('#slider').hover(function(e){
   slider.toggle();
});

this would refer to the #slider DOM element, not your slider object.

At the jQuery help page they suggest a basic Module pattern that would help you a lot designing your plugin slider.

\$\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.