2

I keep getting an unexpected token } error when trying to pass this.value as an argument.

In my body I have:

<input type="text" size="3" id="rbgTextbox" onkeypress="if (ValidateRGBTextboxInput(event)) ChangeBackgroundColor("+this.value+"); else return false;">

The functions:

<script type="text/javascript">
    function ValidateRGBTextboxInput(e) {
      return ById('rbgTextbox').value.length < 6 &&
       (e.which >= 48 &&
        e.which <= 57 ||
        e.which >= 65 &&
        e.which <= 70 ||
        e.which >= 97 &&
        e.which <= 102);
    }

    function ChangeBackgroundColor(color) {
      alert(color);
      ById('gameArea').style.backgroundColor = '#'+color;
      ById('nextPiece').style.backgroundColor = '#'+color;
    }
  </script>

If I change ChangeBackgroundColor("+this.value+"); for ChangeBackgroundColor('000000'); I get no more error, but of course I need the pass the text input's value as the argument.

Mickael Bergeron Néron
  • 1,472
  • 1
  • 18
  • 31

3 Answers3

2

You don't need the quotes. If you have the double quotes you are breaking the string required by the onkeypress attribute on the input field. Hope that makes sense ;)

<input type="text" 
       size="3" 
       id="rbgTextbox" 
       onkeypress="if (ValidateRGBTextboxInput(event)) ChangeBackgroundColor(this.value); else return false;">

This popular post on SO may help you understand quotes in Javascript.

Community
  • 1
  • 1
geedubb
  • 4,048
  • 4
  • 27
  • 38
1

Try placing the if statement in a function and calling the function. It would achieve a couple things. It makes you code more readable as well as will allow to easily you place some alerts within your if and else to see what is really going on.

Lawrence Thurman
  • 657
  • 8
  • 15
1

I don't think you need the + this.value + ' statement just this.value.

Additionally I would suggest that you just add a click handler to the html and avoid additional logic as shown below.

http://jsfiddle.net/9BtAM/13/

Note I think your design needs to improve further as the element value is not set untill after key press event. So you need to concatenate the new key press if acceptable to the existing element value.

 <input type="text" size="3" id="rbgTextbox" onkeypress="CheckKeyPress(event);" />

With the javascript here

 function CheckKeyPress(event) { 
    var element = document.getElementById('rbgTextbox');

     if (ValidateRGBTextboxInput(keyPressEvent)) 
         ChangeBackgroundColor(element.value); 
     else 
         return false;
}

Which is much easier to follow. As a rule you don't want complex javascript directly in the html.

Captain John
  • 1,859
  • 2
  • 16
  • 30
  • 1
    That use of `event` won't work except on IE (and browsers like Chrome that throw IE-specific code a bone). Further, the way you're calling `CheckKeyPress`, `this` will not be the element reference (it'll be the global object). – T.J. Crowder Jan 12 '14 at 13:04