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

jQuery File Tree Toggle

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

Problem

I am making a web app that will have a "my files" area and to do so I am writing my own html and jQuery to control the opening and closing of each folder (or ul li). I want to make sure that this web app is as efficient as I can make it as it may have to deal with a large amount of hierarchy.

jsFiddle


    
        
        
        Title
        
        
            li {
                cursor: pointer;
                line-height: 20px;
                list-style: none;
            }
            li ul {
                display: none;    
            }
            .active {

            }
        
    
    
        
            
                
                    Animals
                    
                        
                            Birds
                        
                        
                            Mammals
                            
                                Elephant
                                Mouse
                            
                        
                        
                            Reptiles
                        
                    
                
                Plants
                        
                            
                                Flowers
                                
                                    Rose
                                    Tulip
                                
                            
                            Trees
                        
                    
                
            
        
        
            var folderToggle = $(".tree li > a");   

            $(function () {
                folderToggle.click(function(e) {
                    e.preventDefault();
                    $(this).find("+ ul").toggle("slow");
                    $(this).toggleClass("active");
                });
            });
        
    

Solution

Step 1: Fix your markup. You have the "plants"-list nested inside the link, which is invalid

Step 2: Fix your JS. You correctly add the click handler after the document's ready, but you set the folderToggle variable when the script's run, regardless of whether the document's been loaded. It still works because you have the script at the end of your page, but still. Keep everything inside the $(...) - it'll also keep the folderToggle var out of the global scope.

Step 2: Cache jQuery objects. Don't write $(this) twice.

Step 3: Don't use find() for this. While I can't speak to an exact performance difference, it's more precise to use next(), as the nested list will be the next element/sibling relative to the link.

$(function () {
    $(".tree li > a").click(function(e) {
        var link = $(this);
        e.preventDefault();
        link.next().toggle("slow");
        link.toggleClass("active");
    });
});


Step 4: If you're going to be adding more stuff to the tree via ajax or something, you'll probably want to rewrite the click handler to handle clicks for links added later:

$(".tree").on("click", "li > a", function (event) {
    ...
});


Step 5: You'll probably want to add a class to the "branch" links in the tree. Otherwise "leaf" links will also attempt to open a nested list, which doesn't make much sense, as there won't be one.

Step 6: Use OL (ordered list) unless your lists are actually "unordered". But if they're file lists, they're probably ordered somehow. Minor thing, but semantics matter.

All that said, there's nothing terribly inefficient in your code. If anything's going to lag, it's the toggle animation, but that can't really be sped up, as it's a single function call to jQuery. It's either there or not there.

Code Snippets

$(function () {
    $(".tree li > a").click(function(e) {
        var link = $(this);
        e.preventDefault();
        link.next().toggle("slow");
        link.toggleClass("active");
    });
});
$(".tree").on("click", "li > a", function (event) {
    ...
});

Context

StackExchange Code Review Q#26744, answer score: 3

Revisions (0)

No revisions yet.