Skip to content
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

London 11 | Pezhman-Azizi | Data-Groups | sprint 3 | Alarm Clock #275

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

Conversation

Pezhman-Azizi
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 COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

This PR adds the core functionality for the alarm clock, including a countdown timer, alarm sound at 00:00, and a stop button to pause the alarm. It also fixes the testing setup by installing the missing @testing-library/jest-dom dependency, ensuring the test environment works correctly.

Questions

Is the way I update the DOM for the countdown timer efficient, or could it be improved? Are there better practices for handling events or updating elements that I might have missed? Does the interaction between the HTML and JavaScript look clean and easy to maintain? Any suggestions to make the DOM-related parts of the project more efficient or user-friendly?

2- Plays alarm at 00:00 and stops with the button.
3- Basic input validation included.
Installed @testing-library/jest-dom to resolve test errors.
Updated and verified the test environment is working smoothly.
@Pezhman-Azizi Pezhman-Azizi added the Needs Review Participant to add when requesting review label Jan 4, 2025

Choose a reason for hiding this comment

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

Your script has some duplicate code lines, please fix it. It's causing some unexpected behaviour.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks very much for taking time reviewing my approach. I just removed the duplicated parts to prevent unexpected behaviors.

@KudahShambare KudahShambare added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Jan 7, 2025
@Pezhman-Azizi Pezhman-Azizi added Complete Participant to add when work is complete and review comments have been addressed and removed Complete Participant to add when work is complete and review comments have been addressed labels Jan 16, 2025
@Pezhman-Azizi Pezhman-Azizi added Complete Participant to add when work is complete and review comments have been addressed and removed Reviewed Volunteer to add when completing a review labels Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Participant to add when work is complete and review comments have been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants