Skip to content
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

add tast solution #5192

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

serhii-lesik
Copy link

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a 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! 😊 However, there are a few critical issues that need to be addressed to meet the task requirements:

  1. Class Name Typo: There is a consistent typo in the class name 'card__buttom'. It should be corrected to 'card__button' across all files to ensure styles are applied correctly and consistently.

  2. Opacity Value: The opacity property for .card__image is incorrectly set to '0px'. It should be a value between 0 and 1. If you want the image to be fully transparent, use opacity: 0;.

  3. SCSS Variables and Structure: The task requires the use of SCSS variables and a separate file for them, as well as the implementation of two BEM blocks (card and stars) in their own files. These are currently missing.

  4. SCSS Nesting: The SCSS file should demonstrate the use of nesting for elements, modifiers, and pseudo-classes, which is not currently shown.

  5. Background Image URL: Ensure that the background-image: url() is relative to index.scss and starts with ../images, instead of using absolute paths.

  6. Link Styles on Hover: The task specifies changing link styles on :hover, which is not currently implemented.

Please address these issues to ensure the functionality and requirements of the task are fully met. Keep up the great work, and feel free 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/index.html Outdated
<div class="card__priceText">Price:</div>
<div class="card__priceValue">$2,199</div>
</div>
<div class="card__buttom" data-qa="hover">Buy</div>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the class name 'card__buttom'. It should be 'card__button' to maintain consistency and avoid potential styling issues.

background-size: cover;
width: 160px;
height: 134px;
opacity: 0px;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'opacity' property should be a value between 0 and 1, not '0px'. If you want the image to be fully transparent, use 'opacity: 0;'.

color: #060B35;
}

.card__buttom {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the class name 'card__buttom'. It should be 'card__button' to match the HTML and ensure styles are applied correctly.

transition: transform 0.3s ease;
}

