0

Currently tying to keep my code within an object, I would like to convert the contents of a click event into a method but not overly sure how this is done. The current code looks like this:

Current JS

init: function(){

            var _this = this,
                slides_li = _this.els.slides.children(),
                large = 260,
                small = 60;

            // Hover
            slides_li.on('mouseover', function(){
                $(this).stop(true, true).animate({ opacity: 1 }, 300);
            }).on('mouseout', function(){
                $(this).stop(true, true).animate({ opacity: .8 }, 300)
            });

            // Click handlers
            slides_li.on('click', function(){

// Would like to move this code into its own method toggle_slides() ??

                $('.active', _this.els.slides).not(this).animate({
                    width: small
                }, 300).removeClass('active');
                // animate the clicked one

                if ( !$(this).hasClass('active') ){
                    $(this).animate({
                        width: large
                    }, 300).addClass('active');
                }                
            });                 
        }

but I would like the code to look like this but I know I'm missing a few key things plus this obviously doesn't mean the clicked event:

JS

init: function(){

            var _this = this,
                slides_li = _this.els.slides.children(),
                large = 260,
                small = 60;

            // Hover
            slides_li.on('mouseover', function(){
                $(this).stop(true, true).animate({ opacity: 1 }, 300);
            }).on('mouseout', function(){
                $(this).stop(true, true).animate({ opacity: .8 }, 300)
            });

            // Click handlers
            slides_li.on('click', function(){
                toggle_slides(); //pass in this?
            });                 
        },
        toggle_slides: function(){ // add argument for this?
               $('.active', _this.els.slides).not(this).animate({
                    width: small
                }, 300).removeClass('active');
                // animate the clicked one

                if ( !$(this).hasClass('active') ){
                    $(this).animate({
                        width: large
                    }, 300).addClass('active');
                }                
        }

Can anyone offer some advice on how to make this work?

styler
  • 15,779
  • 23
  • 81
  • 135
  • ive moved the content of slider_li into the method toggle_slides and now want to call it? – styler Jun 13 '12 at 09:10
  • init is part of the object bep so im calling bep.init(); to initiate the object – styler Jun 13 '12 at 09:11
  • So what prevents you from doing `bep.toggle_slides()`? – PeeHaa Jun 13 '12 at 09:13
  • when i do this i get Uncaught ReferenceError: _this is not defined – styler Jun 13 '12 at 09:16
  • here is a fiddle: http://jsfiddle.net/E5GSM/10/ – styler Jun 13 '12 at 09:16
  • Ofc you get that. You Didn't define `var _this = this` in that method. – PeeHaa Jun 13 '12 at 09:17
  • Also check out: http://stackoverflow.com/questions/500431/javascript-variable-scope – PeeHaa Jun 13 '12 at 09:21
  • i still get an error though: Cannot read property 'defaultView' of undefined http://jsfiddle.net/E5GSM/11/ – styler Jun 13 '12 at 09:21
  • I see no use for _this at all in your code above. You can use this directly in both functions – primavera133 Jun 13 '12 at 09:26
  • Try this one - http://jsfiddle.net/E5GSM/12/ - if it works the way you expect, I'll write up an answer explaining what I did and why... – BrendanMcK Jun 13 '12 at 09:32
  • @BrendanMcK this code seems to be the same as what I already have and doesnt use the toggle_slide() ? PeeHaa yeah I read the post but still slightly unsure about what I have to do to pass the clicked event to a method – styler Jun 13 '12 at 09:37
  • @styler - *slaps head* - totally forgot to hit the jsfiddle update button: this one should have the updates: http://jsfiddle.net/E5GSM/13/ – BrendanMcK Jun 13 '12 at 09:40
  • @BrendanMcK yeah this works and this is what I kind of thought had to happen although I wasn't sure if this was the right thing to do! If you could give me any further explanation that would be great. Currently trying to convert to doing things the object way – styler Jun 13 '12 at 09:47

1 Answers1

2

The problem is the ever-context-dependent value of 'this'.

First, a note about how the original code worked:

When init() is originally called, this refers to the parent object (gep). But within the click event handler, this refers to the clicked element. So that you can still access the parent within the click handler, you capture the 'parent value of this' into _this, so you can use it for later - and everything works fine.

But when you move the code in the handler into a separate method, quite a few things change:

  • First, small, large and other variables are no longer in the local scope, so have to be either redefined or imported as parameters.

  • this now refers to the parent element again, because the method now executing is not the event handler, but a method on the parent element. So references to _this in the old code can just use this in the new code.

  • Finally, in the old code, in the event handler, this referred to the element that was clicked. But (see above) in the new method, this means something different: the 'parent' object. So we need a parameter to capture that clicked element - I've passed it as an el parameter, and references to this in the old code change accordingly.

So the real thing to watch for is: what object does this code belong to? If you move code belonging to one object to another - eg. from an event handler on one object to a method on another object - you'll likely need to rework/rename any this or this-like variables as appropriate.

Annotated copy of the updated code follows, also available as a jsfiddle:

...
        // Click handlers
        slides_li.on('click', function(){
            // pass 'this' - the clicked element - as the el param; also small and large.
            gep.toggle_slides(this, small, large);
        });                 
    },

    toggle_slides: function(el, small, large){
           // '_this' in old code in handler replaced with 'this';
           // 'this' in handler code replaced with 'el' here
           $('.active', this.els.slides).not(el).animate({
                width: small
            }, 300).removeClass('active');
            // animate the clicked one

            // 'this' in handler code replaced with 'el'here
            if ( !$(el).hasClass('active') ){
                $(el).animate({
                    width: large
                }, 300).addClass('active');
            }                
    }
BrendanMcK
  • 14,252
  • 45
  • 54