Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add task solution #3591

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eugenia-andropova
Copy link

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.

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. The speed-up modifier should correctly set the animation duration to 10s for the seconds hand and 600s for the minutes hand, as per the task requirements. Additionally, the BEM modifier should be applied to the stopwatch block, not nested within the elements. Lastly, there's a typo in the class name for the second hand within the speed-up block that needs correction. Please make these changes, and you'll be all set! Keep up the great work! 🚀

✨ 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.

animation-duration: 600s / 6;
}

&__seconds-hand {

Choose a reason for hiding this comment

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

There is a typo in the class name &__seconds-hand. It should be &__second-hand to match the class defined earlier in the file.

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.

Thank you for your submission! 🎉 While your code is mostly correct, there are a few critical issues that need attention:

  1. Speed-Up Modifier: Ensure the speed-up modifier is applied directly to the stopwatch block as stopwatch--speed-up, not nested within elements. This will align with the task requirements.

  2. Animation Duration: The speed-up modifier should set the animation duration to 10s for the seconds hand and 600s for the minutes hand. Currently, the division by 6 is incorrect.

  3. Class Name Consistency: Correct the typo in the class name for the speed-up modifier in the HTML file from stopwatch--sped-up to stopwatch--speed-up.

  4. Class Naming for Hands: Ensure the class name for the second hand is stopwatch__seconds-hand to match the task requirements.

These adjustments are crucial for the functionality of your code. Please review the comments and make the necessary changes to improve your solution. Keep up the great work and continue learning from these experiences! 🚀

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

@@ -17,6 +17,10 @@
/>
</head>
<body>
<h1>Stop watch</h1>
<div class="stopwatch stopwatch--sped-up">

Choose a reason for hiding this comment

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

The BEM modifier stopwatch--sped-up is incorrectly named. According to the task requirements, it should be stopwatch--speed-up.

@@ -17,6 +17,10 @@
/>
</head>
<body>
<h1>Stop watch</h1>
<div class="stopwatch stopwatch--sped-up">
<div class="stopwatch__second-hand"></div>

Choose a reason for hiding this comment

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

The class name for the second hand should be stopwatch__seconds-hand to match the task requirements.

Comment on lines +59 to +67
&--speed-up {
&__minutes-hand {
animation-duration: 600s / 6;
}

&__second-hand {
animation-duration: 10s / 6;
}
}

Choose a reason for hiding this comment

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

The BEM modifier &--speed-up is correctly named here, but it should not be nested within the elements. According to the task requirements, the modifier should be applied directly to the stopwatch block.

animation: rotate 600s steps(60) infinite;
}

&__second-hand {

Choose a reason for hiding this comment

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

The class name for the second hand should be &__seconds-hand to match the task requirements.

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