3

I have a weird problem with my code (link or code below).

I want to create a simple game to practice basic Javascript.

The concept of this game is: There are 8 slots given (input). Fill all the slots with hex numbers (00 - FF). Press Check button and see how many values you've guessed. (The numbers, to compare your input with, are randomly generated using getRandomHexValues()).

In js file, function checkValues() generates 8 hex numbers; inserts in div#lotoResult 8 inputs (each holds one of the generated numbers). Each input in div#lotoResult has the border colored in red or green, depending whether the value was correctly guessed. Then it adds a p element showing how many errors there are.

The problem is: if I add the p element to the innerHTML of div#lotoResult, the values in inputs (generated by checkValues()) are not shown. If I do not add p, the elements are shown (but I need this p).

I want to understand what is the problem (I've tried, but I cannot). Do you have any suggestions?

function checkValues() {
  var inputs = document.getElementsByName("num");
  var inputOk = true;

  for (i = 0; i < inputs.length; ++i) {
    if (inputs[i].value == "") {
      inputOk = false;
      break;
    }
  }

  var lotoResult = document.getElementById("lotoResult");
  lotoResult.style.visibility = "visible";
  if (!inputOk) {
    lotoResult.style.color = "red";
    lotoResult.innerHTML = "!!! Not all slots are filled.";
  } else {
    var genValues = getRandomHexValues();
    for (i = 0; i < inputs.length; ++i) {
      lotoResult.innerHTML += "<input type=\"text\" pattern=\"[a-fA-F0-9]{2}\" name=\"numResult\" readonly />";
    }

    var lotoResultValues = document.getElementsByName('numResult');
    for (i = 0; i < inputs.length; ++i) {
      lotoResultValues[i].value = genValues[i];
    }

    var allValuesTrue = true;
    var nrWrongValues = 0;
    for (i = 0; i < inputs.length; ++i) {
      if (inputs[i].value.toUpperCase() != genValues[i].toUpperCase()) {
        allValuesTrue = false;
        lotoResultValues[i].style.border = "1px solid red";
        nrWrongValues++;
      } else
        lotoResultValues[i].style.border = "1px solid green";
    }

    // If I remove if-else, then the values in input elements will be displayed
    if (!allValuesTrue) {
      lotoResult.style.color = "red";
      lotoResult.innerHTML += "<p>!!! " + nrWrongValues + " wrong values.</p>";
    } else {
      lotoResult.style.color = "green";
      lotoResult.innerHTML += "<p>All elements are correct.</p>";
    }

  }
}


function getRandomHexValues() {
  var length = 2;
  var chars = "0123456789ABCDEF";
  var hex = "";
  var hexNums = new Array();
  var n = 8;

  for (i = 0; i < n; ++i) {
    length = 2;
    hex = "";
    while (length--) hex += chars[Math.floor(Math.random() * 16)];
    hexNums[i] = hex;
  }
  return hexNums;
}
body {
  margin: 10px auto;
  border: 2px solid red;
  width: 800px;
}
header {
  height: 10%;
  color: orange;
  width: 500px;
}
section {
  padding: 10px;
  border: 1px solid blue;
  display: block;
  width: 600px;
  float: left;
  margin: 20px 10px;
}
section p {
  color: red;
  display: inline-block;
  margin: 10px 10px;
}
footer {
  clear: both;
  margin-top: 50px;
}
footer p {
  text-align: center;
}
#S2 h1 {
  margin-left: 10px;
  color: red;
  font-family: sans-serif;
}
#S2 div {
  margin: 0 auto;
  width: 400px;
}
#S2 div input {
  width: 30px;
  border: 1px solid blue;
  color: blue;
}
#S2 p {
  display: block;
  color: blue;
  font-family: monospace;
}
#S2 .buttonSubmit {
  border-radius: 3px;
  border: 1.5px solid blue;
  width: 60px;
  position: relative;
  left: 20px;
  transition: background-color 1s;
}
#S2 .buttonSubmit:hover {
  background: rgba(26, 198, 255, 0.7);
}
#S2 .buttonSubmit:active {
  color: red;
}
#S2 #lotoResult {
  text-align: center;
  margin-top: 5px;
  visibility: hidden;
  color: blue;
  border: 1px solid orange;
  padding: 10px;
}
<header>
  <h1>HelloJavascript</h1>
</header>
<nav>
</nav>
<section id="S2">
  <h1>Game</h1>
  <p>Insert the numbers below:</p>

  <div class="tabs">
    <input type="text" pattern="[a-fA-F0-9]{2}" name="num" />
    <input type="text" pattern="[a-fA-F0-9]{2}" name="num" />
    <input type="text" pattern="[a-fA-F0-9]{2}" name="num" />
    <input type="text" pattern="[a-fA-F0-9]{2}" name="num" />
    <input type="text" pattern="[a-fA-F0-9]{2}" name="num" />
    <input type="text" pattern="[a-fA-F0-9]{2}" name="num" />
    <input type="text" pattern="[a-fA-F0-9]{2}" name="num" />
    <input type="text" pattern="[a-fA-F0-9]{2}" name="num" />
    <input class="buttonSubmit" type="button" onclick="checkValues()" value="Check" />
  </div>
  <div id="lotoResult"></div>
</section>
<footer>
  <p>
    &copy; 2016
  </p>
</footer>
Heretic Monkey
  • 11,687
  • 7
  • 53
  • 122
wonderingdev
  • 1,132
  • 15
  • 28

2 Answers2

1

This happened because you messed with object dom manipulation, in the commented code, and string dom manipulation in the paragraph concatenation.

Edited:

for (i = 0; i < inputs.length; ++i)
{
    lotoResult.innerHTML += "<input type=\"text\" pattern=\"[a-fA-F0-9]{2}\" name=\"numResult\" readonly value=\""+genValues[i]+"\" />";
}


//var lotoResultValues = document.getElementsByName('numResult');
//for (i = 0; i < inputs.length; ++i) {
//      lotoResultValues[i].value = genValues[i];
//  }
Vinícius Fagundes
  • 1,983
  • 14
  • 24
  • I added the values(using 'value' attribute) on lines: 30 - 33. Without that last if-else the values are shown. I've checked everything and came to the conclusion that there is something wrong with this last

    .

    – wonderingdev Nov 17 '16 at 21:56
1

Using innerHTML is not really a best practice. Doing "real" DOM manipulation will solve your problem here:

if (!allValuesTrue) {
    lotoResult.style.color = "red";
    //lotoResult.innerHTML += "<p>!!! " + nrWrongValues + " wrong values.</p>";

    var para = document.createElement('p');
    para.appendChild(document.createTextNode("!!! " + nrWrongValues + " wrong values."));
    lotoResult.appendChild(para);
} 
Tim Grant
  • 3,300
  • 4
  • 23
  • 31
  • It works well this way. But it's strange, why isn't innerHTML working here? I inserted 'input' with innerHTML and it worked fine. – wonderingdev Nov 17 '16 at 22:03
  • 1
    You should use only DOM manipulation or string concatenation. If you mess strange things happens. ;) – Vinícius Fagundes Nov 17 '16 at 22:11
  • 2
    See this answer regarding all the ways .innerHTML can bite you: http://stackoverflow.com/questions/11515383/why-is-element-innerhtml-bad-code – Tim Grant Nov 17 '16 at 22:11
  • 1
    Also, the DOM manipulation I showed you is probably overkill here, anyway. it would probably be better to show/hide the "wrong values" paragraph, and just change its content. (Note how my code will create multiple paragraphs.) – Tim Grant Nov 17 '16 at 22:14