patternjavascriptMinor
jQuery File Tree Toggle
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
jsFiddle
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
Step 2: Cache jQuery objects. Don't write
Step 3: Don't use
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:
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
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.
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.