snippetjavascriptMinor
How can I improve performance of my selecting code
Viewed 0 times
canimproveperformanceselectinghowcode
Problem
I have a page with a list of items. Items have several actions assigned to them. (see screenshot).
One may choose to directly click on an icon next to each row or check a checkbox on the left hand side and click action above the list.
The issue is that after clicking an item OR checking a checkbox of several items and then clicking an action there is a lag (a second or so). Imagine having 100 rows or more.
Script:
Sample HTML of one row:
One may choose to directly click on an icon next to each row or check a checkbox on the left hand side and click action above the list.
The issue is that after clicking an item OR checking a checkbox of several items and then clicking an action there is a lag (a second or so). Imagine having 100 rows or more.
Script:
$("a.action:not(.remove)").click(function (e) { // .remove => do not execute on download tasks page
var selected = new Array(); // array to hold selections or clicks
var obj = $(this), objData = obj.data();
e.preventDefault();
if (!obj.hasClass('disablelink')) {
if (objData.ajaxMachineid) {
selected.push(objData.ajaxMachineid);
} else {
$("input.checkbox:checkbox:checked:not(.checkall)").each(function () {
var checkbox = $(this), machineId = checkbox.val(), packageId = objData.ajaxPackageid.removeSpecialChars().toUpperCase(), operation = objData.ajaxType;
var $row = $("#" + machineId + packageId.removeSpecialChars().toUpperCase());
if ($row.length) {
$row.has("a[data-ajax-type=" + operation + "]:not(.hide)").length ? selected.push(machineId) : $(this).attr('checked', false);
}
});
}
if (selected.length > 0) {
// do something more here blah blah blah
};
}
$("input.checkall").attr("checked", false);
});Sample HTML of one row:
GD009000246
PCGames
Up
10.48.1.236
View
Install
Remove
Solution
So pretty much everything will make a insignificant effect apart from the section where you are looping through each of the checkboxes. See my modifications and notes below.
The obvious thing to do on top of my suggestions above is to cache the checkbox list, this means that every time it enters that section, the app will no longer need to go through every element in the DOM looking for all the checkboxes. Doing this will probably have the most meaningful impact on performance.
$("a.action:not(.remove)").click(function (e) { // .remove => do not execute on download tasks page
var selected = new Array(); // array to hold selections or clicks
var obj = $(this),
objData = obj.data(),
// Moved these up here as they are not specific to the checkboxes
packageId = objData.ajaxPackageid.removeSpecialChars().toUpperCase(),
operation = objData.ajaxType;
e.preventDefault();
if (!obj.hasClass('disablelink')) {
if (objData.ajaxMachineid) {
selected.push(objData.ajaxMachineid);
} else {
// This seems a little convoluted to me, normally you would
// use [type=checkbox]
$("input[type=checkbox]:checked:not(.checkall)").each(function () {
var checkbox = $(this),
machineId = checkbox.val();
// No need to call .removeSpecialChars().toUpperCase() on packageId
var $row = $("#" + machineId + packageId);
if ($row.length) {
// Not 100% whether you need .length here
// It could also be beneficial to add a class to the row instead
// of calling this complex selector
$row.has("a[data-ajax-type=" + operation + "]:not(.hide)").length
? selected.push(machineId)
: $(this).attr('checked', false);
}
});
}
if (selected.length > 0) {
// do something more here blah blah blah
};
}
$("input.checkall").attr("checked", false);
});The obvious thing to do on top of my suggestions above is to cache the checkbox list, this means that every time it enters that section, the app will no longer need to go through every element in the DOM looking for all the checkboxes. Doing this will probably have the most meaningful impact on performance.
// in document ready
var checkboxes = $('input[type=checkbox]');
// When you need to find the checked check boxes
var checked = checkboxes.find(':checked:not(.checkall)');Code Snippets
$("a.action:not(.remove)").click(function (e) { // .remove => do not execute on download tasks page
var selected = new Array(); // array to hold selections or clicks
var obj = $(this),
objData = obj.data(),
// Moved these up here as they are not specific to the checkboxes
packageId = objData.ajaxPackageid.removeSpecialChars().toUpperCase(),
operation = objData.ajaxType;
e.preventDefault();
if (!obj.hasClass('disablelink')) {
if (objData.ajaxMachineid) {
selected.push(objData.ajaxMachineid);
} else {
// This seems a little convoluted to me, normally you would
// use [type=checkbox]
$("input[type=checkbox]:checked:not(.checkall)").each(function () {
var checkbox = $(this),
machineId = checkbox.val();
// No need to call .removeSpecialChars().toUpperCase() on packageId
var $row = $("#" + machineId + packageId);
if ($row.length) {
// Not 100% whether you need .length here
// It could also be beneficial to add a class to the row instead
// of calling this complex selector
$row.has("a[data-ajax-type=" + operation + "]:not(.hide)").length
? selected.push(machineId)
: $(this).attr('checked', false);
}
});
}
if (selected.length > 0) {
// do something more here blah blah blah
};
}
$("input.checkall").attr("checked", false);
});// in document ready
var checkboxes = $('input[type=checkbox]');
// When you need to find the checked check boxes
var checked = checkboxes.find(':checked:not(.checkall)');Context
StackExchange Code Review Q#23524, answer score: 2
Revisions (0)
No revisions yet.