Javascript for onclick function

Question

I'm trying to practice my scripting by making a Battleship game. As seen here.

I'm currently trying to make the board 2D. I was able to make a for-loop in order to make the board, however, due to testing purposes, I'm just trying to make the board, upon clicking a square, it turns red... But, the bottom square always lights up. I tried to debug it by making the c value null, but then it just stops working. I know it's not saving the c value properly, but I'm wondering how to fix this.

Do I have to make 100 squares by my self or can I actually have the script do it?

 maincansize = 400;
 document.getElementById("Main-Canvas").style.height = maincansize;
 document.getElementById("Main-Canvas").style.width = maincansize;
 document.getElementById("Main-Canvas").style.position = "relative";
 var ize = maincansize * .1;
 for (var a = 0; a < 10; a++) {
   for (var b = 0; b < 10; b++) {
     var c = document.createElement("div");
     var d = c;
     c.onclick = function() {
       myFunction()
     };
     function myFunction() {
       console.log("A square was clicked..." + c.style.top); d.style.backgroundColor = "red";
     }
     c.style.height = ize;
     c.style.width = ize;
     c.style.left = b * ize;
     c.style.top = a * ize;
     c.style.borderColor = "green";
     c.style.borderStyle = "outset";
     c.style.position = "absolute";
     console.log(ize);
     document.getElementById('Main-Canvas').appendChild(c);

   } //document.getElementById('Main-Canvas').innerHTML+="<br>";
}

#Main-Canvas {
  background-color: #DDDDDD;
}

<div>
  <div id="header"></div>
  <script src="HeaderScript.js"></script>
</div>
<div id="Main-Canvas" style="height:400;width:400;">

</div>

When you insert code, put for spaces before every line (Or select the whole code and press the { } symbol) — Bálint
– Bálint, Commented Jan 26, 2017 at 15:27
What you want to do is get the element that is actually triggering the click - stackoverflow.com/questions/1553661/… — Craicerjack
– Craicerjack, Commented Jan 26, 2017 at 15:35
just spent the past 10 minutes playing that battleship game! — luke_mclachlan
– luke_mclachlan, Commented Jan 26, 2017 at 15:37
can you take myFunction() out of for loop and to add element as input parameter to it, then try again? — K D
– K D, Commented Jan 26, 2017 at 15:39

Giladd · Accepted Answer · 2017-01-26 15:46:08Z

Here's your code with some fixes:

adding 'px' to style assignment
passing the clicked element to myFunction

var maincansize = 400;
document.getElementById("Main-Canvas").style.height = maincansize;
document.getElementById("Main-Canvas").style.width = maincansize;
document.getElementById("Main-Canvas").style.position = "relative";
var ize = maincansize * .1;

for (var a = 0; a < 10; a++) {
  for (var b = 0; b < 10; b++) {
    var c = document.createElement("div");
    c.onclick = function(ev) {
      myFunction(ev.currentTarget);
    };

    function myFunction(el) {
      console.log("A square was clicked...");
      el.style.backgroundColor = "red";
    }
    c.style.height = ize+'px';
    c.style.width = ize+'px';
    c.style.left = (b * ize)+'px';
    c.style.top = (a * ize)+'px';
    c.style.borderColor = "green";
    c.style.borderStyle = "outset";
    c.style.position = "absolute";
    document.getElementById('Main-Canvas').appendChild(c);
  }
}

#Main-Canvas {
  background-color: #DDDDDD;
}

<div id="Main-Canvas" style="height:400;width:400;">

</div>

hungerstar · Accepted Answer · 2017-01-26 16:01:46Z

Here's a solution with major revamps. Since you're using a set width for the container element of your board cells you can float the cells and they will wrap to the next line. Absolute positioning tends to be a bit of a bugger. If you want 10 items per row it's as easy as:

<container width> / <items per row> = <width>

Using document fragments is faster than appending each individual element one at a time to the actual DOM. Instead you append the elements to a document fragment that isn't a part of the DOM until you append it. This way you're doing a single insert for all the cells instead of 100.

I moved some of the styling to CSS, but could easily be moved back to JS if you really need to.

function onCellClick() {
  
  this.style.backgroundColor = 'red';
  
  console.log( 'selected' );
  
}

var main = document.getElementById( 'board' ),
    frag = document.createDocumentFragment(),
    i    = 0,
    len  = 100;

for ( ; i < len; i++ ) {

  div = document.createElement( 'div' );
  
  div.addEventListener( 'click', onCellClick, false );
  frag.appendChild( div );
  
}

main.appendChild( frag );

#board {
  width: 500px;
  height: 500px;
}
#board div {
  float: left;
  box-sizing: border-box;
  width: 50px;
  height: 50px;
  border: 1px solid #ccc;
}

<div id="board"></div>

Collectives™ on Stack Overflow

Javascript for onclick function

2 Answers 2

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related