2
\$\begingroup\$

I created a script to hide a menu during user inactivity and when the position of my page is left < 2.

I would like to know if I use a good method regarding the performances. It's not too heavy ? Should I optimize my code ?

var interval = 1;

setInterval(function(){
    var posx=$("#page").offset();
    if (interval == 30 && posx.left < 2) {
    $('#menu').fadeOut(1000);
       interval = 1; 
   }
   interval = interval+1;
    console.log(interval);
    },1000);

$(document).bind('mousemove keydown scroll touchstart', function() {
    var posx=$("#page").offset();
    if($('#menu').is(':hidden') && posx.left < 2) {
        $('#menu').fadeIn(1000);
        interval = 1; 
    }
});
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Hmmm... polling at regular intervals instead of using a handler... not good. It means that the interval between the event (mousemove, etc) and the call to fadeOut is variable. It also means that the script continues to use up a small amount of CPU while nothing is happening. And there's a tradeoff between these two things: If you increase the second argument to setInterval you get more variation in waiting time but less needless CPU usage.

Also, the logic feels slightly wrong: the element is hidden if it is near the left edge after 30 seconds; but if it isn't, and subsequently is moved left, fadeOut will not be called. Having said that, I'm not sure what the events are; if it's impossible to move it left without triggering one of the events, then ignore this paragraph. If this is an issue then I'm not sure exactly what behaviour you want, so I've ignored this issue in my answer - my code will behave as I just described.

Anyway, what you need is window.setTimeout; this function schedules another function to run after a given interval. So if you call window.setTimeout in the event handler, it will cause another function (hideIfNecessary, below) to run 30 seconds after an event is fired.

Of course, if another event is fired while the clock is ticking, you want to call hideIfNecessary 30 seconds after the second event, but cancel the one 30 seconds after the first event. To cancel events scheduled with window.setTimeout, use setTimeout's evil twin, clearTimeout.

So without further ado, here's my version of your code with setTimeout. (I haven't tested it though.)

var timeoutId = null;

function hideIfNecessary(){
    var posx=$("#page").offset();
    if (posx.left < 2) {
        $('#menu').fadeOut(1000);
    }
}

$(document).bind('mousemove keydown scroll touchstart', function() {
    var posx=$("#page").offset();
    if($('#menu').is(':hidden') && posx.left < 2) {
        $('#menu').fadeIn(1000);
    }
    if (timeoutId !== null) window.clearTimeout(timeoutId);
    timeoutId = window.setTimeout(hideIfNecessary, 30000);
});
\$\endgroup\$
4
  • \$\begingroup\$ Thank you for you help. Unfortunately your code doesn't works. \$\endgroup\$
    – Xroad
    Commented May 21, 2014 at 12:28
  • 1
    \$\begingroup\$ Tell me more. What happens? What error message(s) do you get? \$\endgroup\$ Commented May 21, 2014 at 18:07
  • \$\begingroup\$ In fact the menu doesn't disappear \$\endgroup\$
    – Xroad
    Commented May 21, 2014 at 20:19
  • \$\begingroup\$ Do you have Firebug, IE Developer Tools, or something equivalent? (You should!) If so, do you get error messages? I've found some mistakes in my code (and fixed them). The function name hideIfNecessary had a lowercase N in its definition. (This was the most serious error.) Also the legacy variable interval was mentioned, and there was an unneeded semicolon after the function definition. \$\endgroup\$ Commented May 21, 2014 at 22:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.