Javascript "For" Loop not working?

Question

I have a Javascript file that I am using to try to animate a dropdown menu. I have the "Toggle" function in that file set to run when I click on a certain div. Here's the script I'm using:

var up = true;
        function Toggle(x)
    {
        if (up)
        {
            for (var i = x.offsetTop; i <= 0; i++)
            {
                x.style.top = i;
                if (i == 0)
                {
                    up = false; 
                }
            }
        }

        else if (up == false)
        {
            for (var i = x.offsetTop; i >= -50; i--)
            {
                x.style.top = i;
                if (i == -50)
                {
                    up = true;
                }
            }
        }
    }

In the HTML div I want to animate, I have the "onclick" property set to "onclick=Toggle(this)". The first for loop works as it should (it sets the div's top offset to 0). However, the second for loop doesn't set the offsetTop. I know that the for loop is activating because I've tested it and it gives me every integer between 0 and -50. Why isn't it setting the offset position?

Answer 1

1) You must specify a unit to the top ie: x.style.top = i +"px"

2) Your function won't animate instead of you use a setInterval or a setTimeout

Like you asked, an example. I wouldn't do it like this for one of my project, but i kept your function to make you more familiar with the code. I Used setTimeout instead of setInterval because setInterval must be cleared when not needed and setTimeout is just launched one time :

var Toggle = (function() { // use scope to define up/down
    var up = true; 
    return function(element) {
        var top = parseInt(element.style.top, 10); // element.offsetTop ?
        if ( !top ) {
            top = 0;
        }

        if (up) {
            if (element.offsetTop < 0) { // if we are not at 0 and want to get up
                element.style.top = (top+1) + "px";
                setTimeout(function() { Toggle(element); }, 10); // recall the function in 10 ms
            } else { // we change up value
                up = !up;  
            }
        }
        else {
            if (element.offsetTop > -50) {
                element.style.top = (top-1) + "px";
                setTimeout(function() { Toggle(element); }, 10); // recall the function in 10 ms
            } else {
                up=!up;
            }
        }
    }
})();

Answer 2

You'd have to use x.style.top = i + 'px' as top and similar css properties must define the type (px, em, %, etc.) unless they are 0, as this is 0 in any case.

But your script would actually snap the div directly to -50px, as you do not wait between those iteration steps.

I'd recommend to use a library like jQuery to use it's animate() method.

function Toggle(obj) {
  $(obj).animate({ 
    top: parseInt($(obj).css('top')) === 0 ? '-50px' : '0px'
  })
}