0

I have this code here and i want the background to stop changing when i click the same button (the interval to stop). But i cannot work my way around it.(The else part works because in the console isCicked changes it's value).

let btn = document.querySelector('#btn');
let body = document.querySelector('body');
let isClicked = false;

btn.addEventListener('click', function(){
    let x;
    let y;
    if(isClicked == false){
        isClicked = true;
         x= setInterval(function(){
            let r,g,b;
            r = Math.round(Math.random() * 255);
            g = Math.round(Math.random() * 255);
            b = Math.round(Math.random() * 255);
            body.style.backgroundColor = `rgb(${r},${g},${b})`;
            btn.classList.toggle('button-style')
        },1000)
    }else{
        isClicked = false;
        clearInterval(x);
    }
    console.log(isClicked);

})
<button type="button" id="btn">Click</button>
pilchard
  • 12,414
  • 5
  • 11
  • 23
Alex A
  • 1
  • 1
  • 1
    You need to declare `x` outside the listener. You redeclare it on each click and overwrite the interval id so there's nothing to clear. – pilchard Nov 06 '21 at 23:45

2 Answers2

1

To fix your current code you need to declare x outside the event listener since as it stands you redeclare it on each click and overwrite your stored interval id.

let isClicked = false;
let x;

btn.addEventListener('click', function(){
    if(isClicked == false){
        isClicked = true;
        x = setInterval(function(){
    
    ...
}

But you can actually simplify a little by combining isClicked and x and just checking if an interval is stored.

let btn = document.getElementById('btn');
let body = document.querySelector('body');

let interval = null;

btn.addEventListener('click', function () {
  if (interval === null) {
    interval = setInterval(function () {
      let r, g, b;
      r = Math.round(Math.random() * 255);
      g = Math.round(Math.random() * 255);
      b = Math.round(Math.random() * 255);
      body.style.backgroundColor = `rgb(${r},${g},${b})`;
      btn.classList.toggle('button-style');
    }, 1000);
  } else {
    clearInterval(interval);
    interval = null;
  }
  console.log(interval);
});
<button type="button" id="btn">Click</button>
pilchard
  • 12,414
  • 5
  • 11
  • 23
0

Rather then trying to manage the state of setInterval, you could use setTimeout and recursion (a function calling itself) to create an interval. Likewise, rather than using variable mutation, since you're toggling a class on the button, you can just use the button itself as the exit condition for the recursion.

let btn = document.querySelector('#btn'),
    body = document.querySelector('body');

function ChangeBackground() {
  if (btn.classList.contains('button-style')) { //check exit condition
    body.style.backgroundColor = '#' + Math.random().toString(16).slice(-6);        
    setTimeout(ChangeBackground, 1000); //recursion
  }
}

btn.addEventListener('click', function(){
  this.classList.toggle('button-style'); //toggle exit condition
  ChangeBackground(); //start recursion
});
<button type="button" id="btn">Click</button>
PoorlyWrittenCode
  • 1,003
  • 1
  • 7
  • 10
  • The recursion seems a fine solution, more discussion of tradeoffs here: [recursive function vs setInterval vs setTimeout javascript](https://stackoverflow.com/questions/23178015/recursive-function-vs-setinterval-vs-settimeout-javascript), but your color calculation sometimes returns invalid strings. – pilchard Nov 07 '21 at 13:07
  • Copied the color generator from a comment on this post: https://stackoverflow.com/questions/1484506/random-color-generator. Picked it because it was a short one-liner and voted highly. Can't say I've ever actually used it though. – PoorlyWrittenCode Nov 07 '21 at 13:20
  • 1
    Who would have thought someone would post invalid code and people would just upvote it without testing it?!? I picked another one liner from the post in my previous comment and it tested 100% over 1 million iterations, so I'm going to call it valid. For future reference `(Math.random()*0xFFFFFF<<0).toString(16)` is not valid. Thanks for pointing this out. – PoorlyWrittenCode Nov 07 '21 at 13:41