Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I read here that you should avoid coding JavaScript inline (see item #1).

It sounds like a great idea; however, I'm not sure how to use js to select dynamic content without setting the id in the function call inline.

For example, I have a simple blog. The comments are hidden, but can be displayed by clicking an anchor (generated dynamically) that calls a JavaScript function to toggle the comments for that specific post:

$commentsButton = "<a name='comments" . $post->id . "' href='#comments" . $post->id . "'> comments (" . count($comments) . ")</a>";

toggleComments() function:

function toggleComments(id) {
    $('#' + id + ' .commentsContainer').toggle(500);
}

How do you keep from doing this kind of thing? Is it always appropriate to avoid inline JavaScript? Or is this a case where it's necessary?

Update:

I ended up using this:

$('.commentButton').each(function() {
    $(this).click(function() {
        $(this).parents('.post').find('.commentsContainer').toggle(500);
    });
});

with this:

$commentButton = "<a class='commentButton'> comments (" . count($comments) . ")</a>";
share|improve this question
    
Your code could be shortened to $('.commentButton').click(...). There is no need to use each just to call another jQuery function. –  kevin cline Mar 8 '12 at 22:46
    
Very similar: programmers.stackexchange.com/questions/126671/… –  user16764 Mar 9 '12 at 7:33
    
I can't believe nobody recommended event delegation on this one. –  Erik Reppen Nov 28 '12 at 6:54
add comment

3 Answers 3

up vote 4 down vote accepted

How do you keep from doing this kind of thing?

In browser based javascript, this is set to the DOM element that triggered the event in the event handler. You can take advantage of that to re-use a function for any DOM element.

function toggleComments() {
    $(this).find('.commentsContainer').toggle(500);
}

Then you don't have to emit the ID into the javascript.

share|improve this answer
    
For other readers, since this example looks like JQuery, in Prototype you would use $$('.commentsContainer') to get a list of DOM elements –  Izkata Nov 10 '11 at 21:21
add comment

Inline javascript is the devil. Never use it, there is never an excuse for breaking seperations of concerns other then "I'm a bad developer".

In this case you just want to bind a click handler to all your toggle comment anchors that hides the comment container.

var as = document.getElementsByClassName("toggleComments");
[].forEach.call(as, function (a) {
  var container = a.getElementsByClassName("commentsContainer")[0];
  a.addEventListener("click", function (ev) {
    container.classList.toggle("hidden");
  });
}); 

Rather then dynamically creating javascript inline just attach a handler to all toggle comments links.

You will have to add a class to all your toggle comments anchors though.

Legacy platform support: Yes standards compliant code doesn't run on legacy platforms. Use the DOM shim and the ES5 shim. Problem solved.

share|improve this answer
2  
I'm not sure that first part ("I'm a bad developer") is a very constructive/professional way to convey your expertise. –  user25791 Nov 10 '11 at 15:53
3  
Hmmm...I'd argue that you should never use code that breaks on commonly used versions of IE (unless you just want to use the excuse, "I'm a bad developer"). But that's just me. –  Stargazer712 Nov 10 '11 at 17:34
2  
@Stargazer712 I would argue you should stop coding the lowest denominator and use shims to emulate standard compliance. Besides it's an example piece of code, my examples are standards compliant. If he wants to use support legacy platforms then that's his problem, not mine. –  Raynos Nov 11 '11 at 0:24
1  
@Raynos, one of the reasons I enjoy jQuery. Code however you want--just don't bring that attitude to a job interview. –  Stargazer712 Nov 11 '11 at 16:52
1  
@Raynos: +1 for the honesty of "I'm a bad developer" that nobody else wants to accept. Too many people are thin-skinned these days... –  insta Mar 8 '12 at 16:01
show 5 more comments

The solution you ended up with is a good one. In truth the only place you should have Javascript be generated in a server side template like you were talking about is for exporting data from the server to the browser. So for example code like this is a pretty reasonable thing to do, but not actual logic

window.countries = <?php echo json_encode($countries); ?>;
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.