0

All,

A total newbie here but could someone please review this code and let me know where I am going wrong?

What effect I am trying to achieve: I have about 9 divs on the webpage, consisting of images and text, the text comes into focus when the user hovers over the image inside the div. I want these divs to 'appear' to be floating around the page.

So what I did: I absolutely positioned the divs using CSS on the webpage. Now I am using JS to move the divs from their position to set positions on the page (will do this for each div) and try to give the effect of random movement to the divs. Here is the code:

        <script language="javascript">
            var x = document.getElementById("cr001").style.left;
            var y = document.getElementById("cr001").style.top;
            var d_x = 75;
            var d_y = 100;
            var interval = 2;                   //move by only 2px...

            function moveImage() {
                if(x < d_x) x = x + interval;
                if(y < d_y) y = y + interval - 1;                   //move y by only 1px

                document.getElementById("cr001").style.left = x+'px';
                document.getElementById("cr001").style.top = y+'px';

                if ((x + interval < d_x) && (y + interval < d_y)) {
                    window.setTimeout('moveImage()',100);
                }
            }   
    </script>   
</head>

<body onload="moveImage()">
    <div id="blackhole"><img src="img/bimg.png" alt="blackhole" /></div>
    <div id="container">

            <div id="cr001" class="twinkles">
                <a href="#">
                <img src="img/cr.png" alt="Co is about your freedom" />
                <span>Co is about your freedom</span></a>

    </div>

</body> 

Could someone please explain where I am going wrong?

Cheers,

Kayote
  • 14,579
  • 25
  • 85
  • 144

3 Answers3

1

The problem is here:

document.getElementById("cr001").style.left

This actually returns the css value, which is a string for example '100px' or '10%' etc. Yet, later on your code, you use this value as if it was an integer.

linepogl
  • 9,147
  • 4
  • 34
  • 45
  • Use `parseInt(document.getElementById("cr001").style.left);` – jcubic Mar 09 '11 at 22:22
  • Thanks linepogl and jcubic. That definitely makes sense. However, Im still getting the same error as before: document.getElementById("cr001") is null, something else Im doing wrong? – Kayote Mar 09 '11 at 22:33
  • @Kayote, the code that is outside the function moveImage() will be evaluated as soon as it is read, which is too soon: the div 'cr001' is not even read yet. – linepogl Mar 09 '11 at 22:38
  • linepogl, please have a look at the amended code description Ive posted in comment to jordancpaul's answer. Does that sound the right way of resolving the prob. Im not getting the error now, just nothing... nothing happens, the div is still at its original position. – Kayote Mar 09 '11 at 22:54
1

Based on the refined post, I now see that you are trying to access body content in the head with var x = document.getElementById("cr001").style.left;

This won't work because when the head is loaded, the body is not ready. You should make an init() function that looks like:

function init(){
    window.x = document.getElementById("cr001").style.left;
    window.y = document.getElementById("cr001").style.top;
    moveImage();
}

and then attach that to the body onload.

EDIT: ok, here is some modified code that does what you want. You can stick this in a file named index.html and launch it in Firefox. I broke it down for you:

<html>
<head> 
    <script language="javascript">
        var d_x = 75;
        var d_y = 100;
        var interval = 2;                   

        //init is used for getting things up and running once the page is loaded
        function init(){
            //optimization: only get the element once
            var el = document.getElementById("cr001")
            x = parseInt(el.style.left);
            if(isNaN(x)){
                //parseInt("") == NaN
                x = 0;
            }
            y = parseInt(el.style.top);
            if(isNaN(y)){
                //ditto
                y = 0;
            }
            //call the nuts of this script
            moveImage();
        }

        //this is mostly unchanged
        function moveImage() {
            if(x < d_x){
                x = x + interval;
            }else{
                //lets keep looping just for fun!
                x = 0;
            }
            if(y < d_y){
                y = y + interval - 1;                   //move y by only 1px
            }else{
                y = 0;
            }

            //optimization: only get the element once
            var el = document.getElementById("cr001")
            el.style.left = x + 'px'; //dont forget to add 'px' back in
            el.style.top = y + 'px';

            //loop, don't use strings in setTimeout since this is basically eval...eval is evil
            setTimeout(moveImage, 100);

        }   
    </script>   
</head>
<body onload="init()">
    <div id="container">
            <!-- IMPORTANT: POSITION IS ABSOLUTE -->
            <div id="cr001" style="position:absolute; width:10px; height:10px; background-color:black"></div>
    </div>
</body> 

jordancpaul
  • 2,954
  • 1
  • 18
  • 27
  • You've lost me jordancpaul. Would you please talk me through this code. Here is what I just tried after reading your comment: I moved the script to the bottom of the html (inbetween html & body close tags), the error has disappeared (hurrah!) but the animation is still not playing...? – Kayote Mar 09 '11 at 22:49
  • Well, that would also work, but it leads to fragile code (dependent on when the DOM is loaded). In general, you should try to avoid accessing page content with javascript before the page has loaded. The getElementById() function requires that the element actually exists, which is why this script fails in the head. I will add more info to this answer – jordancpaul Mar 09 '11 at 23:12
  • jordancpaul, thanks a million and one for helping out with this. I finally got it working. Now I just need to read it again and again till my brain adopts it as its own. Lots for me to play with. Thank you again. Best, – Kayote Mar 10 '11 at 00:00
0

You have an unclosed div. You open <div id="container"> and <div id="cr001" class="twinkles"> but only close one of them

jordancpaul
  • 2,954
  • 1
  • 18
  • 27
  • There is quite a bit of html code and in deletion for posting, I deleted (mistakenly) the div close as well. Thanks for the pointer. :) – Kayote Mar 09 '11 at 22:30