Skip to content

Conversation

@jyjiayi
Copy link

@jyjiayi jyjiayi commented Sep 9, 2021

Please fill out the survey before submitting the pull request. Thanks!

🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀

How many hours did you spend on this assignment? 5

Please fill in one error and/or error message you received while working on this assignment.
Still cannot fix bug of reset button shifting after clicking.

What part of the assignment did you spend the most time on?
stopwatch feature

Comfort Level (1-5): 3

Completeness Level (1-5): 3

What did you think of this deliverable? good recap of concepts

Is there anything in this code that you feel pleased about? -

Copy link

@michelle-mok michelle-mok left a comment

Choose a reason for hiding this comment

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

good work using setTimeout and setInterval!
consider adding more comments so that it is easier for the reader to follow your code

stopwatchTime.innerHTML = `${displayHMS(milliSecs)}`;
if (isStopPressed) {
clearInterval(stopwatchCount);
milliSecs -= 1000;

Choose a reason for hiding this comment

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

what does this line do? will your code still work without it?

stopwatchReset.classList.add("stopwatchButton");
stopwatchReset.addEventListener("click", () => {
milliSecs = 0;
isStopPressed = true;

Choose a reason for hiding this comment

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

isStopPressed needs to be true here for the stopwatch to start again

stopwatchReset.addEventListener("click", () => {
milliSecs = 0;
isStopPressed = true;
runStopWatch();

Choose a reason for hiding this comment

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

consider not running runStopWatch() here, instead, reset the timer display like this :
stopwatchTime.innerHTML = "00:00:00";
also consider adding a canClick variable so that the reset button cannot be clicked unless the stop button has already been clicked:
let canClick = false;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants