create products catalog #4931
create products catalog #4931
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. These include inconsistencies in data-qa attributes, missing hover effects, and the need for smooth transitions. Additionally, ensure that all mixins and imported files are correctly defined and referenced to avoid compilation errors. Once these issues are resolved, your code will be in great shape! 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.
| </div> | ||
| <button | ||
| class="card__button" | ||
| data-qa="card-hover" |
There was a problem hiding this comment.
The data-qa attribute here is card-hover, while in other similar buttons it is just hover. For consistency, consider using the same data-qa attribute across all similar elements unless there is a specific reason for the difference.
src/index.html
Outdated
| </div> | ||
| <button | ||
| class="card__button" | ||
| data-qa="hover" |
There was a problem hiding this comment.
The data-qa attribute here is hover, which is inconsistent with the card-hover used in another button. Ensure consistency in data-qa attributes unless different identifiers are needed for testing purposes.
src/index.html
Outdated
| </div> | ||
| <button | ||
| class="card__button" | ||
| data-qa="hover" |
There was a problem hiding this comment.
The data-qa attribute here is hover, which is inconsistent with the card-hover used in another button. Ensure consistency in data-qa attributes unless different identifiers are needed for testing purposes.
src/index.html
Outdated
| </div> | ||
| <button | ||
| class="card__button" | ||
| data-qa="hover" |
There was a problem hiding this comment.
The data-qa attribute here is hover, which is inconsistent with the card-hover used in another button. Ensure consistency in data-qa attributes unless different identifiers are needed for testing purposes.
src/index.html
Outdated
| </div> | ||
| <button | ||
| class="card__button" | ||
| data-qa="hover" |
There was a problem hiding this comment.
The data-qa attribute here is hover, which is inconsistent with the card-hover used in another button. Ensure consistency in data-qa attributes unless different identifiers are needed for testing purposes.
| } | ||
|
|
||
| &__price-label { | ||
| @include font($size: 12px, $line-height: 18px, $color: $secondary-color); |
There was a problem hiding this comment.
Verify that the mixin font is defined in your mixins file. If it's not defined, this will cause an error during SCSS compilation.
| } | ||
|
|
||
| &__price-value { | ||
| @include font($size: 16px, $weight: 700, $line-height: 18px); |
There was a problem hiding this comment.
Verify that the mixin font is defined in your mixins file. If it's not defined, this will cause an error during SCSS compilation.
| } | ||
|
|
||
| &__button { | ||
| @include font(14px, 700, 16px, $white-color); |
There was a problem hiding this comment.
Verify that the mixin font is defined in your mixins file. If it's not defined, this will cause an error during SCSS compilation.
| } | ||
|
|
||
| .stars { | ||
| @include flex($direction: row, $items: center); |
There was a problem hiding this comment.
Ensure that the mixin flex is defined in your mixins file. If it's not defined, this will cause an error during SCSS compilation.
| @include flex($direction: row, $items: center); | ||
|
|
||
| &__star { | ||
| @include set-size(16px, 16px); |
There was a problem hiding this comment.
Ensure that the mixin set-size is defined in your mixins file. If it's not defined, this will cause an error during SCSS compilation.
There was a problem hiding this comment.
Great job on your submission! 🎉 Your code meets most of the task requirements, and it's clear you've put in a lot of effort. Here are a few points to consider for improvement:
-
Hover Effects and Transitions: Ensure that all hover effects have a smooth transition duration of 300ms as required. This will enhance the user experience.
-
Data-qa Attributes: Make sure that
data-qaattributes are consistently applied across all similar elements, such as buttons and navigation links, to avoid confusion in testing or automation scripts. -
SCSS Variables: It's good to see the use of variables for colors. Ensure that all repeated sizes and special colors are also consistently defined as variables for maintainability.
-
Mixins and Imports: Verify that all mixins like
flex,font, andset-sizeare correctly defined and that imported files are available to prevent compilation errors.
Your adherence to BEM methodology and SCSS is commendable. Keep up the great work, and make sure to address these points in future projects to enhance your skills further. Happy coding! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <li class="nav__item"> | ||
| <a | ||
| href="#Smartphones" | ||
| class="nav__link" |
There was a problem hiding this comment.
The data-qa attribute is missing for this navigation link. Ensure consistency by adding data-qa attributes to all similar elements if needed for testing or automation.
| <li class="nav__item"> | ||
| <a | ||
| href="#gadgets" | ||
| class="nav__link" |
There was a problem hiding this comment.
The data-qa attribute is missing for this navigation link. Ensure consistency by adding data-qa attributes to all similar elements if needed for testing or automation.
| <li class="nav__item"> | ||
| <a | ||
| href="#tablets" | ||
| class="nav__link" |
There was a problem hiding this comment.
The data-qa attribute is missing for this navigation link. Ensure consistency by adding data-qa attributes to all similar elements if needed for testing or automation.
| <li class="nav__item"> | ||
| <a | ||
| href="#photo" | ||
| class="nav__link" |
There was a problem hiding this comment.
The data-qa attribute is missing for this navigation link. Ensure consistency by adding data-qa attributes to all similar elements if needed for testing or automation.
| <li class="nav__item"> | ||
| <a | ||
| href="#video" | ||
| class="nav__link" |
There was a problem hiding this comment.
The data-qa attribute is missing for this navigation link. Ensure consistency by adding data-qa attributes to all similar elements if needed for testing or automation.
| } | ||
|
|
||
| &__price-label { | ||
| @include font($size: 12px, $line-height: 18px, $color: $secondary-color); |
There was a problem hiding this comment.
Verify that the mixin 'font' is correctly defined in the imported mixins file. Incorrect or missing mixin definitions can cause compilation errors.
| } | ||
|
|
||
| &__price-value { | ||
| @include font($size: 16px, $weight: 700, $line-height: 18px); |
There was a problem hiding this comment.
Verify that the mixin 'font' is correctly defined in the imported mixins file. Incorrect or missing mixin definitions can cause compilation errors.
| } | ||
|
|
||
| &__button { | ||
| @include font(14px, 700, 16px, $white-color); |
There was a problem hiding this comment.
Verify that the mixin 'font' is correctly defined in the imported mixins file. Incorrect or missing mixin definitions can cause compilation errors.
| } | ||
|
|
||
| .stars { | ||
| @include flex($direction: row, $items: center); |
There was a problem hiding this comment.
Verify that the mixin 'flex' is correctly defined in the imported mixins file. Incorrect or missing mixin definitions can cause compilation errors.
| @include flex($direction: row, $items: center); | ||
|
|
||
| &__star { | ||
| @include set-size(16px, 16px); |
There was a problem hiding this comment.
Verify that the mixin 'set-size' is correctly defined in the imported mixins file. Incorrect or missing mixin definitions can cause compilation errors.
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 ❗️