Setting TabIndex efficiently

Question

Is there a more efficient way to write this? Seems redundant to me:

    var leaves = $(".leaf");
    var trees = $(".tree");

    $(".my-class").on("click", function (e) {
        if ($(e.target).hasClass("leaf")) {
            trees.each(function() {
                $(this).attr("tabIndex", -1);
            });
            leaves.each(function() {
                $(this).attr("tabIndex", 1);
            });
        } else {
            trees.each(function() {
                $(this).attr("tabIndex", 1);
            });
            leaves.each(function() {
                $(this).attr("tabIndex", -1);
            });
        }
    });

200_success · Accepted Answer · 2014-03-24 07:08:03Z

3

.attr() can work on all jQuery matched elements at once.

Assuming that performance isn't a concern, I think that the clearest way would be to "reset" all nodes to have a tabIndex of -1 first.

var leaves = $(".leaf");
var trees = $(".tree");

$(".my-class").on("click", function(e) {
    leaves.add(trees).attr("tabIndex", -1);

    var selected = $(e.target).hasClass("leaf") ? leaves : trees;
    selected.attr("tabIndex", 1);
});

edited Mar 24, 2014 at 7:08

answered Mar 22, 2014 at 8:08

200_success

146k22 gold badges191 silver badges481 bronze badges

\$\begingroup\$ Apologies. I made a typo --the correction is above. The conditionals are actually different. \$\endgroup\$

hama_hama
– hama_hama

2014-03-24 04:27:42 +00:00
Commented Mar 24, 2014 at 4:27

Add a comment |

user1994380 · Accepted Answer · 2014-03-30 15:47:32Z

if ($(e.target).hasClass("leaf")) {
    // do x
} else {
    // do x but in a different way
}

This part makes it harder to maintain. If you change one you must change the other.

$(this).attr("tabIndex", -1);

You do not need jQuery for this. Try this.setAttribute("tabIndex", -1) or even this.tabIndex = -1.

Also, I see some problems with this code:

Why are you checking the event's target in $().on("click")? Would the event's target not be this? Maybe you should change it to $(".leaf.my-class, .tree.my-class")?
What if a user clicks on an element besides .leaf or .tree? The code suggests that you expect one or the other. Should $(".my-class") be changed to $(".leaf, .tree")?
What if you add leaves? You will need to update leaves and trees if new leaves/trees are created.

Here is my proposed solution...

var $leaves = $(".leaf"), $trees = $(".tree"), tabIndexLeaves = -1, tabIndexTrees = 1;
$(".my-class").on("click", function (e) {
    var isLeaf = $(e.target).hasClass("leaf");
    if ((isLeaf && tabIndexLeaves !== 1) || (!isLeaf && tabIndexLeaves === -1)) {
        // Invert the tabIndex
        tabIndexLeaves = -tabIndexLeaves;
        tabIndexTrees = -tabIndexTrees;
        // Update all the leaves' and trees' tabIndexes
        $leaves.attr("tabIndex", "" + tabIndexLeaves);
        $trees.attr("tabIndex", "" + tabIndexTrees);
    }
});

Stack Exchange Network

Setting TabIndex efficiently

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Setting TabIndex efficiently

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions