Skip to content

Conversation

alijsh
Copy link
Contributor

@alijsh alijsh commented Dec 21, 2023

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.

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.
@kyle-wannacott
Copy link
Owner

kyle-wannacott commented Dec 21, 2023

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

@kyle-wannacott kyle-wannacott merged commit 30c6cca into kyle-wannacott:master Dec 21, 2023
@alijsh
Copy link
Contributor Author

alijsh commented Dec 21, 2023

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

@kyle-wannacott
Copy link
Owner

@alijsh Yep, looks good to me.

@alijsh alijsh deleted the patch-1 branch December 21, 2023 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants