London | 26-ITP-Jan | Karla Grajales | Sprint 3 | Alarm Clock App#1183
London | 26-ITP-Jan | Karla Grajales | Sprint 3 | Alarm Clock App#1183Grajales-K wants to merge 12 commits intoCodeYourFuture:mainfrom
Conversation
- add assets folder for music and favicon
…ckground color on alarm trigger
Grajales-K
added
🏕 Priority Mandatory
| @@ -1,8 +1,59 @@ | |||
| function setAlarm() {} | |||
| let countdown; | |||
There was a problem hiding this comment.
Consider renaming countdown to countdownInterval or intervalId since this stores the interval ID, not the countdown value.
| clearInterval(countdown); | ||
| document.body.style.backgroundColor = "white"; |
There was a problem hiding this comment.
Currently when starting a new countdown, the application does not always return to a clean initial state,
which can lead to inconsistent behavior between runs.
Hint: a user may not click the "Stop" button first before starting a new count down.
You can also consider introducing a dedicated reset function to return the app to a clean initial state to help ensure consistency. There are few places in this script you can call this reset function instead of repeating the reset logic.
| let inputTime = Number(document.getElementById("alarmSet").value); | ||
|
|
||
| if (isNaN(inputTime) || inputTime <= 0) { | ||
| alert("Please type or select your time 👇⏰"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
What type of numbers should inputTime be? (Currently some input value can corrupt the timer display)
| let display = `Time Remaining: ${minutes | ||
| .toString() | ||
| .padStart(2, "0")}:${remainingSeconds.toString().padStart(2, "0")}`; |
There was a problem hiding this comment.
Having a long expression within a template string literal can make the code harder to read.
Better to use an inline function to perform the repeated task, or to store the formatted
values in variables first.
| let repetitions = 0; | ||
| countdown = setInterval(() => { | ||
| document.body.style.backgroundColor = `rgb( | ||
| ${Math.floor(Math.random() * 256)}, | ||
| ${Math.floor(Math.random() * 256)}, | ||
| ${Math.floor(Math.random() * 256)})`; | ||
| repetitions++; | ||
|
|
||
| if (repetitions > 100) { | ||
| clearInterval(countdown); | ||
| } |
There was a problem hiding this comment.
It is best practice to separate presentation logic from application logic.
Can you either implementing the background animation in CSS or move the code that perform the animation into a separate function?
Note: CSS random() function is still in experimental stage. With pure CSS, we can only cycle through a pre-defined set of colors (e.g. https://codepen.io/beben-koben/pen/eYPNew)
If you are implementing this purely in CSS, you can consider defining a CSS class, and use classList.toggle() to apply/remove the style. For example,
document.body.classList.toggle("alarm-activated", true); // apply style
document.body.classList.toggle("alarm-activated", false); // remove style
cjyuan
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
This application features a dynamic countdown engine built with JavaScript that accurately converts numerical user input into a
mm:ssdisplay format. It integrates a multisensory alert system that triggers both a continuous audio track and a synchronised disco background effect using random RGB generation once the alarm reaches zero.The core logic was optimised for test-driven development, implementing a repetition counter to prevent infinite loops and ensure all Jest unit tests pass successfully. The project includes an input validation to handle non-numeric values, ensuring a stable and functional application.