4
\$\begingroup\$

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

<!doctype html>
<html>
    <head>
        <meta charset="UTF-8" />
        <meta name="viewport" content="width=device-width" />
        <title>Title</title>
        <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
        <style>
            li {
                cursor: pointer;
                line-height: 20px;
                list-style: none;
            }
            li ul {
                display: none;    
            }
            .active {

            }
        </style>
    </head>
    <body>
        <div class="tree-wrapper">
            <ul class="tree">
                <li>
                    <a href="#">Animals</a>
                    <ul>
                        <li>
                            <a href="#">Birds</a>
                        </li>
                        <li>
                            <a href="#">Mammals</a>
                            <ul>
                                <li>Elephant</li>
                                <li>Mouse</li>
                            </ul>
                        </li>
                        <li>
                            <a href="#">Reptiles</a>
                        </li>
                    </ul>
                </li>
                <li><a href="#">Plants
                        <ul>
                            <li>
                                <a href="#">Flowers</a>
                                <ul>
                                    <li><a href="#">Rose</a></li>
                                    <li><a href="#">Tulip</a></li>
                                </ul>
                            </li>
                            <li><a href="#">Trees</a></li>
                        </ul>
                    </a>
                </li>
            </ul>
        </div>
        <script>
            var folderToggle = $(".tree li > a");   

            $(function () {
                folderToggle.click(function(e) {
                    e.preventDefault();
                    $(this).find("+ ul").toggle("slow");
                    $(this).toggleClass("active");
                });
            });
        </script>
    </body>
</html>
\$\endgroup\$
3
\$\begingroup\$

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.

\$\endgroup\$
  • 2
    \$\begingroup\$ Its also a slight performance increase to use 'return false' rather than 'e.preventDefault' (jsperf showed a 6% benefit, not earth shattering, but still) \$\endgroup\$ – atmd May 29 '13 at 16:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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