jQuery - Drop Down QA

Question

Here's a fiddle: http://jsfiddle.net/YF8cg/

Focussing entirely on the javascript and on the structure of the HTML (.qapair .question and .answer), how could I improve this system?

JS:

$(document).ready(function() {
 $('.question').click(function() {
 $(this).siblings('.answer').slideToggle(400); // Display the clicked answer
 $(this).parents('.QApair').siblings('.QApair').children('.answer').each( // Hide all the others
 function() {
 if($(this).is(':visible')) { // Detects if it's visible
 $(this).slideToggle(400); // If true then toggles it
 }
 });
 });
});

HTML:

<div class="qapair">
 <h4 class="question">First Question</h4>
 <div class="answer">Here would be lods of boring text, or some images, or some tables, or some fish, or some lets-pick-another-random-nouns, you get the picture. I just needed to fill space.
 </div>
 </div>
 <div class="qapair">
 <h4 class="question">Second Question</h4>
 <div class="answer">I've run out of anything to write. So keyboard spas in 5... 4... 3... 2... 1... agdkfdakgjajgjregafgi iej fujagijrfgj rhgjahngufg ughuafng uuagj u unhu up u dnfajng hfgnajgnurngr gugnjfngja uugf n ndnfaung urgjangjnfg. That was interesting....
 </div>
 </div>

I'm fairly new to jQuery, so any help would be appreciated.

Thanks in advance.

• Benjamin Gruenbaum: Cache your selectors for starters

Answer 1

HTML: The only change is that they are wrapped in a div id'ed container which will be used in event handling, explained below.

<div id="container">
 <div class="qapair">
 <h4 class="question">First Question</h4>
 <div class="answer"></div>
 </div>
 <div class="qapair">
 <h4 class="question">Second Question</h4>
 <div class="answer"></div>
 </div>
 <div class="qapair">
 <h4 class="question">Third Question</h4>
 <div class="answer"></div>
 </div>
</div>

JS: Explained in the comments

//shorthand for $(document).ready(fn) is $(fn)
$(function () {
 //container will be used for delegated event handling
 var container = $('#container'),
 //since questions remain static, it's best we reference ahead of time
 questions = $('.question',container);
 //take advantage of delegation which assigns one handler to the parent
 //for all events in it's descendant. Here, we place a handler on container
 //for all question instead of putting a handler per question
 container.on('click', '.question', function (event) {
 var question = $(this);
 //we use 'next' which directly gets the next sibling instead of siblings(selector). 
 //though internal implementations might be the same, but at least we avoid
 //writing that many selectors in our implementation
 question
 .next()
 .slideToggle(400);
 //using the referenced questions, we remove the currently clicked, pick
 //their answers that are currently visible and slide them up
 questions
 .not(question)
 .next(':visible')
 .slideUp(400);
 });
});

Here's running code. It's more of a readability and maintainability optimization rather than performance.

• Luke Ashford: Thanks. I learnt some intersting things from that, notable the delegation part. I'd upvote if I had enough rep, but for the time being I've just selected as answer.