first commit #4823
first commit #4823
Conversation
SerhiiUnhurian
commented
Nov 2, 2024
SerhiiUnhurian
commented
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Great job on your submission! 🎉 Your code meets the core requirements, and you've done well with BEM and SCSS usage. There are some minor suggestions for improvement, such as using variables for colors to enhance maintainability and ensuring accessibility with descriptive alt texts. Keep up the good work, and consider these suggestions for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| font-family: Roboto, sans-serif; | ||
| font-weight: 400; | ||
| font-style: normal; | ||
| border: 1px solid #f3f3f3; |
There was a problem hiding this comment.
Consider using a variable for the border color instead of a hardcoded value #f3f3f3. This will improve maintainability and consistency across your styles.
| } | ||
|
|
||
| &:hover .card__title { | ||
| color: #34568b; |
There was a problem hiding this comment.
Consider using a variable for the hover color #34568b to maintain consistency with other color definitions.
| } | ||
|
|
||
| &__logo { | ||
| background-image: url(../images/imac.jpeg); |
There was a problem hiding this comment.
Ensure that the image path is correct and the image exists at the specified location. This is more of a reminder to verify assets.
| margin-top: 16px; | ||
|
|
||
| &:hover { | ||
| background-color: #fff; |
There was a problem hiding this comment.
Consider using a variable for the hover background color #fff to maintain consistency with other color definitions.
|
|
||
| &:hover { | ||
| background-color: #fff; | ||
| color: #00acdc; |
There was a problem hiding this comment.
Consider using a variable for the hover text color #00acdc to maintain consistency with other color definitions.
| text-decoration: none; | ||
| text-transform: uppercase; | ||
| font-weight: 500; | ||
| color: #060b35; |
There was a problem hiding this comment.
Consider using a variable for the color #060b35 to maintain consistency with other color definitions and improve maintainability.
| .stars { | ||
| display: flex; | ||
|
|
||
| &--4 :nth-child(-n + 4) { |
There was a problem hiding this comment.
The selector &--4 :nth-child(-n + 4) might be too specific and could lead to unexpected styling if not used carefully. Ensure that this class is applied correctly in the HTML to avoid styling issues. Consider using a more descriptive class name if possible.
| class="logo" | ||
| > | ||
| <img | ||
| src="./images/logo.png" |
There was a problem hiding this comment.
Ensure that the image path is correct and the image exists at the specified location. This is more of a reminder to verify assets.
| > | ||
| <img | ||
| src="./images/logo.png" | ||
| alt="nav-logo" |
There was a problem hiding this comment.
Consider providing a more descriptive alt text for the logo image to improve accessibility for screen readers.
| </li> | ||
| <li class="nav__item"> | ||
| <a | ||
| href="#laptops&computers" |
There was a problem hiding this comment.
The href attribute value #laptops&computers contains an ampersand, which is not a valid character in a URL fragment identifier. Consider replacing it with a hyphen or another valid character.
