LeeWannacott / table-sort-js

HTML table sorting with no dependencies.
https://cdn.jsdelivr.net/npm/table-sort-js@latest/table-sort.js
MIT License
76 stars 26 forks source link

Mark processed tables #121

Closed alijsh closed 10 months ago

alijsh commented 10 months ago

Make sure each table is processed once by adding the class name "table-processed". This prevents re-processing tables and attaching repeated arrows and click events in scenarios where new tables are dynamically added to a web page/application. Now, only new tables are made sortable.

LeeWannacott commented 10 months ago

@alijsh Thanks, this should help with when someone has the browser extension installed and visiting a site that is running table-sort-js like the documentation demo; (it currently has this problem if you have the extension installed).

I probably would of written it like this:

   if !(sortableTable.classList.contains("table-processed")) {
         sortableTable.classList.add("table-processed");
    } else {
        return;
    }

Not that it really matters in this instance, This is because else is a fall through for any condition that you might not have thought about:

or could of put it up further (this way it only adds one line):

  for (let table of getTagTable) {
    if (table.classList.contains("table-sort")) && !(table.classList.contains("table-processed")) {
      makeTableSortable(table);
    }
  }
 makeTableSortable(sortableTable){
     sortableTable.classList.add("table-processed");
     ....
}

Let me know if you want to make changes or want me to merge it as is.

Cheers.

alijsh commented 10 months ago

@LeeWannacott I think the second approach is better (checking "table-processed" and "table-sort" classes in the same condition and then, adding "table-processed" class to new tables). I made the changes. Please check it.

LeeWannacott commented 10 months ago

@alijsh Yep, looks good to me.