add task solution #5089
add task solution #5089
Conversation
veneracode
commented
Dec 19, 2024
veneracode
commented
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 However, there are a few critical issues that need to be addressed before we can approve the pull request:
-
SCSS Usage: The task requires linking SCSS files directly in HTML, but your implementation links to a CSS file instead. Please ensure SCSS files are used and linked correctly.
-
BEM Blocks: The task specifies separate files for
cardandstarsBEM blocks, which are not present in your submission. Please create these files. -
SCSS Nesting and Variables: The task requires SCSS nesting and the use of SCSS variables, which are missing in your current implementation. Please incorporate these elements.
-
Stars Block: The
starsblock needs to be rewritten using SCSS as per the task requirements. -
Font Usage: A specific font from Google Fonts should be used, but it appears to be missing. Please include this in your implementation.
-
File Structure: Ensure there is a separate SCSS file for variables as required.
-
Image Path: The image paths should be relative to
index.scss, starting with../images. Please adjust this accordingly.
Once these issues are resolved, we can proceed with the approval. 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/index.html
Outdated
| <p class="card__info"> | ||
| Product code: 195434 | ||
| </p> | ||
| <div class="rewiews"></div> |
There was a problem hiding this comment.
The class name 'rewiews' is likely a typo. It should be 'reviews' to maintain consistency and avoid confusion.
src/index.html
Outdated
| <div class="stars star"></div> | ||
| </div> | ||
|
|
||
| <div class="rewiews__text">Rewiews:5</div> |
There was a problem hiding this comment.
The class name 'rewiews__text' contains a typo. It should be 'reviews__text'.
src/index.html
Outdated
| <h1>Product cards</h1> | ||
| <article class="card" data-qa="card"> | ||
| <div> | ||
| <img src="images/imac.jpeg" alt="imacPh"> |
There was a problem hiding this comment.
The 'alt' attribute for the image is not very descriptive. Consider providing a more detailed description to improve accessibility, such as 'Image of an Apple A1419 iMac 27" Retina 5K Monoblock'.
src/styles/index.css
Outdated
| background-image: url(../images/star-active.svg); | ||
| } | ||
|
|
||
| .rewiews { |
There was a problem hiding this comment.
The class name 'rewiews' is misspelled. It should be 'reviews'.
src/styles/index.css
Outdated
| justify-content: space-between; | ||
| } | ||
|
|
||
| .rewiews__text { |
There was a problem hiding this comment.
The class name 'rewiews__text' contains a typo. It should be 'reviews__text'.
src/styles/index.css
Outdated
| height: 134px; | ||
| display: block; | ||
| margin: 0 auto 40px; | ||
| display: block; |
There was a problem hiding this comment.
The 'display: block;' property is declared twice for the 'img' selector. You can remove the redundant declaration.
There was a problem hiding this comment.
Great progress on your project! 🎉 While there are a few areas for improvement, such as creating a variables.scss file for SCSS variables, ensuring correct image paths relative to index.scss, and maintaining consistency in BEM naming conventions, your work is on the right track. Remember to configure the Parcel bundler for SCSS compilation. Addressing these points will enhance your code's maintainability and functionality. Keep up the good work and learn from these insights to refine your skills further! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Roboto:wght@400;500;700&display=swap" | ||
| rel="stylesheet" | ||
| /> | ||
| <link | ||
| rel="stylesheet" | ||
| href="./styles/index.scss" |
There was a problem hiding this comment.
Ensure that the Parcel bundler is configured to compile SCSS files, as linking SCSS directly in HTML requires this setup.
| <div> | ||
| <img | ||
| class="card__imagep" | ||
| src="images/imac.jpeg" |
There was a problem hiding this comment.
The image path should be relative to index.scss, starting with ../images. Please adjust this path accordingly.
| padding: 32px 16px 16px; | ||
| box-sizing: border-box; | ||
| border-radius: 5px; | ||
| background-color: #fff; |
There was a problem hiding this comment.
Consider using SCSS variables for colors, font sizes, and other repeated values to enhance maintainability and readability.
| color: #616070; | ||
| } | ||
|
|
||
| &_imagep { |
There was a problem hiding this comment.
The BEM naming convention should be consistent. Consider renaming &_imagep to &__image to follow the BEM standard.
| height: 16px; | ||
| width: 16px; | ||
| background-image: url(../images/star.svg); | ||
| background-repeat: no-repeat; | ||
| margin-right: 4px; | ||
| background-position: center; |
There was a problem hiding this comment.
Consider using SCSS variables for dimensions and margins to enhance maintainability and readability.
| background-image: url(../images/star.svg); | ||
| background-repeat: no-repeat; | ||
| margin-right: 4px; | ||
| background-position: center; | ||
| } | ||
|
|
||
| &__star:last-child { | ||
| margin-bottom: 0; | ||
| } | ||
|
|
||
| &__star:nth-child(-n + 4) { | ||
| background-image: url(../images/star-active.svg); |
There was a problem hiding this comment.
Ensure that the image paths are correct and relative to the SCSS file location. Verify that the images exist at the specified paths.
| @import './blocks/card'; | ||
| @import './blocks/stars'; |
There was a problem hiding this comment.
Consider creating a variables.scss file for SCSS variables and import it here to enhance maintainability and consistency across styles.
