Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I’m making a family tree. The following script allows the user to expand branches of the tree. It works fine but there must be a better way.

Welcome feedback.

You can see it working here.

$(function(){
    $('.oc').prepend('<img src="icons/bullet_add.png" alt="more" width="16" height="16"/>')
    $("ul[id|=Nested]").hide();
    $("a[id|=trigger_Nested]").click(function() {
        var text = $(this).siblings("ul");
        if (text.is(':hidden')) {
            text.slideDown("fast");
            $(this).next("span").html('<img src="icons/bullet_take.png" alt="close" width="16" height="16" />');     
        } else {
            text.slideUp("fast");
            $(this).next("span").html('<img src="icons/bullet_add.png" alt="more" width="16" height="16" />');     
        }

        return false;
    });
});
share|improve this question
add comment

1 Answer

A couple notes:

  1. I would use classes instead of ids for the nested links. I don't know if it's faster to search the DOM for all elements with a specific class, but it looks cleaner and doesn't require you to create unique ids for each element:

    $('a.nested-trigger').click(...);
    
  2. I would also use classes for the icons, as it's less expensive than replacing DOM nodes:

    HTML:

    <span class="oc collpased"></span>
    

    CSS:

    .oc {
      position: relative;
      right: 10px;
      display: inline-block;
      width: 16px;
      height: 16px;
    }
    .collapsed {
      background: url(icons/bullet_add.png);
    }
    .expanded {
      background: url(icons/bullet_take.png);
    }
    

    JS:

    $(this).next("span").removeClass("collapsed").addClass("expanded");
    
  3. This is related more to the UX of your demo but I would make the icons themselves clickable as well, because they're big enough to look like buttons, and they overlap part of the node.

share|improve this answer
    
All very good points. I especially agree with seand's 3rd point. I tried to click the icons and was thinking the whole thing didn't work for a second. –  nickles80 Dec 18 '12 at 4:35
add comment

Your Answer

 
discard

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.