More SVG questions (and Javascript) - Interactivity [moving a rectangle]

Question

The following code is supposed to make the rectangle move when pressing the WASD keys on the keyboard. It doesn't work. The rectangle does not move, but no errors are generated. How can I make this work?

<?xml version="1.0" encoding="UTF-8" standalone="no"?>

<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
"http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">

<svg width="90%" height="90%" version="1.1"
xmlns="http://www.w3.org/2000/svg">

<script type="text/javascript">
    <![CDATA[
    var x;
    var y;
    function move()
    {
        x = new Number(document.getElementById("player").x);
        y = new Number(document.getElementById("player").y);
        switch (event.keyCode)
        {
            case 119:
            y--;
            document.getElementById("player").y=y;
            break;
            case 115:
            y++;
            document.getElementById("player").y=y;
            break;
            case 97:
            x--;
            document.getElementById("player").x=x;
            break;
            case 100:
            x++;
            document.getElementById("player").x=x;
            break;
            default:
        }
    }
    document.documentElement.addEventListener("keypress", move, true);
    ]]>
</script>

<rect x="0" y="0" height="100%" width="100%" style="stroke-width:5; stroke:black; fill:white"></rect>
<rect x="250" y="250" id="player" height="50" width="50" style="stroke-width:1; stroke:red; fill:red"></rect>

</svg>

@mortalc Perhaps you didn't see [my answer to your previous question](http://stackoverflow.com/questions/5378559/including-javascript-in-svg/5381905#5381905) which included specific things to change about your code, and gave you a much cleaner solution than what you show above. Further, it's not clear what you are asking here. I see is "Why?" that comes after a bullet list of three statements, and then a separate question regarding the basics of DOM interaction. This seems a shotgun question asking for general improvements, when you haven't even reviewed your previous answers. — Phrogz, Mar 28 '11 at 14:27
The reason I brought up the last question is because it wasn't mentioned in the XML DOM tutorial I was reading, but I have recently discovered the answer, so disregard that. Now about your points: 1. As I mentioned, I tried that, and it didn't work. I was wondering why. 2. I wish I knew what that meant! 3. I don't follow... sorry 4. I wish I knew what that meant! 5. Ok 6. Keypress is important so that it keeps moving while the button is held down. — mortalc, Mar 28 '11 at 18:01
About my points: 2. I was wondering why the other answer, which was just my code with an event listener, didn't work when a rectangle was used instead of a circle. 3. An odd phenomenon that confused me. — mortalc, Mar 28 '11 at 18:03
@mortalc If you didn't understand my answer in the other question, then post a comment saying so in that question. I will go and provide more details in my answer on that question. — Phrogz, Mar 28 '11 at 18:19
@mortalc I cannot guess why your code did not work when using a rectangle instead of a circle. If you want help with that, then edit your question with the code that is not working (not a vague description of what you did). — Phrogz, Mar 28 '11 at 18:28

score 4 · Accepted Answer · edited May 23 '17 at 12:19

Your code above does not work because the x and y properties of an SVGRectElement are SVGAnimatedLength objects, representing values that can change over time.

The simplest changes to make your code 'work' are:

Change
x = new Number(document.getElementById("player").x);
to
x = new Number(document.getElementById("player").x.baseVal.value);
(and similarly for y).
Change
document.getElementById("player").x=x;
to
document.getElementById("player").x.baseVal.value=x;
(and similarly for y).

This will cause your code to work as desired. However, your code could generally be improved per my answer to your previous question.

Here is how I would write it (if you really only want to move one unit at a time):

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg width="90%" height="90%" version="1.1"
 xmlns="http://www.w3.org/2000/svg">
<style>
    #player { stroke-width:1; stroke:red; fill:red }
</style>
<rect x="250" y="250" id="player" height="50" width="50" />
<script type="text/javascript"><![CDATA[
  var KEY = { w:87, a:65, s:83, d:68 };
  var rect = document.getElementById('player');
  var x = rect.getAttribute('x')*1,
      y = rect.getAttribute('y')*1;
  document.documentElement.addEventListener("keydown", function(evt){
    switch (evt.keyCode){
      case KEY.w: rect.y.baseVal.value = --y; break;
      case KEY.s: rect.y.baseVal.value = ++y; break;
      case KEY.a: rect.x.baseVal.value = --x; break;
      case KEY.d: rect.x.baseVal.value = ++x; break;
    }
  }, false);
]]></script>
</svg>

Note that I've moved the <rect> above the <script> block so that a) the user sees the rectangle before the script starts to load, but more importantly b) so that the rectangle exists before calling getElementById(). If your script block occurs before your elements have been defined, you must instead set up your code references in response to an onload or similar event handler.

Wow thanks! My code changed in the way you suggested didn't work, but that one did! Now to figure out why (other than the reason you mentioned of course!)... Anyways it was very helpful! Thankyou! — mortalc, Mar 28 '11 at 21:32
Just saw your edits on my other question... I understand much better now... I hope. Let's see how long it will last! — mortalc, Mar 28 '11 at 21:43

More SVG questions (and Javascript) - Interactivity [moving a rectangle]

1 Answers1

Linked