Skip to content

Glasgow | May-2025 | Mansoor Munawar | Sprint 3 | alarm clock #769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
MansoorM11 wants to merge 4 commits into from
Closed

Glasgow | May-2025 | Mansoor Munawar | Sprint 3 | alarm clock #769

MansoorM11 wants to merge 4 commits into from

Conversation

@MansoorM11
Copy link

@MansoorM11 MansoorM11 commented Aug 15, 2025 •
edited
Loading

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.
Created multiple functions in order for the alarm clock to work correctly according to set requirements

Questions

Ask any questions you have for your reviewer.

@MansoorM11 MansoorM11 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 15, 2025
cjyuan
cjyuan reviewed Aug 16, 2025
View reviewed changes
Copy link
Contributor

@cjyuan cjyuan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • Code works fine if a user only clicks the "Set Alarm" button once.
    However, if the user enters a time and then clicks the "Set Alarm" button multiple times, the countdown clock will not display properly.
    Can you fix the issue?

  • Some unusual input values can "spoil" the timer display. Can you add code to reject or convert this kind of values?

  • The Self-checklist in the PR description is not properly checked in MarkDown syntax. Can you fix the checkboxes?

@cjyuan cjyuan 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. labels Aug 16, 2025
@MansoorM11
Copy link
Author

MansoorM11 commented Aug 17, 2025

Hi CJ, I have now made the required adjustment based on the feedback. Thanks

@MansoorM11 MansoorM11 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 Aug 17, 2025
cjyuan
cjyuan reviewed Aug 17, 2025
View reviewed changes
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

Changes look good. You introduced a new bug to your code though.

Sprint-3/alarmclock/alarmclock.js Outdated
return;
}

if ((remainingSeconds > 86, 400)) {
Copy link
Contributor

@cjyuan cjyuan Aug 17, 2025

Choose a reason for hiding this comment

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

Have you tested input like 86400?

Copy link
Author

@MansoorM11 MansoorM11 Aug 17, 2025

Choose a reason for hiding this comment

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

Totally missed that. I fixed bug and now it should accept values upto 24 hours. Thanks

@cjyuan cjyuan 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 Aug 17, 2025
@MansoorM11 MansoorM11 mentioned this pull request Aug 17, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjyuan cjyuan cjyuan left review comments

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

@MansoorM11 @cjyuan @illicitonion