-3

I'm generating a phone list from Active Directory records and I want to hide rows in my table that don't have an entry for ipPhone (column 5 of my table).

My Javascript is essentially like this:

table = document.getElementById("myTable1");
tr = table.getElementsByTagName("tr");
for (i = 0; i < tr.length; i++) {
  td = tr[i].getElementsByTagName("td");
  dbg = td[5].innerHTML; // <<< Error here. 
  if (dbg == "") {
    tr[i].style.display = "none";
  }
}
<table id="myTable1" border="1">
  <tr>
    <td>Martin</td>
    <td>Roger</td>
    <td>Operator</td>
    <td></td>
    <td>Office</td>
    <td id="ipPhone"></td>
    <td></td>
    <td>Ronan.Martina@mydomain.com</td>
    <td>martinro</td>
    <td></td>
  </tr>
</table>

Chrome's debugger is giving me: Uncaught TypeError: Cannot read properties of undefined (reading 'innerHTML') and none of the rows are hidden.

Can anyone spot the problem?

Phil
  • 157,677
  • 23
  • 242
  • 245
Transistor
  • 193
  • 12
  • 1
    You cannot have more than a single element in a page with `id="ipPhone"`. – connexo Feb 24 '22 at 23:24
  • getElementsByTagName returns an HTMLCollection. You need to convert it into an Array. – Bryan Elliott Feb 24 '22 at 23:30
  • 1
    @BryanElliott no you don't, `HTMLCollection` is iterable – Phil Feb 24 '22 at 23:31
  • `HTMLCollection`s don’t even need to be iterable when used like this; they just need to have a `length` property and have indexes in a way that makes sense, i.e. be an array-like object. – Sebastian Simon Feb 24 '22 at 23:34
  • 1
    The code in the question does NOT produce the error - perhaps "essentially like this" means that you have an error in your actual code that isn't present in the code you posted ... even the nested identical for loop with global `i` variable does not break anything (since the outer loop will only run once anyway) – Bravo Feb 24 '22 at 23:34
  • Aaaargh! Copy'n'paste error. I pasted in the loop twice. Sorry! Fixed. -3 votes already. – Transistor Feb 24 '22 at 23:36
  • 1
    still, the code in the question runs fine - not the best code - but no error – Bravo Feb 24 '22 at 23:37
  • The only way your code fails is if you have table rows with fewer than 6 cells. Right now, this question is not reproducible and should be closed – Phil Feb 24 '22 at 23:41
  • @Transistor Now that you’ve fixed it, time to try using your browser’s [debug capabilities](//ali-dev.medium.com/how-to-easily-debug-in-javascript-5bac70f94f1a). What do you believe does the error message mean? Is the error message really so confusing? _“Cannot read properties of undefined”_, so it’s reading _of_ something undefined. What do you believe _is_ the undefined value in `td[5].innerHTML`? Which value would you need to inspect to test your assumptions that `td[5]` exists? See [What is a debugger?](/q/25385173/4642212). Dev tools provide an **Elements** tab. Inspect your elements. – Sebastian Simon Feb 24 '22 at 23:41
  • @SebastianSimon, thanks. I'm an industrial automation engineer and while I manage to create very useful intranet pages I don't do it enough to get all that familiar with it. I have been using the Chrome debugging console already and can see that each row of the table has the full number of columns. To me that means that td[5] must exist. Am I mistaken? Time for bed in Ireland. – Transistor Feb 24 '22 at 23:54
  • 1
    @Transistor You could `console.log(td)`. Most browsers have a console that highlights where a logged element is in the DOM. – Sebastian Simon Feb 24 '22 at 23:59

2 Answers2

1

try that:

const myTable = document.getElementById('myTable1');


myTable
.querySelectorAll(' tr > td:nth-of-type(5):empty')
.forEach( td5 =>
  {
  td5.closest(('tr').style.display = 'none'; 
  }) 
Mister Jojo
  • 20,093
  • 6
  • 21
  • 40
0

You cannot have more than a single element in a page with id="ipPhone". This must be fixed.

Use a CSS class instead, and instead of wiggling with indexes, use a much more readable and easier way of looping:

for (const cell of document.querySelectorAll("#myTable1 td.ipPhone")) {
  cell.parentElement.hidden = !cell.textContent;
}

With selectors level 4 you will be able to achieve this using CSS only:

tr:has(td.ipPhone:empty) { display: none; }
connexo
  • 53,704
  • 14
  • 91
  • 128
  • you CAN have duplicate id, it just breaks some things, but nothing in the OP code will break – Bravo Feb 24 '22 at 23:29
  • Just noticed , OP seems to want to hide the row, not the cell – Phil Feb 24 '22 at 23:29
  • @Phil, Yeah, just noticed that too, edited my answer. – connexo Feb 24 '22 at 23:31
  • Nice update. I'd use `cell.textContent.trim().length === 0` – Phil Feb 24 '22 at 23:33
  • @Phil Since the table is generated, OP shouldn't have the necessity for trimming. And if they have, they should fix the generation so when it lands, it can be solved using selector level 4's `:has()` selector. – connexo Feb 24 '22 at 23:37
  • @connexo I know OP _says_ their table is _"like"_ that but I wouldn't put it past them to actually have `\n\t\t\t\t\n` – Phil Feb 24 '22 at 23:40
  • @Phil Just checked, you are correct. Yet my previous comment is my position on this. – connexo Feb 24 '22 at 23:45
  • @Phil Why? If there is none, the loop simply will run zero times. – connexo Feb 25 '22 at 00:00