-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
catalog solution #4943
base: master
Are you sure you want to change the base?
catalog solution #4943
Conversation
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! 😊 While your code meets most of the requirements, there are a couple of critical issues that need to be addressed before we can approve the pull request:
-
Hover Effect on
.product-card
: The use oftransform: scale(1.2)
for the hover effect might cause layout shifts, which can disrupt the user experience. Consider using a smaller scale value or an alternative effect to minimize these disruptions. -
Repetitive Product Details: It seems that the product details are repeated across multiple product cards. If this was not intentional, please update each card with unique details to better showcase a diverse product catalog.
Once these issues are resolved, we can move forward 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.
</a> | ||
</article> | ||
<article class="product-card"> | ||
<img | ||
src="./images/imac.jpeg" | ||
class="product-card__image" | ||
alt="APPLE A1419 iMac 27" | ||
/> | ||
|
||
<h3 class="product-card__name"> | ||
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</h3> | ||
<p class="product-card__code">Product code: 195434</p> | ||
<div class="product-card__info"> | ||
<div class="stars stars--4"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
<p class="product-card__review">Reviews: 5</p> | ||
</div> | ||
<div class="product-card__price"> | ||
<p class="product-card__price-title">Price:</p> | ||
<p class="product-card__price-value">$2,199</p> | ||
</div> | ||
<a | ||
href="#buy" | ||
class="product-card__buy" | ||
> | ||
Buy | ||
</a> | ||
</article> | ||
|
||
<article class="product-card"> | ||
<img | ||
src="./images/imac.jpeg" | ||
class="product-card__image" | ||
alt="APPLE A1419 iMac 27" | ||
/> | ||
|
||
<h3 class="product-card__name"> | ||
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</h3> | ||
<p class="product-card__code">Product code: 195434</p> | ||
<div class="product-card__info"> | ||
<div class="stars stars--4"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
<p class="product-card__review">Reviews: 5</p> | ||
</div> | ||
<div class="product-card__price"> | ||
<p class="product-card__price-title">Price:</p> | ||
<p class="product-card__price-value">$2,199</p> | ||
</div> | ||
<a | ||
href="#buy" | ||
class="product-card__buy" | ||
> | ||
Buy | ||
</a> | ||
</article> | ||
|
||
<article class="product-card"> | ||
<img | ||
src="./images/imac.jpeg" | ||
class="product-card__image" | ||
alt="APPLE A1419 iMac 27" | ||
/> | ||
|
||
<h3 class="product-card__name"> | ||
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</h3> | ||
<p class="product-card__code">Product code: 195434</p> | ||
<div class="product-card__info"> | ||
<div class="stars stars--4"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
<p class="product-card__review">Reviews: 5</p> | ||
</div> | ||
<div class="product-card__price"> | ||
<p class="product-card__price-title">Price:</p> | ||
<p class="product-card__price-value">$2,199</p> | ||
</div> | ||
<a | ||
href="#buy" | ||
class="product-card__buy" | ||
> | ||
Buy | ||
</a> | ||
</article> |
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 product details, including the image, name, product code, reviews, and price, are repeated across multiple product cards. If this is not intentional, consider updating each product card with unique details to better represent a diverse catalog.
src/styles/blocks/card.scss
Outdated
&:hover { | ||
transform: scale(1.2); |
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 hover effect on the .product-card
class uses transform: scale(1.2)
, which might cause layout shifts and affect the surrounding elements. Consider using a smaller scale value or alternative effects to minimize layout disruptions.
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.
Review Summary
Great job on your submission! 🎉 Your code is well-structured, and you've made significant progress. However, there are a couple of issues that need attention:
-
Hover Effect on
.product-card
: The current hover effect usestransform: scale(1.1)
, which might cause layout shifts. Consider using a smaller scale value or an alternative effect to minimize disruptions. Although the task requirement suggests ascale(1.2)
, ensure that the effect does not negatively impact the user experience. -
Repetitive Product Details: The product cards currently display the same product details. To better showcase a diverse product catalog, each card should have unique details.
Despite these issues, your overall implementation is solid, and all required files are present and correctly linked. The decision to approve this pull request is made to help you move forward with your learning. Please review the comments to improve your code. Addressing these issues will enhance the functionality and user experience of your project. Keep up the great work, and continue learning from your mistakes! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
</a> | ||
</article> | ||
<article class="product-card"> | ||
<img | ||
src="./images/imac.jpeg" | ||
class="product-card__image" | ||
alt="APPLE A1419 iMac 27" | ||
/> | ||
|
||
<h3 class="product-card__name"> | ||
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</h3> | ||
<p class="product-card__code">Product code: 195434</p> | ||
<div class="product-card__info"> | ||
<div class="stars stars--4"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
<p class="product-card__review">Reviews: 5</p> | ||
</div> | ||
<div class="product-card__price"> | ||
<p class="product-card__price-title">Price:</p> | ||
<p class="product-card__price-value">$2,199</p> | ||
</div> | ||
<a | ||
href="#buy" | ||
class="product-card__buy" | ||
> | ||
Buy | ||
</a> | ||
</article> | ||
|
||
<article class="product-card"> | ||
<img | ||
src="./images/imac.jpeg" | ||
class="product-card__image" | ||
alt="APPLE A1419 iMac 27" | ||
/> | ||
|
||
<h3 class="product-card__name"> | ||
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</h3> | ||
<p class="product-card__code">Product code: 195434</p> | ||
<div class="product-card__info"> | ||
<div class="stars stars--4"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
<p class="product-card__review">Reviews: 5</p> | ||
</div> | ||
<div class="product-card__price"> | ||
<p class="product-card__price-title">Price:</p> | ||
<p class="product-card__price-value">$2,199</p> | ||
</div> | ||
<a | ||
href="#buy" | ||
class="product-card__buy" | ||
> | ||
Buy | ||
</a> | ||
</article> | ||
|
||
<article class="product-card"> | ||
<img | ||
src="./images/imac.jpeg" | ||
class="product-card__image" | ||
alt="APPLE A1419 iMac 27" | ||
/> | ||
|
||
<h3 class="product-card__name"> | ||
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</h3> | ||
<p class="product-card__code">Product code: 195434</p> | ||
<div class="product-card__info"> | ||
<div class="stars stars--4"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
<p class="product-card__review">Reviews: 5</p> | ||
</div> | ||
<div class="product-card__price"> | ||
<p class="product-card__price-title">Price:</p> | ||
<p class="product-card__price-value">$2,199</p> | ||
</div> | ||
<a | ||
href="#buy" | ||
class="product-card__buy" | ||
> | ||
Buy | ||
</a> | ||
</article> |
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 product cards are repetitive. Each card displays the same product details, which was previously noted as an issue. Consider updating each card with unique product details to showcase a diverse product catalog.
&:hover { | ||
transform: scale(1.1); |
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 hover effect uses transform: scale(1.1)
, which might cause layout shifts. Consider using a smaller scale value or a different effect to avoid disrupting the layout.
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 :hover are smooth
Code follows all the Code Style Rules ❗️