I've added comment that debounce should be added. I have not added it as I was not sure if the code will be used at all or not.
filterTableRows is worse then my implementation. It is always do some filtering even when searchTerm is empty.
Caching in dataset is sub-optimal as it is writing data into dataset on every cycle of the loop:
Code: Select all
row.dataset.host = host;
row.dataset.value = value;
Modern syntax -- well, it is not always available (I do not know what gargoyle targeting to so I have not used it by porpose) and it is not fastest quite often.
array.forEach was never be the fastest.
Examples:
https://dev.to/maafaishal/benchmarking- ... cenow-1jjg
https://leanylabs.com/blog/js-forEach-m ... or-for_of/
Many others on stack overflow etc.
Code: Select all
const rows = Array.from(tbody.rows);
on every filtering call -- no comments here.
Code: Select all
const input = document.getElementById(inputId);
if(!input) return;
const container = document.getElementById(containerId);
if(!container){
// No container for the table exists -- hide filter input
input.style.display='none';
return;
}
=>
Code: Select all
const input = document.getElementById(inputId);
const container = document.getElementById(containerId);
if (!input || !container) {
if (input) input.style.display = 'none';
return;
}
Less optimal.
There is no reason to find container element if there is no input element.
And for my opinion, new bit of code is harder to read because of the nested IFs.
Code: Select all
if(...condition...){
row.style.display = ''; // 'table-row' can also be used
row.className = visibleIndex++ % 2 ? 'even' : 'odd';
} else {
row.style.display = 'none';
}
=>
Code: Select all
row.style.display = isVisible ? '' : 'none';
row.className = isVisible ? (index % 2 ? 'even' : 'odd') : '';
Shorter, but less optimal. Second 'if' added with no reason.
Code: Select all
rows.forEach((row, index) => {
...
row.className = isVisible ? (index % 2 ? 'even' : 'odd') : '';
});
Bug here.
odd/even class should be calculated based on visible rows only.
Imagine you have 3 rows and with entering something in input field row 2 is filtered out. With the updated code row 1 and row 3 will have 'odd' class so the table will not be striped here.
Code: Select all
const value = row.dataset.value || (row.cells[2]?.querySelector('a')?.getAttribute('href') || '').split('://')[1]?.toUpperCase() || '';
Sub-optimal: querySelector is expensive and should not be used here as we know the structure of the cell.
Potential bug here: split(...)[1] -- technically it might be more then one '://' here (in query string if we decide to display it one day).
I would not approve the code :-P