Skip to content

Glasgow | 26-ITP-May | Sandani Kannangara | Sprint 3 | Alarm Clock - #1276

Open
SandzSoft wants to merge 2 commits into
CodeYourFuture:mainfrom
SandzSoft:data-groups/sprint-3
Open

Glasgow | 26-ITP-May | Sandani Kannangara | Sprint 3 | Alarm Clock#1276
SandzSoft wants to merge 2 commits into
CodeYourFuture:mainfrom
SandzSoft:data-groups/sprint-3

Conversation

@SandzSoft

@SandzSoft SandzSoft commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title

  • My changes meet the requirements of the task

  • I have tested my changes

  • My changes follow the style guide

  • Implemented alarm clock features.

@github-actions

This comment has been minimized.

Sign in to view
@SandzSoft SandzSoft added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 21, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 21, 2026
@SandzSoft
SandzSoft force-pushed the data-groups/sprint-3 branch from 72ce6e9 to be8fbab Compare July 21, 2026 19:52
@github-actions

This comment has been minimized.

Sign in to view
@SandzSoft SandzSoft added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 21, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 21, 2026
@SandzSoft SandzSoft changed the title Glasgow | 26-ITP-May | Sandani Kannangara | sprint 3 | Data group Glasgow | 26-ITP-May | Sandani Kannangara | Sprint 3 | Alarm Clock Jul 21, 2026
@SandzSoft SandzSoft added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Data-Groups The name of the module. labels Jul 21, 2026
@SandzSoft SandzSoft mentioned this pull request Jul 22, 2026
Open
@SandzSoft SandzSoft added Core This is a core task and should be completed by all trainees and removed Core This is a core task and should be completed by all trainees labels Jul 22, 2026
@LonMcGregor LonMcGregor added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Jul 27, 2026
LonMcGregor
LonMcGregor reviewed Jul 27, 2026
View reviewed changes

@LonMcGregor LonMcGregor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start on this task, there are a few things to fix

Comment thread Sprint-3/alarmclock/alarmclock.js Outdated
let timer = setInterval(() => {
remainingSeconds--;
displayTimeRemaining();
if (remainingSeconds === 0) {

@LonMcGregor LonMcGregor Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the alarm clock stop correctly after it reaches zero?

Comment thread Sprint-3/alarmclock/alarmclock.js Outdated
remainingSeconds = Number(alarmTime.value);
displayTimeRemaining();

let timer = setInterval(() => {

@LonMcGregor LonMcGregor Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you start a timer when one is already running?

@SandzSoft SandzSoft Jul 27, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review Leon! Your comments helped me realize that my mental model of setInterval was wrong.

I originally thought setInterval was just another JavaScript function like map() or filter(). While fixing the review comments, I learned that setInterval() actually asks the browser to create and manage an interval timer, and the variable only stores the timer ID/reference returned by the browser. That understanding also helped me see why clearInterval(timer) works.

I also updated the code to prevent multiple timers from running at the same time and added input validation. Thanks again for the helpful feedback!

Comment thread Sprint-3/alarmclock/alarmclock.js
let remainingSeconds;

function setAlarm() {
remainingSeconds = Number(alarmTime.value);

@LonMcGregor LonMcGregor Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you properly handled your input validation?

Comment thread Sprint-3/alarmclock/alarmclock.js Outdated
@@ -1,4 +1,31 @@
function setAlarm() {}
const alarmTime = document.querySelector("#alarmSet");

@LonMcGregor LonMcGregor Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you know the id you are using, there is a better method to use than querySelector, do you know which?

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jul 27, 2026
@SandzSoft SandzSoft added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jul 27, 2026
LonMcGregor
LonMcGregor reviewed Jul 28, 2026
View reviewed changes

@LonMcGregor LonMcGregor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, this looks better. Task complete. Good explanation about how timers and intervals work in js.

@LonMcGregor LonMcGregor added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LonMcGregor LonMcGregor LonMcGregor left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Data-Groups The name of the module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SandzSoft @LonMcGregor