Search for webmon

Want to share your OpenWrt / Gargoyle knowledge? Implemented a new feature? Let us know here.

Moderator: Moderators

Lantis
Moderator
Posts: 7063
Joined: Mon Jan 05, 2015 5:33 am
Location: Australia

Re: Search for webmon

Post by Lantis »

I’ll read it properly and let you know. I can tell you right off the start that I loathe “arrow functions” and think they’re an abomination. Maybe I’m just getting old :roll: but if ChatGPT thinks those are an improvement it has rocks in its head :P

I think ChatGPT can give you a good scaffold but it often misses nuances in code and makes weird assumptions. I think it’s fantastic for quickly saying “what’s the syntax for inserting an element into an array at a given position?” and then you can write your code without having to go look it up. Again, maybe I’m just getting old and forgetful.
https://lantisproject.com/downloads/gargoylebuilds for the latest releases
Please be respectful when posting. I do this in my free time on a volunteer basis.

ispyisail
Moderator
Posts: 5212
Joined: Mon Apr 06, 2009 3:15 am
Location: New Zealand

Re: Search for webmon

Post by ispyisail »

Had a bit of a laugh ......

Tricky to explain the humour to my wife though :)
.......and more modern JavaScript code

ispyisail
Moderator
Posts: 5212
Joined: Mon Apr 06, 2009 3:15 am
Location: New Zealand

Re: Search for webmon

Post by ispyisail »

What is Arrow Function?
An Arrow Function in JavaScript, introduced in ES6, offers a concise syntax for defining function expressions using =>. It maintains lexical `this` binding and provides shorter, more readable code compared to traditional functions. Arrow functions enhance code structure by simplifying function syntax without sacrificing functionality or clarity.

Arrow functions are anonymous functions i.e. functions without a name but they are often assigned to any variable. They are also called Lambda Functions.

fifonik
Posts: 165
Joined: Fri Dec 02, 2016 3:52 am
Location: Brisbane, AU

Re: Search for webmon

Post by fifonik »

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
Last edited by fifonik on Mon Jul 22, 2024 6:31 am, edited 1 time in total.

ispyisail
Moderator
Posts: 5212
Joined: Mon Apr 06, 2009 3:15 am
Location: New Zealand

Re: Search for webmon

Post by ispyisail »

I would not approve the code :-P
Me neither :)

ChatGPT is not as good as Pro's yet :)

Lantis
Moderator
Posts: 7063
Joined: Mon Jan 05, 2015 5:33 am
Location: Australia

Re: Search for webmon

Post by Lantis »

I've implemented this under https://github.com/ericpaulbishop/gargo ... 444698bd10
It is your code with minor modifications.

I added a 1 second debounce to the interface. It's (in my opinion) the best length tradeoff for responsive UI vs not firing the function on every keypress.
It does fire every ~10 seconds when the underlying table refreshes. That behaviour is only noticeable if you're watching for it and I don't think that is worth doing anything to.

Please let me know if you see any issues. It worked fine in my brief testing.
Thanks again for your contribution!
https://lantisproject.com/downloads/gargoylebuilds for the latest releases
Please be respectful when posting. I do this in my free time on a volunteer basis.

fifonik
Posts: 165
Joined: Fri Dec 02, 2016 3:52 am
Location: Brisbane, AU

Re: Search for webmon

Post by fifonik »

Thanks a lot.

I've noticed that you did not remove 'row' class from preceding select dropdown (I mentioned that in html comment after the select field).
It does not look right in this case as the select and input are overlapping (I'm checking with default theme).

As for the debounce interval, I'm usually going for lower value (400 or so), but it does not matter, 1000 will do.

Post Reply