1

I'm trying repeat a function using SetInterval that shrinks and grows an image but it's not catching my flag so it keeps going in one direction rather than alternating the shrinking and growing.

function foo(toggle) {
   let img = document.querySelector('.product-image')
   if (img.style.width == "") { img.style.width = "0px"};

   let width = parseInt(img.style.width.match(/\d+/g)[0])


   if (toggle == false) {
        width = width + 50;
        img.style.width = width.toString() + "px"
        img.style.transition = "all 2s"
         toggle = true;
   } else {

       width = width - 50;
       img.style.width = width.toString() + "px"
       img.style.transition = "all 2s"
       toggle = false;
   }


}

let interval = setInterval(foo(false), 1000)
John Nada
  • 77
  • 1
  • 9
  • 1
    `setInterval(foo(false), 1000)` -> `setInterval(foo.bind(null, false), 1000)` or `setInterval(() => foo(false), 1000)`. JavaScript (sans edge cases) always executes synchronously but you can queue functions to execute later - that's essentially what setTimeout/setInterval do - you need to pass a function rather than call it :) – Benjamin Gruenbaum Jan 15 '19 at 20:23
  • maybe css keyframe would be more appropriate for your usecase; https://css-tricks.com/snippets/css/keyframe-animation-syntax/ –  Jan 15 '19 at 20:23
  • because javascript is single threaded, using a solution that run smoothly without preventing other action to occur would, I think, be better –  Jan 15 '19 at 20:25
  • 1
    I would recommend learning to use the debugger and [stepping through your code](https://developers.google.com/web/tools/chrome-devtools/javascript/) - it would have pointed out the issue (the function is only being called once) and would save time and frustration. – Benjamin Gruenbaum Jan 15 '19 at 20:25

3 Answers3

1

Since you need invoke foo with either true or false, I think you may prefer recursive setTimeout over setInterval:

function foo(toggle) {
  const img = document.querySelector('.product-image');
  img.style.width = img.style.width || "0px";
  const currentWidth = parseInt(img.style.width.match(/\d+/g)[0])
  const newWidth = toggle 
    ? currentWidth + 50 
    : currentWidth - 50;
  img.style.width = newWidth.toString() + "px"
  img.style.transition = "all 2s"
  setTimeout(foo.bind(null, !toggle), 1000);
}

foo(false)
<img class="product-image" src='https://upload.wikimedia.org/wikipedia/commons/9/9a/Random-image.jpg'/>
antonku
  • 7,377
  • 2
  • 15
  • 21
1
  1. Make toggle a global variable,
  2. For performance, make img a global,
  3. Only set the transition on the element once.

Thus :

function foo() {
  if (img.style.width == "") {
    img.style.width = "0px";
  }

  let width = parseInt(img.style.width.match(/\d+/g)[0]);

  if (toggle) {
    width -= 50;
  }
  else {
    width += 50;
  }

  img.style.width = width + "px";

  toggle = !toggle;

}

const img = document.querySelector('.product-image');
let toggle = true;

img.style.transition = "all 2s";

let interval = setInterval(foo, 1000);
do or do not
  • 146
  • 1
  • 5
0

You are calling the function passing the false value in every call. You should pass the variable instead of the hardcoded value in the last line. Anyways, you can solve this without passing the toggle value as argument. On the other hand. you don't need to rewrite the las three lines in each block of the if/else.

It would look something like this:

let toggle = false;

function foo() {
    let img = document.querySelector('.product-image')
    if (img.style.width == "") { img.style.width = "0px"};

    let width = parseInt(img.style.width.match(/\d+/g)[0])

    if (!toggle) { // shorter for toggle == false
        width += 50;
    } else {
        width -= 50;
    }

    img.style.width = width.toString() + "px"
    img.style.transition = "all 2s"
    toggle = !toggle; // inverts the boolean value of toggle
    console.log(toggle);
}

/*
 * pass the function without parentheses
 * so you pass a reference to the function
 * instead of its returning value
 */
let interval = setInterval( foo, 1000);
Santi
  • 1,682
  • 1
  • 13
  • 14
  • hmm, that seems to only trigger the function once and then it stops – John Nada Jan 15 '19 at 20:43
  • I think because `toggle != toggle` is just inverting the value of the argument being passed in with that name – John Nada Jan 15 '19 at 20:55
  • The reason why it's executing only once is explained here https://stackoverflow.com/questions/10182714/why-does-the-setinterval-callback-execute-only-once but i'm still having trouble to make de value of toggle var change – Santi Jan 15 '19 at 21:03
  • I edited my solution. now it should be doing fine. The main problem is that when, as the first argument for the setInterval function, you pass the function with parentheses. you are executing the function and passing its return value instead of passing the function. In order to pass the function so it can be executed later, you have to do it without parentheses. – Santi Jan 16 '19 at 13:14
  • 1
    Problem here is that you wanted also the function to be executed receiving a certain value. That's where the solution provided by benjamin gruenbaum in the first comment comes in. But in this case you don't actually need to pass the parameter since you can handle the toggle variable globally. as shown in my solution – Santi Jan 16 '19 at 13:14