add task solution #5154
add task solution #5154
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. The data-qa attributes are not implemented correctly, and the navigation link color does not meet the specified requirements. Additionally, the hover effects need to be adjusted to ensure smooth transitions and correct color changes. Please address these issues to align with the task requirements. Keep up the great work, and I'm confident you'll nail it with these adjustments! 🚀
✨ 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.
| width: 200px; | ||
| border: 1px solid #f3f3f3; | ||
| border-radius: 5px; | ||
| background: $bg-color; |
There was a problem hiding this comment.
Ensure that the SCSS variable $bg-color is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
| font-weight: 500; | ||
| height: 36px; | ||
| width: 166px; | ||
| color: $main-accent; |
There was a problem hiding this comment.
Ensure that the SCSS variable $main-accent is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
src/styles/blocks/card.scss
Outdated
|
|
||
| &:hover { | ||
| transform: scale(1.2); | ||
| transition: $transition; |
There was a problem hiding this comment.
Ensure that the SCSS variable $transition is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
| font-size: 10px; | ||
| font-weight: 400; | ||
| line-height: 14px; | ||
| color: $secondary; |
There was a problem hiding this comment.
Ensure that the SCSS variable $secondary is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
|
|
||
| &__rate { | ||
| font-weight: 400; | ||
| color: $main-accent; |
There was a problem hiding this comment.
Ensure that the SCSS variable $main-accent is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
| background: #fff; | ||
| border: 1px solid #00acdc; | ||
| border-radius: 5px; | ||
| transition: $transition; |
There was a problem hiding this comment.
Ensure that the SCSS variable $transition is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
| display: flex; | ||
|
|
||
| & :hover { | ||
| color: $blue-color; |
There was a problem hiding this comment.
Ensure that the SCSS variable $blue-color is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
| text-decoration: none; | ||
| text-transform: uppercase; | ||
| position: relative; | ||
| color: $blue-color; |
There was a problem hiding this comment.
Ensure that the SCSS variable $blue-color is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
| position: absolute; | ||
| height: 4px; | ||
| border-radius: 8px; | ||
| background-color: $blue-color; |
There was a problem hiding this comment.
Ensure that the SCSS variable $blue-color is defined elsewhere in your stylesheets. If it's not defined, it will cause a compilation error.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: 7]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: 7]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: 7]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: 7]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
src/index.html
Outdated
| <div class="card__title"> | ||
| APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
| </div> |
There was a problem hiding this comment.
You need to use the semantic tag for text, fix it everywhere
src/styles/blocks/header.scss
Outdated
| @@ -0,0 +1,73 @@ | |||
| header { | |||
src/index.html
Outdated
| <div class="card__price-label">Price:</div> | ||
|
|
||
| <div class="card__price-tag">$2,199</div> |
There was a problem hiding this comment.
You need to use semantic tags for text everywhere
| <div class="card__price-label">Price:</div> | |
| <div class="card__price-tag">$2,199</div> | |
| <p class="card__price-label">Price:</p> | |
| <p class="card__price-tag">$2,199</p> |
src/index.html
Outdated
| <div class="stars__star"></div> | ||
| <div class="stars__star"></div> | ||
| </div> | ||
| Reviews: 5 |
| class="card" | ||
| data-qa="card" | ||
| > | ||
| <div class="card__content"> |
There was a problem hiding this comment.
Do you need this wrapper? Consider it's redundant
| <div class="card__content"> |
There was a problem hiding this comment.
I consider it necessary. If I remove it, here is what happens:
The card content is not positioned as it should be.
DEMO LINK
TEST REPORT LINK
All components follow BEM and use SCSS
Repeated sizes and special colors are put to variables
Grid is used for the columns
Cards are shown in 1, 2, 3, or 4 columns based on screen resolution
All changes on
:hoverare smoothCode follows all the Code Style Rules ❗️