HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinorCanonical

jQuery - Drop Down QA

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
dropjquerydown

Problem

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:


        First Question
        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.
        
    
    
        Second Question
        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....
        
    


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

Thanks in advance.

Solution

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


    
         First Question
         
    
    
         Second Question
         
    
    
         Third Question
         
    


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.

Code Snippets

<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>
//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);
    });
});

Context

StackExchange Code Review Q#24985, answer score: 2

Revisions (0)

No revisions yet.