Stark
Stark

Reputation: 13

Javascript getting id attribute of HTMLTableCellElement

I am trying to build a Minesweeper game, making the game grid out of table. I'm stuck with getting the id I set for specific cell, when I click on it. Here is the code to make the table.

var table = document.createElement("table");
for (var k = 0; k < twoDArray.length; k++) {
    var row = document.createElement("tr");
    for (var l = 0; l < twoDArray[k].length; l++) {
        var cell = document.createElement("td");
        cell.textContent = twoDArray[k][l];
        cell.setAttribute("id", "" + k + "," + l + "");
        cell.setAttribute("onclick", 'clickHandler('+cell+');');
        row.appendChild(cell);
    }
    table.appendChild(row);
}
return document.getElementById("tableDiv").appendChild(table);

And here is the clickHandler() function:

function clickHandler(cell) {
    console.log(cell.getAttribute("id"));
}

Please, what I am doing wrong? Getting Uncaught SyntaxError: Unexpected identifier error.

Upvotes: 0

Views: 3146

Answers (2)

T.J. Crowder
T.J. Crowder

Reputation: 1075755

You can't use string concatenation meaningfully with a DOM element, so this is incorrect:

cell.setAttribute("onclick", 'clickHandler('+cell+');');

Moreover, there's no reason whatsoever to use a string for this. If you want to use "simplistic" event handling, assign a function to the onclick property:

cell.onclick = clickHandler;

To use modern event handling, use addEventListener:

cell.addEventListener(clickHandler);

Sadly, old IE (or IE in "pretend to be old and useless" mode, aka "compatibility mode) doesn't have addEventListener and you have to use attachEvent instead; see this answer if you need to support old IE / useless IE.

(Another alternative below.)

In either case, when the handler is called, the cell you added the handler to is available via this, so:

function clickHandler() {
    console.log(this.id);
}

It's also available via the currentTarget property of the event object you receive, so this also works:

function clickHandler(e) {
    console.log(e.currentTarget.id);
}

(There's also e.target, which is where the event originated — so for instance, if you had <td id="foo"><span>click me</span></td> and hooked click on the td, if you click the span then this and e.currentTarget refer to the td and e.target refers to the span.)

But another alternative for you is to not hook click on each and every td, but instead hook it on the table and then work from e.target:

table.addEventListener("click", clickHandler);

and

function clickHandler(e) {
    var td = e.target;
    while (td.tagName != "TD") {
        td = td.parentNode;
        if (!td || td == this) {
            return; // Click didn't pass through a td
        }
    }
    console.log(td.id);
}

Side note: As you can see above, id is a reflected property for the id attribute, so you can set the id using the property as well:

cell.id = k + "," + l;

Also note that the "" + and + "" on the original version of that line are completely unnecesary; if you use + on a number and string, it does string concatenation, not addition, regardless of the order of the operands.

Upvotes: 2

Quentin
Quentin

Reputation: 944498

cell.setAttribute("onclick", 'clickHandler('+cell+');');

You are converting cell (which is an HTML element object) into a string (which will be "[object Object]" so your resulting JS will look like this:

clickHandler([object Object]);

… which is JS with a syntax error in it.

You should make use of the this keyword instead:

cell.setAttribute("onclick", 'clickHandler(this);');

giving

clickHandler(this);

When used in the context of a click event handler, this is the element that was clicked on.


That said, using JS to generate a string that will later be evaluated as JS is simply nasty. This is a simple case, but add any more complexity and it rapidly becomes a good source of bugs and security problems.

You should use addEventListener and assign a function instead.

cell.addEventListener("click", myFunction);

function myFunction(event) {
    clickHandler(this);
} 

(Rewriting clickHandler to simply use this is probably a better bet than creating a wrapper function though).


Note that table cells are not designed to be interactive controls. Using them as such will create problems for people who aren't using a pointing device to interact with their computer (such as many of those using a screen reader, keyboard or breath switch).

It is generally a better idea to put a button inside the cell and make that the click target.

Upvotes: 1

Related Questions