Can someone Identify wether this is good practice or needs improvement
document.querySelector('#search-input').addEventListener('input', function(event) {
var query = event.target.value;
//console.log(query);
var xhr = new XMLHttpRequest();
xhr.open('GET', '/admin-tasks/search?query=' + encodeURIComponent(query));
xhr.onload = function() {
if (xhr.status === 200) {
var tasks = JSON.parse(xhr.responseText);
//console.log(tasks);
// Hide all rows
var allRows = document.querySelectorAll('.task-row');
allRows.forEach(function(row) {
row.style.display = 'none';
});
// Show only rows that match fetched data
tasks.forEach(function(task, index) {
var taskRow = document.querySelectorAll('.task-row')[index];
if (taskRow) {
console.log(task.faculty_image)
taskRow.style.display = '';
taskRow.querySelector('.task-name-text').textContent = task.task_name;
taskRow.querySelector('.faculty-name img').src = task.faculty_image;
taskRow.querySelector('.faculty-name-text').textContent = task.faculty_name;
var dateCreatedElements = taskRow.querySelectorAll('.date-created');
dateCreatedElements[0].innerHTML = task.date_created_formatted + "<br>" + task.date_created_time;
var dueDateElements = taskRow.querySelectorAll('.due-date');
dueDateElements[0].innerHTML = task.due_date_formatted + "<br>" + task.due_date_time;
// Check if the due date is in the past and add text-danger
if (task.due_date_past) {
dueDateElements[0].classList.add('text-danger');
}
else {
dueDateElements[0].classList.remove('text-danger');
}
}
});
}
};
xhr.send();
});
I tried the developer tool and here what i got in performance, im not sure what this tells wether good enough or not
5 ms Loading
62 ms Scripting
128 ms Rendering
66 ms Painting
129 ms System
4788 ms Idle
5178 ms Total
2
Answers
I think you could win some in speed if you prepared your html into a string and then outputted it all at once to the DOM, if possible. Several small DOM changes will often take more time than one big one (since the browser has to recalculate the DOM for each change).
I do understand that you want to set different event handlers, but you could use delegated events for that:
https://javascript.info/event-delegation
On a slightly off-topic note: Consider stop using XMLHttpRequest for AJAX and replace with fetch. It’s not going to speed of your code, but is much easier to work with:
https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
Comments
Some ways that code can be improved by are:
forEach
ormap
and instead opt for using for-loop or while-loop. They do the same thing but are much faster. See Loop Optimization and Performance BenchmarksJS