London | 26-ITP-May | Gideon Defar | Sprint 3 | Alarm Clock App - #1423
London | 26-ITP-May | Gideon Defar | Sprint 3 | Alarm Clock App#1423gideondefar wants to merge 5 commits into
Conversation
|
|
||
| function setAlarm() { | ||
|
|
||
| if (timer) clearInterval(timer); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Now it keeps countdown to 0.
| 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.
Run Number.isInteger(NaN) into your browser console and what comes back? Is the isNaN(time) check earning its place here?
There was a problem hiding this comment.
You are right, It's kind of redundant, So I removed it. !Number.isInteger(time) already checks the condition.
|
|
||
| function updateDisplay(time) { | ||
|
|
||
| let heading = document.getElementById("timeRemaining"); |
There was a problem hiding this comment.
None of these three are ever reassigned. What's your rule for picking between let and const?
There was a problem hiding this comment.
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.
|
Thanks Shari for taking time to review my PR. But one thing to consider
It’s been three weeks since I published my data structuring and testing PR.
I would appreciate if you have a look at it.
…On Tue 18. Aug 2026 at 06:47, Shadi Elyafi ***@***.***> wrote:
***@***.**** approved this pull request.
—
Reply to this email directly, view it on GitHub
<#1423?email_source=notifications&email_token=BR4RMV56F37RIE6UNQQTJCD5KPUYZA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOJVG43DENZVG432M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4957627577>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BR4RMV3TRQOB3GUPEI2W65T5KPUYZAVCNFSNUABFKJSXA33TNF2G64TZHM3TSOJSGQ2TMMRUHNEXG43VMU5TKMJSGY2DAMJSGM2KC5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/BR4RMV5YFCB2B7XOCHU6FST5KPUYZA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOJVG43DENZVG432M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/BR4RMV3UZXNUQUOG5FHIOED5KPUYZA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOJVG43DENZVG432M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Self checklist
Changelist
Implemented the Alarm Clock assignment.
Updated the page title to "Alarm clock app"
Added the alarm countdown functionality
Added minute and second formatting
Added a countdown that updates every second
Added alarm playback when the countdown reaches zero
Kept the existing Stop Alarm functionality