skip to Main Content

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


  1. 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

    Login or Signup to reply.
  2. Comments

    Some ways that code can be improved by are:

    1. Use arrow functions instead of regular functions. They use less resources.
    2. Avoid querying multiple times for the same thing. Do it once and assign it to a constant variable.
    3. Avoid using shorthand functions like forEach or map and instead opt for using for-loop or while-loop. They do the same thing but are much faster. See Loop Optimization and Performance Benchmarks
    4. Utilize async/await. It makes code easy to read/debug and you avoid the multiple levels of thenable hell.
    5. An improvement that can be done is to create a html template row and to clone that template content for each new row that needs to be created.

    JS

    const searchHandler = async ( { target=null }={} ) => {
        if ( target == null || !target.value ) return;
    
        try {
            const query = `/admin-tasks/search?query=${encodeURIComponent( target.value )}`;
            const result = await fetch( query ).then( response => response.text() ) ?? '';
    
            const tasks = JSON.parse( result );
            const rows = document.querySelectorAll('.task-row');
    
            for ( const row of rows || [] ){
                row.setAttribute('hidden','');
            }
    
            for ( let i=0, il=tasks.length; i<il; i++ ){
                const row = rows[i];
                const task = tasks[i];
    
                if ( !row ) continue;
    
                row.removeAttribute('hidden');
                row.querySelector('.task-name-text').textContent = task.task_name;
                row.querySelector('.faculty-name img').src = task.faculty_image;
                row.querySelector('.faculty-name-text').textContent = task.faculty_name;
                row.querySelector('.date-created').innerHTML = `${task.date_created_formatted}<br>${task.date_created_time}`;
    
                const dueDateElement = row.querySelector('.due-date');
                dueDateElement.innerHTML = `${task.due_date_formatted}<br>${task.due_date_time}`;
    
                if ( task.due_date_past ) dueDateElement.classList.add('text-danger');
                else dueDateElement.classList.remove('text-danger');
            }
        } catch( error ) {
            console.error( error );
        }
    }
    const searchInput = document.querySelector('#search-input');
    searchInput.addEventListener( 'input', searchHandler );
    
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search