-
-
Notifications
You must be signed in to change notification settings - Fork 327
London | 26-ITP-May | Gideon Defar | Sprint 3 | Alarm Clock App #1423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,40 @@ | ||
| function setAlarm() {} | ||
| let timer; | ||
|
|
||
| function updateDisplay(time) { | ||
|
|
||
| let heading = document.getElementById("timeRemaining"); | ||
| let minutes = Math.floor(time / 60); | ||
| let seconds = time % 60; | ||
|
|
||
| heading.innerText = | ||
| "Time Remaining: " + | ||
| String(minutes).padStart(2, "0") + | ||
| ":" + | ||
| String(seconds).padStart(2, "0"); | ||
| } | ||
|
|
||
| function setAlarm() { | ||
|
|
||
| if (timer) clearInterval(timer); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the browser, start a 10-second countdown, then clear the input box and click Set Alarm again. What happens to the countdown, and is that what a user would expect? Have a look at the order of this line and line 23.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now it keeps countdown to 0. |
||
| audio.pause(); | ||
| audio.currentTime = 0; | ||
|
|
||
| let time = Number(document.getElementById("alarmSet").value); | ||
| if (isNaN(time) || !Number.isInteger(time) || time <= 0) return; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Run Number.isInteger(NaN) into your browser console and what comes back? Is the isNaN(time) check earning its place here?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right, It's kind of redundant, So I removed it. !Number.isInteger(time) already checks the condition. |
||
|
|
||
| updateDisplay(time); | ||
|
|
||
| timer = setInterval(function () { | ||
| time = time - 1; | ||
|
|
||
| updateDisplay(time); | ||
|
|
||
| if (time <= 0) { | ||
| playAlarm(); | ||
| clearInterval(timer); | ||
| } | ||
| }, 1000); | ||
| } | ||
|
|
||
| // DO NOT EDIT BELOW HERE | ||
|
|
||
|
|
@@ -22,4 +58,4 @@ function pauseAlarm() { | |
| audio.pause(); | ||
| } | ||
|
|
||
| window.onload = setup; | ||
| window.onload = setup; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of these three are ever reassigned. What's your rule for picking between let and const?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! ooh I see you are right. I use let basically to reassign the variable later but const is for fixed or constant variables.