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

Member list reveals member information on click (#1)

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

Problem

What I'm doing?

I'm creating a member-list where initially only the names are visible. Clicking the names reveals the member information. This is done with jQuery by adding/removing classes.

I left out the CSS with the transitions for this review.

HTML:


    
        member name
        
            
        
    
    
        member name
        
            
        
    
    
        member name
        
            
        
    


jQuery:


if (!window.jQuery) { document.write(''); }

    $(document).ready(function(){
        var blContent = $(".block-list__content");

        // If JS is disabled, the member information is visible by default
        $(blContent).addClass("hidden");

        $(".block-list__link").click(function(e) {
            // Prevent fragment identifier being added to the URL
            e.preventDefault();

            // Save the `block-list__content` which is a sibling of the clicked `block-list__link`
            var blCurrentContent = $(this).siblings(".block-list__content");
            if ($(blCurrentContent).hasClass("hidden")) {
                // Remove `visible` class from all `block-list__content`'s and add `hidden` class
                $(blContent).removeClass("visible").addClass("hidden");

                // Remove the `hidden` class from the current `block-list__content` and add `visible` class
                $(blCurrentContent).removeClass("hidden").addClass("visible");
            } else {
                // If `block-list__link` is clicked again, add `hidden` class
                $(blCurrentContent).removeClass("visible").addClass("hidden");
            }
        });
    });

Solution

You're double-wrapping elements in jQuery. blContent and blCurrentContent are already jQuery objects, so doing $(blContent) isn't necessary. It's common practice to prefix jQuery collections with a dollar sign $.

Other than that your code looks fine, but you could use toggleClass and exclude the current sibling element from the previous collection:

var $blContent = $('.block-list__content').addClass('hidden');

$('.block-list__link').click(function(e) {
  e.preventDefault();
  var $content = $(this)
    .siblings('.block-list__content')
    .toggleClass('hidden visible');
  $blContent
    .not($content)
    .removeClass('visible')
    .addClass('hidden');
});


  • Your code: http://jsbin.com/duqeb/1/edit



  • Updated code: http://jsbin.com/geboq/4/edit

Code Snippets

var $blContent = $('.block-list__content').addClass('hidden');

$('.block-list__link').click(function(e) {
  e.preventDefault();
  var $content = $(this)
    .siblings('.block-list__content')
    .toggleClass('hidden visible');
  $blContent
    .not($content)
    .removeClass('visible')
    .addClass('hidden');
});

Context

StackExchange Code Review Q#41207, answer score: 6

Revisions (0)

No revisions yet.