stop watch 05.02.2025 #3621
stop watch 05.02.2025 #3621
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 There are a few critical issues that need to be addressed before we can approve the pull request:
-
Class Naming: The class
stopwatchshould be applied to the main stopwatch element, not the<html>tag. This is essential for the functionality and styling to work as intended. -
Animation Keyframes: The task specifies using
from/tokeyframes with thetransformproperty androtate. Currently, the SCSS usesrotate: 450deg;directly, which does not meet the requirement. Please adjust this to use thetransformproperty. -
Variable Naming: There's a typo in the SCSS variable name
$speed-variabl. It should be corrected to$speed-variableto ensure consistency and avoid confusion. -
BEM Modifier: Ensure that the BEM modifier
speed-upis applied correctly in both HTML and SCSS. This is crucial for maintaining the code's structure and readability.
Please address these issues to fully meet the task requirements. Once these changes are made, your code will be in great shape! Keep up the great work, and don't hesitate to reach out if you need any help. 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
src/styles/index.scss
Outdated
| @@ -1,3 +1,78 @@ | |||
| $speed-variabl: 60s; | |||
There was a problem hiding this comment.
There is a typo in the variable name $speed-variabl. It should be $speed-variable to match the naming convention and avoid confusion.
src/styles/index.scss
Outdated
| position: relative; | ||
|
|
||
| &--speed-up { | ||
| $speed-variabl: 10s !global; |
There was a problem hiding this comment.
The use of !global is not necessary here. It can lead to unexpected behavior by altering the global scope. Consider removing it unless you have a specific reason to use it.
There was a problem hiding this comment.
Review Summary
Great job on your progress! 🎉 While your code is mostly on the right track, there are a few critical issues that need attention:
-
Class Naming: The class
stopwatchis currently applied to the<body>tag. It should be moved to the main stopwatch element to ensure proper functionality and styling. -
BEM Modifier: The BEM modifier
speed-upshould be applied correctly. Ensure that the HTML and SCSS use this modifier consistently. In the SCSS, it should be nested under the main block class, not as a separate block.
Despite these issues, your improvements in animation keyframes, variable naming, and styling are commendable. The decision to approve this pull request is made to help you move forward with your learning. Please review the comments and address these issues in future iterations to enhance your code quality. Keep up the good work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/index.html
Outdated
| @@ -16,7 +16,11 @@ | |||
| href="./styles/index.scss" | |||
| /> | |||
| </head> | |||
| <body> | |||
| <h1>Stop watch</h1> | |||
| <body class="stopwatch"> | |||
There was a problem hiding this comment.
The class stopwatch should be applied to the main stopwatch element instead of the <body> tag. Please move this class to the appropriate element to ensure proper functionality and styling.
| &--speed-up { | ||
| $speed-variable: 10s; | ||
| } |
There was a problem hiding this comment.
The BEM modifier speed-up should be applied correctly. Ensure that the HTML and SCSS use this modifier consistently. In the SCSS, it should be nested under the main block class, not as a separate block.
DEMO LINK
[x ] Keyframes implemented using from/to + transform with rotate property
[x ] Stopwatch is centered and has the correct arrows size
[x ] All
Typical Mistakesfrom theBEMlesson theory are checked.[x ] Code follows all the Code Style Rules ❗️