.card__buttom:hover {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hover selector '.card__buttom:hover' contains a typo. It should be '.card__button:hover' to match the corrected class name.

@@ -0,0 +1,2 @@
body{margin:0;font-family:Roboto,sans-serif}.card{box-sizing:border-box;width:200px;height:408px;display:flex;flex-direction:column;align-items:center}.card__image{margin-top:32px;box-sizing:border-box;background-image:url(/src/images/imac.jpeg);background-size:cover;width:160px;height:134px;opacity:0px}.card__info{box-sizing:border-box;display:flex;flex-direction:column;margin-left:17px;margin-right:17px}.card__name{margin-top:40px;font-size:12px;font-weight:500;line-height:18px;text-align:left;color:#000}.card__code{font-size:10px;font-weight:400;line-height:14px;text-align:left;color:#616070;margin-top:4px}.card__rating{margin-top:16px;display:flex;justify-content:space-between}.card__stars{display:flex;flex-direction:row;justify-content:space-between}.card__star{box-sizing:border-box;width:16px;height:16px;background-image:url(/src/images/star.svg);background-repeat:no-repeat;background-size:cover;display:flex;flex-direction:row;margin-right:4px}.card__reviews{font-size:10px;font-weight:400;line-height:14px;text-align:right}.card__price{margin-top:24px;display:flex;flex-direction:row;justify-content:space-between}.card__priceText{font-size:12px;font-weight:400;line-height:18px;color:#616070}.card__priceValue{font-size:16px;font-weight:700;line-height:18px;color:#060b35}.card__buttom{box-sizing:border-box;margin-top:16px;background-color:#00acdc;font-size:14px;line-height:16px;width:166px;height:40px;border-radius:5px;display:flex;justify-content:center;align-items:center;color:#fff;text-transform:uppercase}.card .card__star:nth-child(-n+4){background-image:url(/src/images/star-active.svg);transition:transform .3s ease}.card__buttom:hover{box-sizing:border-box;background-color:#fff;color:#00acdc;border-radius:5px;border:1px solid #00acdc}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'opacity' property for '.card__image' should be a value between 0 and 1, not '0px'. Use 'opacity: 0;' if you want the image to be fully transparent.

@@ -0,0 +1,2 @@
body{margin:0;font-family:Roboto,sans-serif}.card{box-sizing:border-box;width:200px;height:408px;display:flex;flex-direction:column;align-items:center}.card__image{margin-top:32px;box-sizing:border-box;background-image:url(/src/images/imac.jpeg);background-size:cover;width:160px;height:134px;opacity:0px}.card__info{box-sizing:border-box;display:flex;flex-direction:column;margin-left:17px;margin-right:17px}.card__name{margin-top:40px;font-size:12px;font-weight:500;line-height:18px;text-align:left;color:#000}.card__code{font-size:10px;font-weight:400;line-height:14px;text-align:left;color:#616070;margin-top:4px}.card__rating{margin-top:16px;display:flex;justify-content:space-between}.card__stars{display:flex;flex-direction:row;justify-content:space-between}.card__star{box-sizing:border-box;width:16px;height:16px;background-image:url(/src/images/star.svg);background-repeat:no-repeat;background-size:cover;display:flex;flex-direction:row;margin-right:4px}.card__reviews{font-size:10px;font-weight:400;line-height:14px;text-align:right}.card__price{margin-top:24px;display:flex;flex-direction:row;justify-content:space-between}.card__priceText{font-size:12px;font-weight:400;line-height:18px;color:#616070}.card__priceValue{font-size:16px;font-weight:700;line-height:18px;color:#060b35}.card__buttom{box-sizing:border-box;margin-top:16px;background-color:#00acdc;font-size:14px;line-height:16px;width:166px;height:40px;border-radius:5px;display:flex;justify-content:center;align-items:center;color:#fff;text-transform:uppercase}.card .card__star:nth-child(-n+4){background-image:url(/src/images/star-active.svg);transition:transform .3s ease}.card__buttom:hover{box-sizing:border-box;background-color:#fff;color:#00acdc;border-radius:5px;border:1px solid #00acdc}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the class name 'card__buttom'. It should be 'card__button' to match the HTML and ensure styles are applied correctly.

@@ -0,0 +1,2 @@
body{margin:0;font-family:Roboto,sans-serif}.card{box-sizing:border-box;width:200px;height:408px;display:flex;flex-direction:column;align-items:center}.card__image{margin-top:32px;box-sizing:border-box;background-image:url(/src/images/imac.jpeg);background-size:cover;width:160px;height:134px;opacity:0px}.card__info{box-sizing:border-box;display:flex;flex-direction:column;margin-left:17px;margin-right:17px}.card__name{margin-top:40px;font-size:12px;font-weight:500;line-height:18px;text-align:left;color:#000}.card__code{font-size:10px;font-weight:400;line-height:14px;text-align:left;color:#616070;margin-top:4px}.card__rating{margin-top:16px;display:flex;justify-content:space-between}.card__stars{display:flex;flex-direction:row;justify-content:space-between}.card__star{box-sizing:border-box;width:16px;height:16px;background-image:url(/src/images/star.svg);background-repeat:no-repeat;background-size:cover;display:flex;flex-direction:row;margin-right:4px}.card__reviews{font-size:10px;font-weight:400;line-height:14px;text-align:right}.card__price{margin-top:24px;display:flex;flex-direction:row;justify-content:space-between}.card__priceText{font-size:12px;font-weight:400;line-height:18px;color:#616070}.card__priceValue{font-size:16px;font-weight:700;line-height:18px;color:#060b35}.card__buttom{box-sizing:border-box;margin-top:16px;background-color:#00acdc;font-size:14px;line-height:16px;width:166px;height:40px;border-radius:5px;display:flex;justify-content:center;align-items:center;color:#fff;text-transform:uppercase}.card .card__star:nth-child(-n+4){background-image:url(/src/images/star-active.svg);transition:transform .3s ease}.card__buttom:hover{box-sizing:border-box;background-color:#fff;color:#00acdc;border-radius:5px;border:1px solid #00acdc}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hover selector '.card__buttom:hover' contains a typo. It should be '.card__button:hover' to match the corrected class name.

background-size: cover;
width: 160px;
height: 134px;
opacity: 0px;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'opacity' property should be a value between 0 and 1, not '0px'. If you want the image to be fully transparent, use 'opacity: 0;'.

color: #060B35;
}

.card__buttom {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the class name 'card__buttom'. It should be 'card__button' to match the HTML and ensure styles are applied correctly.

transition: transform 0.3s ease;
}

.card__buttom:hover {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hover selector '.card__buttom:hover' contains a typo. It should be '.card__button:hover' to match the corrected class name.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on addressing some of the issues from the previous review! 🎉 While there are still a few critical points to address, such as implementing SCSS variables and nesting, and correcting the background-image URLs to be relative, your progress is commendable. Remember to remove unnecessary files and links to streamline your project. Keep up the good work, and use the feedback to enhance your skills further. Your dedication to improving your code is evident, and I'm confident you'll tackle these issues effectively. Happy coding! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

src/index.html Outdated
@@ -7,12 +8,41 @@
content="width=device-width, initial-scale=1.0"
/>
<title>Product cards</title>
<link rel="preconnect" href="https://fonts.googleapis.com">
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,100..900;1,100..900&display=swap" rel="stylesheet">
<link
rel="stylesheet"
href="./styles/index.scss"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linking to the SCSS file directly in HTML is unnecessary if you are already linking to the compiled CSS file. The Parcel bundler should handle the SCSS compilation, so you can remove this line to avoid redundancy.

.card__image {
margin-top: 32px;
box-sizing: border-box;
background-image: url(/src/images/imac.jpeg);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The background-image URL should be relative to the CSS file location. Consider changing it to ../images/imac.jpeg to ensure it works correctly when deployed.

box-sizing: border-box;
width: 16px;
height: 16px;
background-image: url(/src/images/star.svg);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The background-image URL for the star should also be relative. Change it to ../images/star.svg to ensure proper loading.

}

.card .card__star:nth-child(-n+4) {
background-image: url(/src/images/star-active.svg);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, update the background-image URL for the active star to ../images/star-active.svg to maintain consistency and ensure the image loads correctly.

@@ -0,0 +1,2 @@
body{margin:0;font-family:Roboto,sans-serif}.card{box-sizing:border-box;width:200px;height:408px;display:flex;flex-direction:column;align-items:center}.card__image{margin-top:32px;box-sizing:border-box;background-image:url(/src/images/imac.jpeg);background-size:cover;width:160px;height:134px}.card__info{box-sizing:border-box;display:flex;flex-direction:column;margin-left:17px;margin-right:17px}.card__name{margin-top:40px;font-size:12px;font-weight:500;line-height:18px;text-align:left;color:#000}.card__code{font-size:10px;font-weight:400;line-height:14px;text-align:left;color:#616070;margin-top:4px}.card__rating{margin-top:16px;display:flex;justify-content:space-between}.card__stars{display:flex;flex-direction:row;justify-content:space-between}.card__star{box-sizing:border-box;width:16px;height:16px;background-image:url(/src/images/star.svg);background-repeat:no-repeat;background-size:cover;display:flex;flex-direction:row;margin-right:4px}.card__reviews{font-size:10px;font-weight:400;line-height:14px;text-align:right}.card__price{margin-top:24px;display:flex;flex-direction:row;justify-content:space-between}.card__priceText{font-size:12px;font-weight:400;line-height:18px;color:#616070}.card__priceValue{font-size:16px;font-weight:700;line-height:18px;color:#060b35}.card__button{box-sizing:border-box;margin-top:16px;background-color:#00acdc;font-size:14px;line-height:16px;width:166px;height:40px;border-radius:5px;display:flex;justify-content:center;align-items:center;color:#fff;text-transform:uppercase}.card .card__star:nth-child(-n+4){background-image:url(/src/images/star-active.svg);transition:transform .3s ease}.card__button:hover{box-sizing:border-box;background-color:#fff;color:#00acdc;border-radius:5px;border:1px solid #00acdc}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The background-image URLs should be relative. Update /src/images/imac.jpeg, /src/images/star.svg, and /src/images/star-active.svg to ../images/imac.jpeg, ../images/star.svg, and ../images/star-active.svg respectively to ensure they load correctly.

.card__image {
margin-top: 32px;
box-sizing: border-box;
background-image: url(/src/images/imac.jpeg);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The background-image URL should be relative to the SCSS file location. Change it to ../images/imac.jpeg to ensure it works correctly when deployed.

box-sizing: border-box;
width: 16px;
height: 16px;
background-image: url(/src/images/star.svg);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The background-image URL for the star should also be relative. Change it to ../images/star.svg to ensure proper loading.

}

.card .card__star:nth-child(-n+4) {
background-image: url(/src/images/star-active.svg);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, update the background-image URL for the active star to ../images/star-active.svg to maintain consistency and ensure the image loads correctly.

Comment on lines 6 to 116
box-sizing: border-box;
background-image: url(/src/images/imac.jpeg);
background-size: cover;
width: 160px;
height: 134px;
}

.card__info {
box-sizing: border-box;
display: flex;
flex-direction: column;
margin-left: 17px;
margin-right: 17px;
}

.card__name {
margin-top: 40px;
font-size: 12px;
font-weight: 500;
line-height: 18px;
text-align: left;
color: #000000;
}

.card__code {
font-size: 10px;
font-weight: 400;
line-height: 14px;
text-align: left;
color: #616070;
margin-top: 4px;
}

.card__rating {
margin-top: 16px;
display: flex;
justify-content: space-between;
}

.card__stars {
display: flex;
flex-direction: row;
justify-content: space-between;
}

.card__star {
box-sizing: border-box;
width: 16px;
height: 16px;
background-image: url(/src/images/star.svg);
background-repeat: no-repeat;
background-size: cover;
display: flex;
flex-direction: row;
margin-right: 4px;
}

.card__reviews {
font-size: 10px;
font-weight: 400;
line-height: 14px;
text-align: right;
}

.card__price {
margin-top: 24px;
display: flex;
flex-direction: row;
justify-content: space-between;
}

.card__priceText {
font-size: 12px;
font-weight: 400;
line-height: 18px;
color: #616070;
}

.card__priceValue {
font-size: 16px;
font-weight: 700;
line-height: 18px;
color: #060B35;
}

.card__button {
box-sizing: border-box;
margin-top: 16px;
background-color: #00ACDC;
font-size: 14px;
line-height: 16px;
width: 166px;
height: 40px;
border-radius: 5px;
display: flex;
justify-content: center;
align-items: center;
color: #FFFFFF;
text-transform: uppercase;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using SCSS variables for repeated values like colors, font sizes, and dimensions. This will make your code more maintainable and easier to update.

Comment on lines 6 to 116
box-sizing: border-box;
background-image: url(/src/images/imac.jpeg);
background-size: cover;
width: 160px;
height: 134px;
}

.card__info {
box-sizing: border-box;
display: flex;
flex-direction: column;
margin-left: 17px;
margin-right: 17px;
}

.card__name {
margin-top: 40px;
font-size: 12px;
font-weight: 500;
line-height: 18px;
text-align: left;
color: #000000;
}

.card__code {
font-size: 10px;
font-weight: 400;
line-height: 14px;
text-align: left;
color: #616070;
margin-top: 4px;
}

.card__rating {
margin-top: 16px;
display: flex;
justify-content: space-between;
}

.card__stars {
display: flex;
flex-direction: row;
justify-content: space-between;
}

.card__star {
box-sizing: border-box;
width: 16px;
height: 16px;
background-image: url(/src/images/star.svg);
background-repeat: no-repeat;
background-size: cover;
display: flex;
flex-direction: row;
margin-right: 4px;
}

.card__reviews {
font-size: 10px;
font-weight: 400;
line-height: 14px;
text-align: right;
}

.card__price {
margin-top: 24px;
display: flex;
flex-direction: row;
justify-content: space-between;
}

.card__priceText {
font-size: 12px;
font-weight: 400;
line-height: 18px;
color: #616070;
}

.card__priceValue {
font-size: 16px;
font-weight: 700;
line-height: 18px;
color: #060B35;
}

.card__button {
box-sizing: border-box;
margin-top: 16px;
background-color: #00ACDC;
font-size: 14px;
line-height: 16px;
width: 166px;
height: 40px;
border-radius: 5px;
display: flex;
justify-content: center;
align-items: center;
color: #FFFFFF;
text-transform: uppercase;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implement SCSS nesting to better organize your styles. For example, nest the styles for .card__image, .card__info, etc., inside the .card block.

Copy link

@vadiimvooo vadiimvooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you forgot to deploy your solution. Please feel free to ask for help in fe_chat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants