Breaking jQuery/JS code off into functions from .click()

Question

I am using a JQuery cookie library to hide/show elements and then remember the status when the page is reloaded. The working code looks for elements with id #num0 and toggles the next element, and it looks like this:

if(Cookies('divShown0') == 'true') {  //defaults to hidden
    $('#num0').next().show(); // Show if cookie there and 'true'
}
$('#num0').click(function() {
  $(this).next().toggle();
  if(Cookies('divShown0') == 'true') {
    Cookies.set('divShown0', 'false'); // Remember it was hidden
  }
  else {
    Cookies.set('divShown0', 'true');  // Remember it was shown
  }
});

I have multiple of these, each identified by a different #num and stored as a different divShown number. So far I have just added a new code block with new numbers, but obviously this takes up a lot of space. I put the first if() statement into a for() loop, no problem. The second part I broke into a function, toggleShown(num), so I can call $('#num0').click(toggleShown(0));, but this is not working. I suspect that something about the $(this) element isn't properly identifying the next element, but I really don't know.

function toggleShown(num)
  {
   $(this).next().toggle();
   if(Cookies('divShown' + num) == 'true') {
     Cookies.set('divShown' + num, 'false'); // Remember it was hidden
   }
   else {
     Cookies.set('divShown' + num, 'true');
   }
  }

I don't really do Javascript or JQuery, mostly RoR but I am trying to hack this together. Any thoughts on what I am screwing up?

Right, $(this) will only exist within the originalting function in the scope you are looking for it. The good news is you can either pass it in as a paramater to the toggleShown function, or find it in the DOM within the function by building off of the num param. — BReal14
– BReal14, Commented Sep 16, 2015 at 17:07

A.O. · Accepted Answer · 2015-09-16 17:09:36Z

1

You hunch is correct! $(this) inside of your function does not hold the reference to the original element, because you are in a new function with a new scope.

All you should have to do is change:

$(this).next().toggle();

to

$("#num" + num).next().toggle();

answered Sep 16, 2015 at 17:09

A.O.

3,7636 gold badges32 silver badges49 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

David Sampson Over a year ago

Hm so I tried changing that and it still isn't working, however now it appears to run the function once for each div on pageload, but not on clicks.

Jack A. · Accepted Answer · 2015-09-16 17:13:04Z

1

You need to add a handler for each DOM element anyway, so you can use something like this:

function addHandler(num) {
    $('#num' + num).click(function() {
        $(this).next().toggle();
        var cookieName = 'divShown' + num;
        if (Cookies(cookieName) == 'true') {
            Cookies.set(cookieName, 'false'); // Remember it was hidden
        }
        else {
            Cookies.set(cookieName, 'true');  // Remember it was shown
        }
    });
}

for (var num = 0; num < numDivs; ++num) {
    addHandler(num);
}

answered Sep 16, 2015 at 17:13

Jack A.

4,5531 gold badge28 silver badges38 bronze badges

4 Comments

T.J. Crowder Over a year ago

"You need to add a handler for each DOM element anyway..." Not really, they could do $("[id^=num]").click(toggleShown); just once.

A.O. Over a year ago

While you are technically correct @T.J.Crowder that just opens the door for all sorts of errors later (having other elements id starting with 'num') Why put that constraint on yourself, not naming anything else starting with num?

T.J. Crowder Over a year ago

@A.O.: Trivial to weed them out with filter. I'd rather not create unnecessary long-lived functions.

A.O. Over a year ago

Ah yes, especially as num grows higher and higher. Then you might want to consider adding a class to the all the #num* divs in order to grab them all. Much faster than ^=

Ozan · Accepted Answer · 2015-09-16 17:12:52Z

0

You can set the this argument of a function by calling the function through Function.prototype.call(). The first parameter passed will be assigned as this inside the function.

answered Sep 16, 2015 at 17:12

Ozan

3,7492 gold badges21 silver badges23 bronze badges

Comments

chrki · Accepted Answer · 2015-09-16 17:18:06Z

0

If you are going to have multiple similar divs, instead of just IDs, you could assign a class to all of them and let jQuery add click listeners to all of them at once.

You can then either read the div ID from the ID itself, or you could give each a data attribute.

HTML:

<button class="toggle" data-id="1">First</button>
<button class="toggle" data-id="2">Second</button>
<button class="toggle" data-id="3">Third</button>

<div id="div1">First</div>
<div id="div2">Second</div>
<div id="div3">Third</div>

Javascript:

$('.toggle').click(function(){
    var id = $(this).data('id');
    hideOrShow(id);
})

function hideOrShow(id){
    $('#div'+id).toggle();
}

answered Sep 16, 2015 at 17:18

chrki

6,3436 gold badges39 silver badges60 bronze badges

Comments

T.J. Crowder · Accepted Answer · 2015-09-16 17:32:49Z

The second part I broke into a function, toggleShown(num), so I can call $('#num0').click(toggleShown(0));, but this is not working.

You can do

$("#num0").click(toggleShown);

...for just one of them, or

$("[id^=num]").click(toggleShown);

...for all of them at once. (^= means "starts with", so that would be "all elements with an id attribute starting with num"). If you need to weed out ones with other ids (like number):

$("[id^=num]").filter(function() {
    return this.id.match(/^num\d+$/);
}).click(toggleShown);

Either way, you then keep using this within toggleShown, and get the index number from the end of this.id:

function toggleShown() {
    var divName = "divShown" + /\d+$/.exec(this.id)[0];
    $(this).next().toggle();
    if (Cookies(divName) == 'true') {
        Cookies.set(divName, 'false'); // Remember it was hidden
    } else {
        Cookies.set(divName, 'true');
    }
}

Live Example:

$("[id^=num]").click(toggleShown);
function toggleShown() {
  var divName = "divShown" + /\d+$/.exec(this.id)[0];
  $("#" + divName).toggleClass("on");
}

.on {
  color: blue;
}

<div>Click any of the "num"s, and its corresponding "divShown" toggles between blue and normal text</div>
<div id="num0">num0</div>
<div id="num1">num1</div>
<div id="num2">num2</div>
<div id="num3">num3</div>
<hr>
<div id="divShown0">divShown0</div>
<div id="divShown1">divShown1</div>
<div id="divShown2">divShown2</div>
<div id="divShown3">divShown3</div>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>

Alternately, you can use Function#bind or jQuery.proxy to bind the index to toggleShown, and then use e.currentTarget for this:

$("#num0").click(toggleShown.bind(null, "0"));

then:

function toggleShown(num, e) {
    $(e.currentTarget).next().toggle();
    if (Cookies('divShown' + num) == 'true') {
        Cookies.set('divShown' + num, 'false'); // Remember it was hidden
    } else {
        Cookies.set('divShown' + num, 'true');
    }
}

Collectives™ on Stack Overflow

Breaking jQuery/JS code off into functions from .click()

5 Answers 5

1 Comment

4 Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

1 Comment

4 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related