add catalog moyo #4847
add catalog moyo #4847
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! While your implementation covers many aspects of the task, there are a few critical issues that need to be addressed before we can approve the pull request:
-
Image Sources: All product cards currently use the same image source (
'../src/images/imac.jpeg'). If this is not intentional, please ensure each card has the correct image source to display the intended product images. -
Navigation Links: All navigation links have the same
hrefattribute ('./src'). Please update these links to point to the correct destination pages to ensure proper navigation functionality. -
BEM Naming Conventions: Ensure consistent use of BEM naming conventions, particularly changing
&-titleto&__titlefor consistency. -
SCSS Selectors: The
.bodyclass is used instead of thebodyelement selector. If you intend to style the HTML body element, please correct this to ensure styles are applied as intended.
These changes are crucial for the functionality and maintainability of your code. Once these issues are resolved, your work will be ready for approval. Keep up the great effort, and don't hesitate to reach out if you need further assistance! 😊
✨ 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.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link is-active" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute './src'. Ensure that each link points to the correct destination page.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute './src'. Ensure that each link points to the correct destination page.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute './src'. Ensure that each link points to the correct destination page.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute './src'. Ensure that each link points to the correct destination page.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute './src'. Ensure that each link points to the correct destination page.
| </div> | ||
| <div class="card"> | ||
| <img | ||
| src="../src/images/imac.jpeg" |
There was a problem hiding this comment.
The image source for all product cards is the same ('../src/images/imac.jpeg'). If this is not intentional, ensure each card has the correct image source.
| @@ -0,0 +1,9 @@ | |||
| .body { | |||
There was a problem hiding this comment.
The selector '.body' is targeting a class named 'body'. If you intend to style the HTML body element, use 'body' instead of '.body'.
src/styles/card.scss
Outdated
| padding-inline: 3px; | ||
| } | ||
|
|
||
| &-title { |
There was a problem hiding this comment.
The selector '&-title' should be '&__title' to follow the BEM naming convention consistently used elsewhere in the file.
| background-color: white; | ||
| border: 1px solid #00acdc; | ||
| color: #00acdc; | ||
| transition: 0.3s; |
There was a problem hiding this comment.
Consider adding a transition property to the main '.card' class to ensure consistent animation effects across all hover interactions.
| color: #00acdc; | ||
| } | ||
|
|
||
| .is-active { |
There was a problem hiding this comment.
Consider using a more descriptive class name instead of 'is-active' to avoid potential conflicts with other components. For example, 'nav__link--active' would be more specific and consistent with BEM naming conventions.
There was a problem hiding this comment.
Great effort on your submission! 🎉 While your code is on the right track, there are a few critical issues that need attention to ensure full functionality:
-
Image Sources: All product cards currently use the same image source (
'../src/images/imac.jpeg'). If this was not your intention, please ensure each card displays the correct product image. -
Navigation Links: All navigation links have the same
hrefattribute ('./src'). Update these links to point to the correct destination pages for proper navigation functionality. -
SCSS Selectors: The
.bodyclass is used instead of thebodyelement selector. If you intend to style the HTML body element, please correct this to ensure styles are applied as intended. -
BEM Naming Convention: Ensure consistent use of BEM naming conventions. Change
&__titleto&-titlefor consistency.
These issues should be addressed to enhance the quality and functionality of your code. Remember, addressing these points will help you improve and learn from this experience. Keep up the good work and continue to refine your skills! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link is-active" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute ('./src'). Please update these links to point to the correct destination pages to ensure proper navigation functionality.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute ('./src'). Please update these links to point to the correct destination pages to ensure proper navigation functionality.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute ('./src'). Please update these links to point to the correct destination pages to ensure proper navigation functionality.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute ('./src'). Please update these links to point to the correct destination pages to ensure proper navigation functionality.
| <li class="nav__item"> | ||
| <a | ||
| class="nav__link" | ||
| href="./src" |
There was a problem hiding this comment.
All navigation links have the same href attribute ('./src'). Please update these links to point to the correct destination pages to ensure proper navigation functionality.
| </div> | ||
| <div class="card"> | ||
| <img | ||
| src="../src/images/imac.jpeg" |
There was a problem hiding this comment.
All product cards currently use the same image source ('../src/images/imac.jpeg'). If this is not intentional, please ensure each card has the correct image source to display the intended product images.
| </div> | ||
| <div class="card"> | ||
| <img | ||
| src="../src/images/imac.jpeg" |
There was a problem hiding this comment.
All product cards currently use the same image source ('../src/images/imac.jpeg'). If this is not intentional, please ensure each card has the correct image source to display the intended product images.
| </div> | ||
| <div class="card"> | ||
| <img | ||
| src="../src/images/imac.jpeg" |
There was a problem hiding this comment.
All product cards currently use the same image source ('../src/images/imac.jpeg'). If this is not intentional, please ensure each card has the correct image source to display the intended product images.
| @@ -0,0 +1,9 @@ | |||
| .body { | |||
There was a problem hiding this comment.
The .body class is used instead of the body element selector. If you intend to style the HTML body element, please correct this to ensure styles are applied as intended.
| padding-inline: 3px; | ||
| } | ||
|
|
||
| &__title { |
There was a problem hiding this comment.
Ensure consistent use of BEM naming conventions. Change &__title to &-title for consistency.
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 ❗️