soltuion withound speed version#4169
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because some core task requirements and checklist items are still not met, which will break the expected behavior and tests. You’re missing the required BEM modifier on the main element in src/index.html—main should have something like class="stopwatch stopwatch--speed-up" so that the speed-up mode is present for tests. In src/styles/blocks/stopwatch.scss, there is no .stopwatch--speed-up modifier that overrides the animation durations so the seconds hand completes a full circle in 10s and the minutes hand in 10min (600s); you need to add this modifier and adjust animation-duration for both hands inside it. The minutes hand currently uses a linear timing function; per the task, it must use steps(60, ...) over 3600s (and 600s under speed-up), so update its animation-timing-function accordingly. The seconds hand’s starting orientation is also inconsistent: with transform: $shift-to-center rotate(360deg); plus keyframes from rotate(0deg) to rotate(360deg), it does not clearly start at the top as required—align the base transform and keyframes so both hands visually point to 12 o’clock at the beginning. As a minor improvement, consider keeping the translation ($shift-to-center) outside the keyframes and animating only rotate(...) inside them to avoid conflicts when additional transforms are applied. Overall structure, sizing, and general animation setup look good; once you address these animation and modifier issues, your solution should fully satisfy the task.
✨ 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
| <body> | ||
| <h1>Stop watch</h1> | ||
| <body class="page"> | ||
| <main class="stopwatch"> |
There was a problem hiding this comment.
The main stopwatch element is missing the required BEM modifier for the speed-up behavior. The description requires adding a speed-up modifier to the stopwatch block and using it in HTML for tests, e.g. class="stopwatch stopwatch--speed-up", and corresponding CSS/SCSS that changes animation durations to 10s/10min.
| @keyframes move { | ||
| from { | ||
| transform: $shift-to-center rotate(0deg); |
There was a problem hiding this comment.
Checklist item #1 (“Keyframes implemented using from/to + transform with rotate property”) is satisfied, but combining $shift-to-center with rotate inside keyframes means any extra transform on elements (like the seconds hand) can cause incorrect start positions. This interacts with your seconds hand transform below and breaks the requirement that hands start at the top.
| width: 5vmin; | ||
| height: 5vmin; | ||
| border-radius: 50%; | ||
|
|
There was a problem hiding this comment.
The task requires adding a BEM modifier speed-up for the stopwatch block and also applying it in HTML for tests. Currently main has only class="stopwatch" without any stopwatch--speed-up (or similar), which violates the description requirement about the speed-up modifier.
| animation: move; | ||
| animation-duration: 3600s; | ||
| animation-timing-function: linear; |
There was a problem hiding this comment.
According to the description, the minutes hand must use a steps animation with 60 steps and take 60 minutes for a full circle. You have the correct duration (3600s), but the timing function is linear instead of steps(60, ...), so this doesn’t satisfy the requirement: "For minutes hand use steps animation with 60 steps."
| &__seconds-hand { | ||
| position: absolute; | ||
| z-index: 2; | ||
| transform-origin: bottom; | ||
| transform: $shift-to-center rotate(360deg); |
There was a problem hiding this comment.
The seconds hand is required to start at the top and rotate evenly over 60 seconds. With transform: $shift-to-center rotate(360deg); here and keyframes from rotate(0deg) to rotate(360deg), the initial visual position is not clearly aligned to the top. Consider adjusting the base rotation and keyframes so the hand points to 12 o’clock at animation start, as required by the description.
| animation-timing-function: linear; | ||
| animation-iteration-count: infinite; | ||
| } | ||
| } |
There was a problem hiding this comment.
The speed-up modifier must change animation durations so the seconds hand completes a full circle in 10s and the minutes hand in 10min (600s). There is no modifier block (e.g. .stopwatch--speed-up) here overriding animation-duration, which means this core requirement from the description is not implemented yet.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because it fully meets the functional requirements and checklist items for the CSS stopwatch. The stopwatch is correctly centered, sized to 80vmin with a 1vmin dotted border, and the hands and center circle match the specified dimensions and colors. Both hands start at the top, use only CSS animations, and run infinitely, with the seconds hand rotating linearly in 60s and the minutes hand using steps(60) over 3600s, satisfying [CHECKLIST ITEM #1] and [CHECKLIST ITEM #2]. The stopwatch--speed-up BEM modifier is correctly applied in HTML and SCSS, changing the seconds duration to 10s and the minutes duration to 600s without breaking the base behavior. Your BEM usage is consistent and there are no critical style or structural issues that would block task completion. As a very minor improvement, you could consider a more descriptive keyframes name than move (e.g., rotate-hand) for readability, but this is not required. Overall, this is a clean, correct implementation—well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
❗️ Copy this
Checklistto thePull Requestdescription after links, and put- [x]before each point after you checked it.Typical Mistakesfrom theBEMlesson theory are checked.