add task solution #4992
add task solution #4992
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 While your code is well-structured and meets most of the requirements, there are a few critical issues that need to be addressed before we can approve the pull request:
-
Inconsistent
data-qaAttributes: There is inconsistency in the use ofdata-qaattributes across similar elements. This can lead to confusion or errors in testing or automation scripts. Please standardize these attributes to ensure consistency. -
SCSS Variable Definitions: Ensure all SCSS variables, such as
$transition__duration,$color-accent,$color-button-text,$color-primary,$color-secondary, and$color-hover, are defined in your SCSS files. Undefined variables can lead to compilation errors and unexpected behavior.
Once these issues are resolved, your code will be in great shape! Keep up the great work, and don't hesitate to reach out if you need any 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.
| </div> | ||
|
|
||
| <a | ||
| data-qa="card-hover" |
There was a problem hiding this comment.
Inconsistent use of data-qa attribute. Here, it is card-hover, while in other similar elements, it is just hover. Consider standardizing this attribute across similar elements for consistency.
src/index.html
Outdated
| </div> | ||
|
|
||
| <a | ||
| data-qa="hover" |
There was a problem hiding this comment.
Inconsistent use of data-qa attribute. Here, it is hover, while in other similar elements, it is card-hover. Consider standardizing this attribute across similar elements for consistency.
src/index.html
Outdated
| </div> | ||
|
|
||
| <a | ||
| data-qa="hover" |
There was a problem hiding this comment.
Inconsistent use of data-qa attribute. Here, it is hover, while in other similar elements, it is card-hover. Consider standardizing this attribute across similar elements for consistency.
src/index.html
Outdated
| </div> | ||
|
|
||
| <a | ||
| data-qa="hover" |
There was a problem hiding this comment.
Inconsistent use of data-qa attribute. Here, it is hover, while in other similar elements, it is card-hover. Consider standardizing this attribute across similar elements for consistency.
src/index.html
Outdated
| </div> | ||
|
|
||
| <a | ||
| data-qa="hover" |
There was a problem hiding this comment.
Inconsistent use of data-qa attribute. Here, it is hover, while in other similar elements, it is card-hover. Consider standardizing this attribute across similar elements for consistency.
| margin: 0; | ||
| color: $color-button-text; | ||
| background-color: $color-accent; | ||
| border-radius: $border-radius; |
There was a problem hiding this comment.
The variable $border-radius is used for the border-radius. Ensure that this variable is defined in your SCSS files.
| text-decoration: none; | ||
|
|
||
| &:hover { | ||
| background-color: transparent; |
There was a problem hiding this comment.
The variable $color-accent is used for the hover color. Ensure that this variable is defined in your SCSS files.
|
|
||
| &:hover { | ||
| background-color: transparent; | ||
| color: $color-accent; |
There was a problem hiding this comment.
The variable $color-accent is used for the hover color. Ensure that this variable is defined in your SCSS files.
| &:hover { | ||
| background-color: transparent; | ||
| color: $color-accent; | ||
| border: 1px solid $color-accent; |
There was a problem hiding this comment.
The variable $color-accent is used for the border color on hover. Ensure that this variable is defined in your SCSS files.
| background-color: transparent; | ||
| color: $color-accent; | ||
| border: 1px solid $color-accent; | ||
| transition-duration: $transition__duration; |
There was a problem hiding this comment.
The variable $transition__duration is used here. Ensure that this variable is defined in your SCSS files to avoid compilation errors.
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.
Great job on addressing the issues from the previous review! 🎉 Your code now meets the task requirements and constraints. The data-qa attributes have been standardized, and all necessary SCSS variables are defined in the variables.scss file, ensuring consistency and preventing compilation errors.
While the decision is to approve, please remember to address the following points in future projects to enhance your code quality:
- Ensure all SCSS variables are defined to avoid any potential errors.
- Maintain consistency in the use of
data-qaattributes across similar elements to facilitate testing and automation.
Keep up the great work, and continue learning from these experiences to improve your coding skills! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| data-qa="nav-hover" |
There was a problem hiding this comment.
Inconsistent use of data-qa attributes. Consider adding data-qa attributes to other navigation links for consistency.
| </div> | ||
|
|
||
| <a | ||
| data-qa="card-hover" |
There was a problem hiding this comment.
Inconsistent use of data-qa attributes. Consider adding data-qa attributes to other 'BUY' links for consistency.
| @@ -0,0 +1,104 @@ | |||
| .card { | |||
| max-width: 200px; | |||
| background-color: $color-button-text; | |||
There was a problem hiding this comment.
The variable $color-button-text is used but not defined. Ensure this variable is defined in your SCSS files.
| max-width: 200px; | ||
| background-color: $color-button-text; | ||
| border: 1px solid #f3f3f3; | ||
| border-radius: $border-radius; |
There was a problem hiding this comment.
The variable $border-radius is used but not defined. Ensure this variable is defined in your SCSS files.
| background-color: $color-button-text; | ||
| border: 1px solid #f3f3f3; | ||
| border-radius: $border-radius; | ||
| transition: transform $transition__duration; |
There was a problem hiding this comment.
The variable $transition__duration is used but not defined. Ensure this variable is defined in your SCSS files.
| background-color: transparent; | ||
| color: $color-accent; | ||
| border: 1px solid $color-accent; | ||
| transition-duration: $transition__duration; |
There was a problem hiding this comment.
The variable $transition__duration is used but not defined. Ensure this variable is defined in your SCSS files.
| align-items: center; | ||
| text-transform: uppercase; | ||
| text-decoration: none; | ||
| color: $color-primary; |
There was a problem hiding this comment.
The variable $color-primary is used but not defined. Ensure this variable is defined in your SCSS files.
| } | ||
|
|
||
| &__link.is-active { | ||
| color: $color-accent; |
There was a problem hiding this comment.
The variable $color-accent is used but not defined. Ensure this variable is defined in your SCSS files.
| } | ||
|
|
||
| &__link:hover { | ||
| color: $color-accent; |
There was a problem hiding this comment.
The variable $color-accent is used but not defined. Ensure this variable is defined in your SCSS files.
| &__link.is-active::after { | ||
| content: ''; | ||
| position: absolute; | ||
| background-color: $color-accent; |
There was a problem hiding this comment.
The variable $color-accent is used but not defined. Ensure this variable is defined in your SCSS files.
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 ❗️