dadadodo dadadodo - 17 days ago 9
Javascript Question

clear interval not stoping at desired point

I looked at some other similar problems on this site and could not fix this problem. Below is part of a pomodoro clock program that I'm making. The problem is that I'm unable to make this set interval method stop when the clock reaches 00:00.

here is my code:

var break_minutes = 0;
var ses_minutes = 0;
var ses_minutes_sec;
var display = document.querySelector('#time');

function increment_ses (id) {
ses_minutes = ses_minutes + 1;
document.getElementById("ses_value").innerHTML = ses_minutes ;
document.getElementById("timmer_circle").innerHTML = ses_minutes;
}
function decrement_ses (id) {
if (ses_minutes > 0) {
ses_minutes = ses_minutes - 10;
} if (ses_minutes < 0) {
ses_minutes = 0;
}
document.getElementById("ses_value").innerHTML = ses_minutes ;
document.getElementById("timmer_circle").innerHTML = ses_minutes;
}
function runTimer () {
var minutes = ses_minutes-1;
var seconds = 10;
var interval = setInterval(function () {
seconds = seconds -1;
if (seconds == 0) {
minutes --;
seconds = 10 -1;
}
function str_pad_left(string,pad,length) {
return (new Array(length+1).join(pad)+string).slice(-length);
}
var finalTime = str_pad_left(minutes,'0',2)+':'+str_pad_left(seconds,'0',2);
document.getElementById("timmer_circle").innerHTML= finalTime;
if (minutes == 0) {
if (seconds == 0) {
return clearInterval(interval);
}
}
},1000);
}


the HTML

<html>
<head>
<title>Pomodoro</title>
<script src="pomodoro.js"></script>
</head>
<body>
<style>
.timmer_circle
{
width: 300px;
height: 300px;
border-radius: 50%;
font-size: 50px;
color: #fff;
line-height: 300px;
text-align: center;
background: #000;
}
.break_length{width:100%;}
#decrement{float:left;width:100px;}
#break_value{text-align: center;padding-left: 100px;}
#increment{margin:0 auto;width:100px;}

.session_length{width:100%; margin-top: 10px;}
#decrement_ses{float:left;width:100px;}
#ses_value{padding-left: 100px;}
#increment_ses{margin:0 auto;width:100px;}
#start_but{margin-top: 20px;}
#pause_but{margin-top: 20px; margin-left: 2px;}
</style>
<table>
<tr>
<td>
<div class = "timmer_circle" id ="timmer_circle" value = ""> <span id = "time">Session</span> </div>
</td>
</tr>
</table>
<div class="session_length">
<button type="button" id = "decrement_ses" onClick = "decrement_ses(this.id);">ses/dec</button>
<button type="button" id = "ses_value" >0</button>
<button type="button" id = "increment_ses" onClick = "increment_ses(this.id);">ses/inc</button>
</div>

<div class="break_length">
<button type="button" id = "decrement" onClick = "decrement_break(this.id);">brk/dec</button>
<button type="button" id = "break_value" value = "" >0</button>
<button type="button" id = "increment" onClick = "increment_break(this.id);">brk/inc</button>
</div>
<button id ="start_but" onClick="runTimer();">START</button>
<button id ="pause_but">PAUSE</button>

</body>
</html>

Answer

So, there's a number of issues with your code, specifically how you're calculating seconds and minutes. Seconds are never zero when you hit your case statement the way you're doing it, because you test for zero at the start of your function and then reset them to 10-1.

I would move that check to the start of the function, as part of your initial conditional. There are still other issues, but this is to answer your specific question about the interval. The below code will exit as expected:

var ses_minutes = 1;
var minutes = ses_minutes-1;
var seconds = 10;
var interval = setInterval(function () {
        seconds = seconds -1;
        if (seconds == 0) {
           if(minutes === 0){
              return clearInterval(interval);
           }
           minutes --;
           seconds = 10 - 1;
        } 
        function str_pad_left(string,pad,length) {
            return (new Array(length+1).join(pad)+string).slice(-length);
        }
        var finalTime = str_pad_left(minutes,'0',2)+':'+str_pad_left(seconds,'0',2);
        document.getElementById("timmer_circle").innerHTML= finalTime;
    },1000);

/* EDIT */

So taking a few minutes to think about how I'd do this if it were me, I'd break things up into functions a little differently, and I'd also use setTimeout. I'd also calculate based on elapsed time rather than assuming the interval or timeout tick will happen at exactly 1000ms, since that's not always reliable. The below is probably still not perfect (I've only done cursory testing) but closer to what I'd do if I were tasked with this:

const display = document.getElementById('timer');
const countDownFrom = 15000; // time in milliseconds to count from
let startTime = new Date().getTime();

function padNumber(num){
  let str = num.toString();
  return str.length > 1 ? str : '0' + str;
}

function getDisplay(milliseconds){
   const seconds = milliseconds / 1000;
   const displayMinutes = padNumber(Math.floor(seconds / 60));
   const displaySeconds = padNumber(Math.floor(seconds % 60));
   return displayMinutes + ':' + displaySeconds;
}

function tick(){
   const currentTime = new Date().getTime();
   const elapsedTime = currentTime - startTime;

   // test to see if the timer has expired
   if(countDownFrom - elapsedTime <= 0){ 
    display.innerHTML = '00:00';
    return;
   }

   display.innerHTML = getDisplay(countDownFrom - elapsedTime);
   setTimeout(tick,1000);
}

display.innerHTML = getDisplay(countDownFrom);
setTimeout(tick, 1000);
Comments