London | 26-ITP-Jan | Zadri Abdule | Sprint 3 | Alarm Clock app - #1118
London | 26-ITP-Jan | Zadri Abdule | Sprint 3 | Alarm Clock app#1118Zadri415 wants to merge 2 commits into
Conversation
Zadri415
added
📅 Sprint 3
cjyuan
left a comment
cjyuan
left a comment
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 behaviour between runs.
Note: a user may not click the "Stop" button first before starting a new count down.
| const m = Math.floor(remainingSeconds / 60); | ||
| const s = remainingSeconds % 60; | ||
| heading.textContent = `Time Remaining: ${pad(m)}:${pad(s)}`; | ||
| } |
There was a problem hiding this comment.
For a "simple" function used locally in a function, we can define it as an arrow function in the function where it is used:
function updateHeading() {
const pad = (n) => String(n).padStart(2, "0");
...
}This way, we can keep only relatively key functions in the outermost scope.
| @@ -1,5 +1,79 @@ | |||
| function setAlarm() {} | |||
| let timerId = null; | |||
| let remainingSeconds = 0; | |||
There was a problem hiding this comment.
Best practice is to avoid declaring variable in the global/outermost scope.
You can declare remainingSeconds as a local variable in setAlarm() and then pass it via a parameter to updateHeading().
| // Visual feedback helpers (add/remove a class on <body>) | ||
| function triggerVisualAlarm() { | ||
| try { | ||
| document.body.classList.add("alarm-active"); | ||
| } catch (e) { | ||
| // ignore if DOM not available in test environment | ||
| } | ||
| } | ||
|
|
||
| function clearVisualAlarm() { | ||
| try { | ||
| document.body.classList.remove("alarm-active"); | ||
| } catch (e) { | ||
| // ignore if DOM not available in test environment | ||
| } | ||
| } |
There was a problem hiding this comment.
Nice touch in using a CSS class to manage animated background.
Could also consider combining this into one function and use a parameter to control whether to set or clear visual alarm, and set the default parameter value to "clear" or false.
cjyuan
added
Reviewed
|
Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it. |
3 participants
Learners, PR Template
Self checklist
Changelist
Added visual helpers: triggerVisualAlarm() and clearVisualAlarm()
Call triggerVisualAlarm() when the countdown reaches zero (and when a 0-second alarm is set)
Added a small load-time listener that attaches click handlers to the Set and Stop buttons to call clearVisualAlarm()
Questions
N/A