Implement stopwatch animation - #4223
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the visual regression tests are failing across all three BackstopJS scenarios, which indicates the rendered stopwatch does not match the expected baseline and blocks task completion. While your HTML and SCSS meet the functional and BEM requirements (correct structure, colors, sizes, keyframes, and speed-up modifier), the diffs (~0.81–0.84% vs a 0.4% threshold) show subtle appearance mismatches.
Critical issues to fix:
- Align the rendered visuals to the baseline expected by the tests. Inspect the Backstop report (
backstop_data/html_report/index.html) and the failed diffs to identify what differs (e.g., slight shifts in hand positions, border sizes, center dot size, or anti-aliasing/artifacts due to animation state). - Stabilize the test state so screenshots are deterministic. If animations are causing minor pixel drift, add a test CSS override to pause animations during capture or ensure the scenarios use a
readySelector/small delay before snapshot.
How to proceed:
- Compare your stopwatch dimensions, borders, and element positions against the screenshots and fine-tune margins, paddings, border widths/radii, and transform origins. Ensure the hands’
transform-originis exact and that the center element’s size/position matches the baseline. - If fonts are involved, ensure consistent loading (e.g.,
font-display: swapand waiting for font readiness) or switch to a system/monospace font if the baseline expects it. Only after visuals match should you consider updating the baseline, and only if that’s the team’s accepted workflow.
You did a great job implementing the required structure, colors, animations, and the speed-up modifier. Once the visual diffs are resolved, this should pass cleanly.
✨ 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
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The src/styles/index.scss file contains animation-delay: -0.17s on line 42 that creates an unintended visual offset for the seconds hand. According to the task requirements, both hands must start at the top (12 o'clock position), which is already correctly achieved through the keyframes using rotate(0deg). This negative delay is unnecessary and causes the visual regression test failures with ~0.80-0.83% differences. Remove the animation-delay: -0.17s property from the seconds hand animation to align with the baseline expectations. All other aspects of your implementation are correct and meet the 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
|
|
||
| animation: rotate-seconds 60s linear infinite; | ||
| animation-delay: -0.17s; |
There was a problem hiding this comment.
The animation-delay: -0.17s creates a visual offset for the seconds hand that causes regression test failures. The task requires both hands to start at the top (12 o'clock position), which is already achieved by the keyframes using rotate(0deg). This negative delay is unnecessary and creates a mismatch with the expected baseline.
DEMO LINK: https://yuran4ik13.github.io/layout_stop-watch/
REPOSITORY: https://github.com/yuran4ik13/layout_stop-watch
Checklist