-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
create file card product #5019
create file card product #5019
Conversation
olga-wukelan
commented
Nov 28, 2024
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
-
Image Paths: Ensure that the paths to the
star-active.svg
andstar.svg
images are correct. Incorrect paths will lead to missing images in the UI. -
SCSS Imports: Verify that the paths in the
@import
statements are correct and that the referenced SCSS files (card
andstars
) exist. Incorrect paths will cause compilation errors. -
Undefined Variables: Several variables such as
$main-width
,$font-family
,$secondary-color
, and$primary-color
are used but not defined in the SCSS files. Make sure these variables are defined in an imported file or within the file itself to avoid compilation errors.
These issues are critical as they affect the functionality and appearance of the application. Once these are resolved, we can proceed with the approval. Keep up the great work, and don't hesitate to reach out if you need any help! 🚀
✨ 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.
src/styles/card.scss
Outdated
} | ||
|
||
&__description { | ||
font-family: $font-family; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $font-family
is used here, but it is not defined within this file. Ensure that it is defined in a file that is imported into this SCSS file, or define it here if it is missing.
font-size: 12px; | ||
font-weight: 500; | ||
line-height: 18px; | ||
color: $primary-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $primary-color
is used here, but it is not defined within this file. Ensure that it is defined in a file that is imported into this SCSS file, or define it here if it is missing.
font-size: 10px; | ||
font-weight: 400; | ||
line-height: 14px; | ||
color: $secondary-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $secondary-color
is used here, but it is not defined within this file. Ensure that it is defined in a file that is imported into this SCSS file, or define it here if it is missing.
src/styles/card.scss
Outdated
} | ||
|
||
&__reviews { | ||
font-family: $font-family; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $font-family
is used here again, but it is not defined within this file. Ensure that it is defined in a file that is imported into this SCSS file, or define it here if it is missing.
src/styles/card.scss
Outdated
} | ||
|
||
&__price { | ||
font-family: $font-family; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $font-family
is used here again, but it is not defined within this file. Ensure that it is defined in a file that is imported into this SCSS file, or define it here if it is missing.
src/styles/card.scss
Outdated
font-size: 14px; | ||
font-weight: 700; | ||
line-height: 16px; | ||
font-family: $font-family; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $font-family
is used here again, but it is not defined within this file. Ensure that it is defined in a file that is imported into this SCSS file, or define it here if it is missing.
&__button { | ||
display: block; | ||
box-sizing: border-box; | ||
width: $main-width; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $main-width
is used here, but it is not defined within this file. Ensure that it is defined in a file that is imported into this SCSS file, or define it here if it is missing.
src/styles/index.scss
Outdated
@import './style/card'; | ||
@import './style/stars'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the paths in the @import
statements are correct and that the referenced SCSS files (card
and stars
) exist in the specified directory. If the paths are incorrect, it will lead to a compilation error.
src/styles/stars.scss
Outdated
margin-right: 4px; | ||
background-repeat: no-repeat; | ||
background-position: center; | ||
background-image: url(/src/images/star.svg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the path to the star.svg
image is correct. If the image is not located at /src/images/star.svg
, update the path accordingly.
src/styles/stars.scss
Outdated
} | ||
|
||
&__star:nth-child(-n + 4) { | ||
background-image: url(/src/images/star-active.svg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the path to the star-active.svg
image is correct. If the image is not located at /src/images/star-active.svg
, update the path accordingly.