Skip to content

Sheffield| May 2025| Mayowa Fadare| Sprint 3| Alarm Clock #712

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.

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mayowa0-7
Copy link

Learners, PR Template

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.

Questions

Ask any questions you have for your reviewer.

@mayowa0-7 mayowa0-7 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 26, 2025
@arun-john arun-john added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. 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 Aug 1, 2025
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.

  • On page load, the clock shows 10 seconds. Why starts at 10 seconds?
  • When the clock countdowns to 0, the input field is disabled. Once disabled, I could not figure out a way to enable it.
image
  • I don't understand how the interface work.
    • "Start Alarm" and "Pause Alarm" buttons are disabled at all times.
    • What is the purpose of the "Flash Background" button?
    • Only three of the buttons are assigned an event listener. That means four of the buttons won't respond to mouse click (even when they are enabled).
    • While the clock is counting down, clicking "Stop Alarm" follow by clicking "Resume Alarm" will trigger the alarm sound even though the countdown has not yet reached zero.

Comment on lines 104 to 57
if (typeof window !== "undefined") {
window.onload = setup;
window.setup = setup;
window.setAlarm = setAlarm;
window.pauseAlarm = pauseAlarm;
window.resumeAlarm = resumeAlarm;
window.playAlarm = playAlarm;
window.flashBackground = flashBackground;
window.updateTimeDisplay = updateTimeDisplay;
window.formatTime = formatTime;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for? Can the code work equally well without this code?

Copy link
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, Start and Stop button now working as expected, every other button removed.

@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 1, 2025
@mayowa0-7 mayowa0-7 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 2, 2025
@mayowa0-7 mayowa0-7 force-pushed the Alarmclock-Sprint-3 branch from 582910b to 3272128 Compare August 2, 2025 14:15
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.

App is working good now.

@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 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and all review comments have been addressed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants