This works for me, but is this the right way to create this timer?
I might be naive about this but I think this is close to perfect in pure JS.
What would be the drawbacks of using this Timer compared to creating it in a different way?
- Click start and the timer starts to countdown.
- Click reset and the timer resets back to the starting time.
HTML:
<button onclick='startStopTimer()'>Start</button>
<button onclick='startStopTimer()'>Reset</button>
<h1>00:10</h1>
JavaScript:
console.clear()
let timer = document.querySelector('h1')
let startingTime = 9
let startTimer;
let startStopTimer = () => {
if(startTimer){
clearInterval(startTimer);
timer.innerHTML = '00:10'
startingTime = 9
return startTimer = undefined
}else{
return startTimer = setInterval(()=>{
timer.innerHTML = `00:0${startingTime--}`
}, 1000)
}
}
3 Answers 3
HTML
- The inline
onClickevent handler attached to your HTML element probably works fine; however there are better ways to deal with the events in pure JS. Next time, add the event handlers in the JS file like so:element.addEventListener('click', callbackName). h1is a header. I'd not place the timer value inside that particular tag. I think thatspanwould be better here according to semantic HTML rules.
JavaScript
- Some of your
let-based variables could be changed toconst. The idea is to useconstwhenever it's possible. If it's not, only then uselet. innerHTMLis a dangerous method. It might be used to perform XSS attacks. Try to avoid it to display text on your website.
-
\$\begingroup\$ Your right I should have used addEventListener for a "pure" js timer. I do need to think about semantics more when I write HTML. It isn’t good practice to not do so. Thanks for that. I should have used innerText instead of innerHTML. \$\endgroup\$Myke24– Myke242021年03月15日 21:01:27 +00:00Commented Mar 15, 2021 at 21:01
-
\$\begingroup\$ No problem :) Good luck with refactoring ! \$\endgroup\$AdamKniec– AdamKniec2021年03月15日 21:03:00 +00:00Commented Mar 15, 2021 at 21:03
From a short review;
- You use
9twice, this should be assigned to a nicely named global constant - You hard code both
9and00:10, this is not flexible. I would expect to be able to change9to99and to have the timer start at01:40 - Similarly,
00:0${startingTime--}does not allow for much flexibility in the script - The browser tries to honor the
1000, but it cannot be trusted. You are supposed to keep track of the original start and deduct that from current time to determine how much time has passed. Otherwise the timer is not reliable. - Placing the timer in a
h1element is probably not a good idea semantically - Your entire code should be in a well named (self executing) function
- I am not sure why you return anything in
startStopTimer? If you are never using the returned value, then just drop thereturnkeyword - since
clearIntervalreturnsundefinedand because it makes sense, I would go forstartTimer = clearInterval(startTimer); - Why
console.clear()? Production code should avoidconsole
-
\$\begingroup\$ I thought I was good lol, guess not. Thanks for theses informative gems! Console.clear() is there because I was playing around in codepen. I just copied my whole script. I’ll make sure to keep that out of production code for sure. \$\endgroup\$Myke24– Myke242021年03月15日 20:52:09 +00:00Commented Mar 15, 2021 at 20:52
Countdowns
Some points missed or skimmed over by other answers.
Visibility:
Always keep DOM interaction to a minimum. In timers you should check if the document is visible before you make changes that may not be viewed by the client.
Timers are one such app that contains animations that the client will not continually watch.
Use the page visibility API to check for page visibility and listen to visibility change events.
Time element:
Use appropriate elements. To display a time such as a count down use the HTMLTimeElement and set its HTMLTimeElement.dateTime property to match the visible countdown time.
This gives meaning to the content and lets machines understand what the content represents.
Text is not Markup:
Never use Element.innerHTML to set text only content as it will force a CPU/GPU expensive re-flow. Use Node.textContent and avoid the expensive re-flow.
Calculate Time
Timers such as setInterval and setTimeout should not be use to measure time. To measure time till an event store the expected time and calculate the time from now till then.
Avoid using setInterval as it can fire well off the expected interval, rather calculate the time until the next visual change and use setTimeout
Scope:
Avoid using let in the global scope as its scope is un-intuitive and may throw errors due to clashes with existing named variables unrelated to you code.
Add don't set listeners:
Never set event listeners directly eg
onclick="somefunction"as the event listener can be overwritten by accident by you, or by poorly written 3rd party code or extensionsAlways use EventTarget.addEventListener to add event listeners.
Disable unusable UI
A button that does nothing should be disabled.
Rewrite
Following points outlined above the snippet implements a countdown timer with start/restart and stop buttons.
The code details...
- Isolates scope using
{} - Calculates timer value rather than count time using timer events
- Calculates time till next visual change rather than use unreliable interval timer.
- Uses the correct element to display time. Also sets the machine readable property
dateTime. (note that I was lazy and only set the seconds) - Adds event listeners rather than set event listeners.
- Listens to
visibilityChangeso that the time the client sees is always correct. - Checks the page visibility before making visual changes to the page. (note the last event will make changes to the DOM)
- Disables the stop button when there is nothing to stop.
- Stores the timer length as the buttons
valueproperty (Note that the stop button has a value of 0)
{ // to isolate the scope of your code from any existing code.
const TICK_RESOLUTION = 1000; // in ms
const TIMER_LENGTH = 10; // multiples of TICK_RESOLUTION
startBtn.value = TIMER_LENGTH * TICK_RESOLUTION;
startBtn.addEventListener("click", startStopTimer);
stopBtn.addEventListener("click", startStopTimer);
document.addEventListener("visibilityChange", tick);
let endTime, timeHdl;
function startStopTimer(event) {
endTime = performance.now() + Number(event.target.value);
tick();
}
function tick() {
clearTimeout(timeHdl);
var till = endTime - performance.now();
till = till <= 0 ? 0 : till;
if (till) {
if (document.visibilityState === "visible") {
stopBtn.disabled = false;
timeEl.textContent = (till / TICK_RESOLUTION + 1 | 0);
}
timeEl.dateTime = "PT0H0M" + (till / TICK_RESOLUTION + 1 | 0) + "S";
timeHdl = setTimeout(tick, (till % TICK_RESOLUTION) + 10);
} else {
timeEl.dateTime = "PT0H0M0S";
timeEl.textContent = 0;
stopBtn.disabled = true;
}
}
}
div {
font-family: arial;
font-size: 48px;
}
<button id="startBtn" value="0">Start</button>
<button id="stopBtn" disabled value="0">Stop</button>
<div><time id="timeEl" dateTime="PT0H0M0S">0</time> seconds</div>
-
\$\begingroup\$ Wow, I didn’t know about the time element! \$\endgroup\$konijn– konijn2021年03月16日 21:59:42 +00:00Commented Mar 16, 2021 at 21:59
restartvsreset. To me,resetmeans put it back to zero andrestartmeans put it back to zero and start the timer again. Could just be me though. \$\endgroup\$