add task solution again #4980
add task solution again #4980
Conversation
There was a problem hiding this comment.
Your submission has been reviewed and approved! 🎉 While there are a few areas for improvement, you've done a great job overall. Here are some key points to consider for future enhancements:
-
Data Attributes: Ensure the
data-qaattribute for the "Buy" button is corrected todata-qa="card-hover". -
SCSS Variables: Make sure all repeated sizes and special colors are defined in variables for better maintainability.
-
Hover Effects: Verify that all specified hover effects are applied correctly, such as changing the card title text color and button background.
-
Responsive Design: Consider using relative units or media queries for elements like line-height, logo positioning, and container dimensions to improve responsiveness.
-
Dynamic Content: For repeated elements like product names, prices, and star ratings, consider using a dynamic approach to avoid hardcoding and make updates easier.
-
Background and Transform Properties: Use relative units or media queries for background properties and consider a smaller scale value for transforms to maintain layout integrity.
These improvements will help you create more flexible and maintainable code. 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! ✨
| <h2 class="card__apple"> | ||
| APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) |
There was a problem hiding this comment.
The product name is hardcoded in multiple places. Consider using a templating engine or JavaScript to dynamically generate these elements to avoid repetition and make updates easier.
| APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
| </h2> | ||
|
|
||
| <p class="card__product">Product code: 195434</p> |
There was a problem hiding this comment.
The product code is repeated across multiple cards. If these are meant to be different products, ensure each card has unique content.
| <div class="stars stars--4"> | ||
| <div class="stars__image"></div> | ||
| <div class="stars__image"></div> | ||
| <div class="stars__image"></div> | ||
| <div class="stars__image"></div> | ||
| <div class="stars__image"></div> | ||
| <div class="stars__text">Reviews: 5</div> |
There was a problem hiding this comment.
The star rating is hardcoded as 'stars--4' and 'Reviews: 5' across all cards. If these are meant to represent different products, consider making these values dynamic.
| <div class="card__price"> | ||
| <p class="card__price_text">Price:</p> | ||
| <p class="card__price_amount">$2,199</p> |
There was a problem hiding this comment.
The price is hardcoded as '$2,199' across all cards. If these are meant to represent different products, ensure each card has a unique price.
| <button | ||
| class="card__button card__button--text" | ||
| data-qa="card-hover" | ||
| > | ||
| BUY |
There was a problem hiding this comment.
The 'BUY' button is repeated across all cards. Consider using a loop or a component-based approach to generate these cards dynamically if the content is similar.
| @@ -0,0 +1,65 @@ | |||
| .header { | |||
| $blue: #00acdc; | |||
There was a problem hiding this comment.
The $blue variable is defined within the .header scope. If this color is used across multiple components, consider moving it to a global variables file for better maintainability.
| position: absolute; | ||
| margin: 0 auto; | ||
| top: 10px; | ||
| left: 50px; |
There was a problem hiding this comment.
The logo is positioned absolutely with fixed values for top and left. This might cause issues on smaller screens or when the layout changes. Consider using relative positioning or media queries to ensure the logo is positioned correctly across different screen sizes.
|
|
||
| &__link { | ||
| display: block; | ||
| line-height: 60px; |
There was a problem hiding this comment.
The line-height is set to 60px, which might be too large for smaller screens. Consider using relative units or media queries to adjust the line-height for better responsiveness.
| line-height: 16px; | ||
| padding: 16px 16px 0; | ||
|
|
||
| &--4 :nth-child(-n + 4) { |
There was a problem hiding this comment.
The selector '&--4 :nth-child(-n + 4)' is used to apply styles to the first four children. Ensure that this selector matches the intended HTML structure, as it might not work as expected if the number of children changes.
| padding: 16px 16px 0; | ||
|
|
||
| &--4 :nth-child(-n + 4) { | ||
| background-image: url(../images/star-active.svg); |
There was a problem hiding this comment.
The background image URL is hardcoded. If the image is meant to be dynamic or changeable, consider using a variable or a mixin to handle different images.
DEMO LINK
TEST REPORT LINK
[x ] All components follow BEM and use SCSS
[x ] Repeated sizes and special colors are put to variables
Grid is used for the columns
[x ] Cards are shown in 1, 2, 3, or 4 columns based on screen resolution
[x ] All changes on
:hoverare smooth[x ] Code follows all the Code Style Rules ❗️