Skip to content

London | 26-ITP-Jan | Kris Oldrini | Sprint 3 | Alarm Clock App#1208

Open
XiaoQuark wants to merge 7 commits intoCodeYourFuture:mainfrom
XiaoQuark:49-build-an-alarm-clock-app
Open

London | 26-ITP-Jan | Kris Oldrini | Sprint 3 | Alarm Clock App#1208
XiaoQuark wants to merge 7 commits intoCodeYourFuture:mainfrom
XiaoQuark:49-build-an-alarm-clock-app

Conversation

@XiaoQuark
Copy link
Copy Markdown

@XiaoQuark XiaoQuark commented Apr 20, 2026

Learners, PR Template

Self checklist

  • 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

Changelist

HTML

  • Changed title to "Alarm clock app"

Script

  • Implemented setAlarm() function to display the inputted time in the UI
  • Created formatTime() to convert seconds into MM:SS format
  • Store the time inside state, for easier access across functions
  • Implemented timer() function, using setInterval(), that decreases the time every 1000 millisecond
  • Triggered playAlarm() when the countdown reaches 00:00
  • Ensured the display correctly shows 00:00 when the alarm triggers
  • Prevented multiple intervals by clearing any existing timer before starting a new one

Test

  • Manually tested functionality in the browser
  • Run and passed all Jest tests

Extra Tasks

  • Changed background colour when the alarm triggers
  • Reset background colour when alarm is stopped

@XiaoQuark XiaoQuark added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 20, 2026
A-O-Emmanuel
A-O-Emmanuel reviewed Apr 20, 2026
View reviewed changes
Copy link
Copy Markdown

@A-O-Emmanuel A-O-Emmanuel left a comment

Choose a reason for hiding this comment

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

This is a good attempt, but i noticed the following things:

  1. When I click the set alarm button the timer starts counting backwards.
  2. The inpput field accepts negative numbers.

@A-O-Emmanuel A-O-Emmanuel added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. 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 Apr 20, 2026
@XiaoQuark
Copy link
Copy Markdown
Author

XiaoQuark commented Apr 20, 2026

@A-O-Emmanuel

Thank you for the feedback. I implemented input validation so so the timer only accepts positive integers, and cannot count below 00:00. I also added a small inline error message for invalid inputs.

@XiaoQuark XiaoQuark 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 Apr 20, 2026
@XiaoQuark
Copy link
Copy Markdown
Author

XiaoQuark commented Apr 22, 2026

Hello @A-O-Emmanuel
I was wondering if you had the chance to check the changes I did.
As the deadline for submitting is tomorrow, do you think the PR could be marked as completed? Or is there anything else I need to do?

illicitonion
illicitonion reviewed Apr 23, 2026
View reviewed changes
Comment thread Sprint-3/alarmclock/alarmclock.js
state.remainingTime = +timeInput.value;
if (state.remainingTime <= 0 || !Number.isInteger(state.remainingTime)) {
state.remainingTime = 0;
displayedTime.textContent = `Time Remaining: 00:00`;
Copy link
Copy Markdown
Member

@illicitonion illicitonion Apr 23, 2026

Choose a reason for hiding this comment

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

I've noticed you have three different places where you set this content to "Time remaining: XX:YY" - imagine you wanted to change this in some way (e.g. "Time Left: XX:YY" or similar) - you'd need to change all three. Can you think how to avoid that?

Copy link
Copy Markdown
Author

@XiaoQuark XiaoQuark Apr 23, 2026

Choose a reason for hiding this comment

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

Thank you very much Daniel.
I will have a look at this as soon as I'm done with all my submissions.

illicitonion
illicitonion reviewed Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

I'm going to mark this as complete, but I left one bonus thing to consider :)

@illicitonion illicitonion 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 Apr 23, 2026
@XiaoQuark XiaoQuark mentioned this pull request Apr 23, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@illicitonion illicitonion illicitonion left review comments

+1 more reviewer

@A-O-Emmanuel A-O-Emmanuel A-O-Emmanuel left review comments

Reviewers whose approvals may not affect merge requirements

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@XiaoQuark @illicitonion @A-O-Emmanuel