London | 26-ITP-May | Dipa Sarker | Sprint 3 | Alarm Clock App - #1294
London | 26-ITP-May | Dipa Sarker | Sprint 3 | Alarm Clock App#1294Dipa-Sarker wants to merge 2 commits into
Conversation
Dipa-Sarker
added
the
Needs Review
LonMcGregor
added
the
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start on this task, there is still some work to do however
| bodyElement.style.backgroundColor = "yellow"; | ||
| alarmSoundElement.play(); | ||
| } | ||
| function setAlarm() { |
There was a problem hiding this comment.
Have you properly handled the input validation here?
There was a problem hiding this comment.
No, I didn't handle input validation. Now I have tried with empty input and input time < 0.
| .padStart(2, "0"); | ||
| const showTimeSeconds = (remainingTime % 60).toString().padStart(2, "0"); | ||
| timeRemainingElement.innerText = showTimeMinutes + ":" + showTimeSeconds; | ||
| if (remainingTime === 0) { |
There was a problem hiding this comment.
Do you properly finish the timer countdown after it hits zero?
There was a problem hiding this comment.
Sorry, I am unclear about your question. But I think I finished the countdown after hitting zero at here:
function countDown() {
if (remainingTime <= 0) {
clearInterval(intervalID);
triggerAlarm();
return;
}
Which means:
- Timer reaches 00:00
- Interval stops
- No negative counting
- Alarm starts
- returns from the code so no extra code runs afterward.
|
|
||
| let remainingTime; | ||
| let intervalID; | ||
| function triggerAlarm() { |
There was a problem hiding this comment.
A bit more spacing between functions / improved formatting would help readability.
There was a problem hiding this comment.
Actually, I used prettier tool according to the style guide for formatting. Should I provide extra spaces in between functions?
| window.onload = setup; | ||
|
|
||
| const timeRemainingElement = document.querySelector("#timeRemaining"); | ||
| const alarmSetElement = document.querySelector("#alarmSet"); |
There was a problem hiding this comment.
If you know you are using IDs, can you think of something better tha n querySelector?
LonMcGregor
added
Reviewed
Dipa-Sarker
added
Needs Review
2 participants
Learners, PR Template
Self checklist
Changelist
Implemented the alarm clock functionality with countdown and alarm controls.
** Changes made**
I also tested the functions to works the app properly.