Allow sorting to succeed when a TR element has no TD elements.

[deverac]

I had a bug in my code where I was generating an HTML TABLE that had one row which had a TR element that did not have any TD child elements. When I tried sorting the table (by clicking on a TH element), the table rows would not sort because sortable.js threw an Error (due to a missing TD element). This PR allows the sort to succeed when a TR element has no child TD elements.

The failure to sort the table only occurred when run on a mobile browser (Firefox on Android). My desktop browser automatically added the missing TD elements to the (empty) TR element, so sortable.js was able to sort the the table without a problem.

Since the root cause of the problem was my code generating an invalid HTML TABLE, please feel free to reject this PR, if you wish.

An example of a (invalid HTML) table which can cause sortable.js to fail:

<table class='sortable'>
  <thead>
    <tr><th>Col1</th></tr>
  </thead>
  <tbody>
    <tr><td>aaa</td></tr>
    <tr></tr> <!-- TR has no child TD elements -->
    <tr><td>ccc</td></tr>
  </tbody>
</table>

[tofsjonas]

...let's go all in on readability while we're at it:

  function getValue(element: HTMLTableCellElement): string {
    if (element) {
      if (alt_sort && element.dataset.sortAlt) return element.dataset.sortAlt
      if (element.dataset.sort) return element.dataset.sort
      if (element.textContent) return element.textContent
    }
    return ''
  }

[tofsjonas]

I like it.

Could you add a test for it? 🙏

[deverac]

The code has been refactored per your comment and a test has been added. If you want more/other changes, please let me know.

FYI. I added a test, and it passes, but the test may become ineffective in the future. The browser used for testing (Chromium) appears to not correct invalid HTML. Specifically, when a TD element does not exist in the HTML source, Chromium does not add an empty TD element in order to generate a valid HTML table. This behavior is exactly what is needed for this test, but at some point in the future Chromium may/could be updated to automatically correct invalid HTML tables. If Chromium is updated to do so, then the test will still pass, but the test will not be testing what it claims to be testing. (And it will be impossible to write a test case to ensure that empty TR elements are handled correctly.)

[deverac]

I added a second test which verifies that a TR element with no TD element exists. If/When Chromium ever automatically corrects invalid HTML tables, the test will fail.

[tofsjonas]

Everything seems to work! 🥳

Before merging, can you do an

npm run pre-push

and then push?

[deverac]

I ran npm run pre-push, and there were no errors. The command built/generated ten other files (e.g. ./dist/sortable.js, ./docs/index.html). Do you want me to include the generated files in this PR?

[tofsjonas]

Yes please 😊

[deverac]

I've included the build artifacts in the PR, per your request. However, I must say that I think that not building the source yourself puts too much trust in me. You have asked me (a total stranger) to check-in the build artifacts that could be downloaded by tens/hundreds/thousands of other people to include in their projects. There is no telling what bad things I might have embedded/hidden in the minified code. I would not be offended at all if you excluded the build artifacts that I submitted.

[tofsjonas]

I appreciate your concern, but I still create the releases and publish to npm manually. So if anyone tries to sneak in malicious code I will catch it. (If I ever start doing that in the pipeline I will follow your advice)