How do I unobtrusivify this javascript?

Question

I pulled this simple javascript function for showing or hiding a div from here. The function is:

function ReverseDisplay(d) {
  if(document.getElementById(d).style.display == "none"){
    document.getElementById(d).style.display = "block"; 
  }else{ 
    document.getElementById(d).style.display = "none";
  }
}

This requires making a link like this:

<a href="javascript:ReverseDisplay('uniquename')">
   Click to show/hide.
</a>

It's my understanding that having the link call javascript like that is bad practice. I'd like to make the javascript unobtrusive following https://stackoverflow.com/a/688228/2063292. But, that template provides a way to make some javascript execute for any link with a specified ID (e.g. all links with id="test" will call some function). I need to have a way to allow any link to pass the name of a specific div to the function, as in the original example, but I don't know how to do it.

mplungjan · Accepted Answer · 2014-08-03T05:52:13.670

0

I would prefer the ID of the div to be in the hash of the link:

Live JavaScript Demo

<a class="reversible" href="#arbitrarydivId">
Click to show/hide.
</a>

using

function ReverseDisplay() {
  var divId=this.hash.substring(1), div=document.getElementById(divId);
  div.style.display = div.style.display=="none"?"block":"none"; 
  return false;
}

or

Live jQuery Demo

$(function() {
  $(".reversible").on("click",function(e) {
    e.preventDefault();
    $(this.hash).toggle();
  });
});

Older suggestions

window.onload=function() {
  var links = document.querySelectorAll(".reversible");
  for (var i=0;i<links.length;i++) {
    links[i].onclick=ReverseDisplay;
  }
}

using

<a class="reversible" href="#">
Click to show/hide.
</a>

To hide something else, try

function ReverseDisplay() {
  var divId=this.getAttribute("data-div"), div=document.getElementById(divId);
  div.style.display = div.style.display=="none"?"block":"none"; 
  return false;
}

using

<a class="reversible" data-div="arbitrarydivId" href="#">
Click to show/hide.
</a>

In jQuery the whole thing would be

$(function() {
  $(".reversible").on("click",function(e) {
    e.preventDefault();
    $("#"+$(this.data("div")).toggle();
  });
});

edited Aug 03 '14 at 05:52

answered Aug 03 '14 at 04:40

mplungjan

169,008
28
173
236

Sorry, I don't think I explained well enough. There are multiple "uniquename"s that may change over time. So "uniquename" can't be hard coded into the function. Any link needs to be able to tell any uniquely named div to disappear. The code that generates the page will know the names of all the divs, but more divs may be added over time. – Ben S. Aug 03 '14 at 04:46
So use the second version which uses a classname. PS: once you hide something on click of itself you cannot click it again. Please note I fixed some typos – mplungjan Aug 03 '14 at 05:10
I don't want the link to hide itself. The link needs to specify a div, by name, that will be reversibly hidden. Like this: – Ben S. Aug 03 '14 at 05:14
Ok, I think I have finalised all your options now – mplungjan Aug 03 '14 at 05:53
Note that the first raw js, no jquery approach risks a memory leak, since .onclick = (obj created in js) crosses the memory boundary between the HTML memory area and the JS memory area. All it takes is a closure in the js that points to an HTML tag and bam, circular reference, leak. http://javascript.info/tutorial/memory-leaks – Chris Moschini Aug 03 '14 at 06:00
So if I add `...=null;` to which function? it will be less prone to leaks in ancient browsers? I must admit I have coded like the above since 1996 and never noticed a problem – mplungjan Aug 03 '14 at 06:06
I used the option starting with "To hide something else, try". It works nicely. Thank you. – Ben S. Aug 03 '14 at 06:33

score 0 · Answer 2 · answered Aug 03 '14 at 05:26

Depends on how unobtrusive you want to be. You could do this (using jquery for shorthand purposes):

<a href="#" id="someUniqueId1">
Click to show/hide.
</a>

<a href="#" id="someUniqueId2">
Click to show/hide.
</a>

<script>
  $("#someUniqueId1").click(function(){ReverseDisplay("#DivICareAbout");});
  $("#someUniqueId2").click(function(){ReverseDisplay("#AnotherDivICareAbout");});
</script>

But then you need to specify each and every link in your javascript. So I would recommend being a little more obtrusive, not with JS but with the href. Like this:

HTML:

<div id="foo">I am foo</div>
<div id="bar">I am bar</div>

<a class="reverselink" href="foo">Click to show/hide.</a>
<a class="reverselink" href="bar">Click to show/hide.</a>

JS:

function ReverseDisplay(d) {
    if (document.getElementById(d).style.display == "none") {
        document.getElementById(d).style.display = "block";
    } else {
        document.getElementById(d).style.display = "none";
    }
}

$(function () {
    $(".reverselink").click(function(e){
        ReverseDisplay($(this).attr("href"));
        return false;
    });
});

See this fiddle: http://jsfiddle.net/V73uw/ (again, using jquery for shorthand. It's trivial to do with vanilla js too).

It is not a great idea to have the ID as a href without the hash — mplungjan, Aug 03 '14 at 05:43

Quentin Engles · Answer 3 · 2014-08-03T05:57:16.197

Use this javascript to do all that.

function processName(aName){
    //do work like hiding.
    console.log(aName);
}
var body = document.getElementsByTagName('body')[0];
body.addEventListener("click", function(event){
    if(event.target.nodeName === "A"){
        //do work with anchor element
        processName(event.target.dataset.div);
    }
    event.preventDefault();
});

Then use this html as your links.

<a href="#" data-div="aNamedThing">some link</a>

You can define your javascript for this once then all the links only need data-div to send a name to processName.

Only bad thing is data attributes are really new for html so it might, or might not be what you want.

By the way. This is actually similar to how KnockoutJS works. You might want to try that out.

How do I unobtrusivify this javascript?

3 Answers3