Skip to content

stopwatch#4175

Open
AdrJeeN wants to merge 2 commits into
mate-academy:masterfrom
AdrJeeN:develop
Open

stopwatch#4175
AdrJeeN wants to merge 2 commits into
mate-academy:masterfrom
AdrJeeN:develop

Conversation

@AdrJeeN

@AdrJeeN AdrJeeN commented Jul 1, 2026

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because the core stopwatch animation requirements are not yet met in the base state, which is a functional blocker. Right now, both .stopwatch__seconds-hand and .stopwatch__minutes-hand only get animation inside the stopwatch--speed-up modifier, so when mentors remove that modifier the stopwatch hands do not move at all, violating the basic functionality requirement and [CHECKLIST ITEM #1]. You need to add default animations: 60s linear infinite for .stopwatch__seconds-hand and 3600s steps(60, end) infinite for .stopwatch__minutes-hand, both using your existing from/to keyframes with transform: ... rotate(...). Then, in the stopwatch--speed-up modifier block, override only the durations so they become 10s for the seconds hand and 600s (10min) for the minutes hand, keeping the timing function and steps as specified. At the moment, the modifier uses the basic 60s/3600s durations, so it does not actually “speed up” the stopwatch and also mixes up where the base behavior should live. Visually and structurally you’ve done a good job with centering, sizes, colors, and BEM naming, so once you fix these animation durations and add the missing base animations, your solution should fully meet the task requirements.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/styles/index.scss
border-radius: 50%;
position: relative;

&__minutes-hand {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item Keyframes implemented using from/to + transform with rotate property for the minutes hand in base state: you never assign any animation to .stopwatch__minutes-hand without the speed-up modifier. You need a default animation using these keyframes so the watch works when mentors remove stopwatch--speed-up.

Comment thread src/styles/index.scss Outdated
Comment on lines +64 to +65
&--speed-up .stopwatch__minutes-hand {
animation: rotate-clock 3600s steps(60, end) infinite;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the requirement: For minutes hand use steps animation with 60 steps. It should take the 60min for the minutes hand to make a full circle. Currently, the only animation you set for the minutes hand uses 3600s but is applied inside the &--speed-up modifier, and there is no base animation at all. You should set 3600s steps(60, end) on .stopwatch__minutes-hand by default, and use the modifier to shorten it to 10min.

Comment thread src/styles/index.scss Outdated
Comment on lines +68 to +69
&--speed-up .stopwatch__seconds-hand {
animation: rotate-clock 60s linear infinite;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the requirement: For seconds hand use animation with even speed, without acceleration and deceleration. It should take the 60s for the seconds hand to make a full circle. The only animation for the seconds hand is inside the &--speed-up modifier, so there is no base 60s animation. You should add a default 60s linear infinite animation to .stopwatch__seconds-hand, and use the modifier to change duration to 10s.

Comment thread src/styles/index.scss Outdated
Comment on lines +64 to +69
&--speed-up .stopwatch__minutes-hand {
animation: rotate-clock 3600s steps(60, end) infinite;
}

&--speed-up .stopwatch__seconds-hand {
animation: rotate-clock 60s linear infinite;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the description requirement: In addition to the basic functionality create a BEM modifier called speed-up... it will take only 10s for the seconds hand ... it will take only 10min for the minutes hand. Here the modifier uses 3600s for minutes and 60s for seconds, which are the basic durations, not the speed-up ones. The modifier should override the base durations to 600s (10min) for minutes and 10s for seconds.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because it fully meets the functional and visual requirements of the stopwatch task. The stopwatch is correctly centered, sized to 80vmin, has the specified dotted circular border, and uses the required colors and dimensions for the minute and second hands as well as the center circle. Both hands are animated purely with CSS using @keyframes defined with from/to and transform: rotate(...), with the seconds hand using a 60s linear infinite rotation and the minutes hand using 3600s steps(60, end) infinite, matching the specification. The stopwatch--speed-up BEM modifier correctly overrides only the animation-duration for the hands to 10s and 600s, so the base stopwatch works without the modifier and the modifier genuinely speeds up the animation. The starting position of the hands at the top and continuous infinite animation behavior are implemented correctly. Overall, your code is clean, BEM naming is consistent, and there are no critical issues blocking task completion. Nicely done on aligning the implementation closely with the checklist and fixing the earlier concerns about base animations and modifier behavior.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

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