Skip to content

solution#4072

Open
Monggos111 wants to merge 2 commits intomate-academy:masterfrom
Monggos111:develop
Open

solution#4072
Monggos111 wants to merge 2 commits intomate-academy:masterfrom
Monggos111:develop

Conversation

@Monggos111
Copy link

Copy link

@Anton-Kuchmasov Anton-Kuchmasov left a comment

Choose a reason for hiding this comment

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

Well done!

Copy link

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

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 correctly implements all the required stopwatch features: the layout and sizing match the spec, the seconds and minutes hands have the right colors and dimensions, and the animations (60s linear for seconds, 3600s steps(60) for minutes, plus the .stopwatch--speed-up modifier) behave as required. The hands also rotate using @keyframes with from/to and transform: rotate(...), which satisfies the checklist.

One thing to improve in future work is how you handle transforms on the hands: in src/styles/main.scss and the generated CSS files, you use separate translate and rotate longhand properties, while your keyframes animate the transform shorthand. This can cause centering issues in some browsers because the animation may override the separate longhands. A more robust pattern is:

.stopwatch__seconds {
  transform: translate(-50%, 0) rotate(0deg);
}

@keyframes rotate-seconds {
  from { transform: translate(-50%, 0) rotate(0deg); }
  to   { transform: translate(-50%, 0) rotate(360deg); }
}

The same idea applies to the minutes hand. Overall, you met the task requirements well; treating transform as a single combined property will make your animations more reliable across environments in future projects.


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

Footnotes

  1. Rate AI review example

Comment on lines +38 to +39
translate: -50%;
rotate: 0deg;

Choose a reason for hiding this comment

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

The seconds hand uses separate longhand transform properties translate: -50%; and rotate: 0deg;. Using individual transform longhands can be unsupported or behave inconsistently across environments. Consider using a single transform, e.g. transform: translate(-50%, 0) rotate(0deg);, so the hand is reliably centered and starts pointing to the top.

Comment on lines +60 to +61
translate: -50%;
animation: rotate-minutes 3600s steps(60) infinite;

Choose a reason for hiding this comment

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

The minutes hand sets translate: -50%; but doesn't set an initial rotate. Even though the keyframes animate from rotate(0deg), explicitly setting a combined transform (for example transform: translate(-50%, 0) rotate(0deg);) ensures the starting position is at the top and avoids inconsistencies.

Comment on lines +37 to +39
translate: -50%;
rotate: 0deg;
animation: rotate-seconds 60s linear infinite;

Choose a reason for hiding this comment

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

The seconds hand uses separate translate: -50%; and rotate: 0deg; properties and the animation keyframes set transform: rotate(...). Because the animation sets the transform shorthand, it can override separate transform longhands or behave inconsistently across browsers, causing the hand to lose its horizontal centering while animating. Consider using a single transform combining translate and rotate on the element (for example transform: translate(-50%, 0) rotate(0deg);) and include the same translate in the keyframes from/to so centering is preserved during the animation. This keeps the starting position at the top and prevents visual jumps.

Comment on lines +59 to +60
translate: -50%;
animation: rotate-minutes 3600s steps(60) infinite;

Choose a reason for hiding this comment

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

The minutes hand also uses translate: -50%; without an explicit combined transform. For the same reason as above, animations that set transform may discard the separate translate. Use a combined transform (e.g. transform: translate(-50%, 0) rotate(0deg);) and include the translate in the minutes keyframes so the hand remains centered and starts at the top reliably while animating with steps(60).


.stopwatch__center{width:5vmin;height:5vmin;border-radius:100%;background-color:#f6a603;position:absolute;left:50%;top:50%;transform:translate(-50%, -50%);z-index:3}

.stopwatch__seconds{position:absolute;transform-origin:50% 100%;left:50%;bottom:50%;height:38vmin;width:1.5vmin;background-color:#2c8000;translate:-50%;rotate:0deg;animation:rotate-seconds 60s linear infinite}

Choose a reason for hiding this comment

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

The seconds hand uses translate:-50%;rotate:0deg;. Prefer a single transform value (e.g. transform: translate(-50%, 0) rotate(0deg);) to ensure consistent centering and initial rotation across browsers.


@keyframes rotate-seconds{from{transform:rotate(0deg)}to{transform:rotate(360deg)}}

.stopwatch__minutes{position:absolute;transform-origin:50% 100%;left:50%;bottom:50%;height:20vmin;width:3vmin;background-color:#0700ff;translate:-50%;animation:rotate-minutes 3600s steps(60) infinite}

Choose a reason for hiding this comment

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

The minutes hand uses translate:-50%; but has no explicit initial rotate. Add a combined transform such as transform: translate(-50%, 0) rotate(0deg); so the hand is centered and guaranteed to start at the top before the animation runs.

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.

3 participants