NW6| Nazanin-Saedi | Module-JS2 | Alarmclock | Week-3#205
NW6| Nazanin-Saedi | Module-JS2 | Alarmclock | Week-3#205nazaninsaedi wants to merge 3 commits intoCodeYourFuture:mainfrom
Conversation
✅ Deploy Preview for cute-gaufre-e4b4e5 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Ara225
left a comment
Ara225
left a comment
There was a problem hiding this comment.
The code here is quite good, however there is quite a big bug - when you add the alarm, the seconds are the seconds you set minus one and the seconds go down to -1 before the alarm rings. You've also changed the tests which is never a good idea in this sort of situation - they're the benchmark for your work. If you do have time, go back and sort this or we can walk through it together.
| const input = document.getElementById("alarmSet"); | ||
| const heading = document.getElementById("timeRemaining"); | ||
|
|
||
| let timeInSeconds = parseInt(input.value, 10); |
There was a problem hiding this comment.
The parseInt function doesn't need the second param to be specfied for this use case - have a look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt to find out why
|
|
||
| if (isNaN(timeInSeconds) || timeInSeconds <= 0) { | ||
| alert("Please enter a valid positive number for the alarm time."); | ||
| return; |
| let seconds = timeInSeconds % 60; | ||
|
|
||
| countdown = setInterval(() => { | ||
| seconds--; |
There was a problem hiding this comment.
This line causes a bug - the seconds go down to -1 and when you add an alarm, the time is the time you entered -1. Can you see why this happens?
| if (minutes === 0) { | ||
| clearInterval(countdown); | ||
| playAlarm(); | ||
| document.body.style.backgroundColor = "red"; // This was just optional, background color will change |
|
|
||
| heading.innerText = `Time Remaining: ${minutes | ||
| .toString() | ||
| .padStart(2, "0")}:${seconds.toString().padStart(2, "0")}`; |
|
|
||
| function setup() { | ||
| document.getElementById("set").addEventListener("click", () => { | ||
| clearInterval(countdown); // Clear the previous countdown interval |
| document.getElementById("stop").addEventListener("click", () => { | ||
| clearInterval(countdown); | ||
| pauseAlarm(); | ||
| document.getElementById("timeRemaining").innerText = "Time Remaining: 00:00"; |
|
|
||
| expect(mockPlayAlarm).toHaveBeenCalledTimes(1); | ||
| }); | ||
| expect(heading).toHaveTextContent("Time Remaining: 01:58");}); |
There was a problem hiding this comment.
You shouldn't change tests to make your code pass them - tests are there to tell you when things are wrong so you can fix them.
|
|
||
| <button id="set" type="button">Set Alarm</button> | ||
| <button id="stop" type="button">Stop Alert</button> | ||
| <audio id="alarmAudio" src="alarmsound.mp3"></audio> |
| } | ||
|
|
||
| .container-title { | ||
| color: #ece163; |
2 participants
No description provided.