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 task solution #4819

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add task solution #4819

wants to merge 1 commit into from

Conversation

d1nexl
Copy link

@d1nexl d1nexl commented Oct 30, 2024

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 effort on your project! 🎉 While your code is approved to help you move forward with your learning, there are a few areas to improve:

  1. SCSS Compilation: Ensure your SCSS is compiled to CSS, as browsers cannot interpret SCSS files directly. Update your HTML to link to the compiled CSS file.

  2. Hover Effects: The current hover effect on .card using transform: scale(1.2) can cause layout shifts. Consider alternative effects like changing background color or adding a shadow to enhance user experience.

  3. Code Redundancy: The repetitive product card content can be optimized. Consider using a templating engine or JavaScript to dynamically generate cards, which will improve maintainability.

  4. BEM and Data Attributes: Ensure all HTML classes follow BEM conventions and that data attributes match the task requirements, such as data-qa="card-hover" for the first card's "Buy" link.

  5. Responsive Design: Verify that your grid layout and media queries achieve the desired responsive behavior across different screen resolutions.

Keep up the good work, and use this feedback to refine your skills! 🚀

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

@@ -17,11 +17,379 @@
/>
<link
rel="stylesheet"
href="styles/index.scss"
href="/src/styles/index.scss"

Choose a reason for hiding this comment

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

The href attribute is pointing to a SCSS file. Browsers do not understand SCSS directly; it needs to be compiled to CSS first. Ensure that the SCSS is compiled to a CSS file and update the href to point to the resulting CSS file.

Comment on lines +147 to +391
/>
<div class="card__description card__description--margin--top">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</div>
<div class="card__code card__code--margin--top">
Product code: 195434
</div>
<div class="card__rating card__rating--margin--top">
<ul class="stars">
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
</ul>
<div class="card__reviews">Reviews: 5</div>
</div>
<div class="card__priceBlock card__priceBlock--margin--top">
<div class="card__txt">Price:</div>
<div class="card__price">$2,199</div>
</div>
<div data-qa="hover">
<a
class="card__button card__button--margin--top"
href="/"
>
BUY
</a>
</div>
</div>
<div class="card">
<img
class="card__image"
src="/src/images/imac.jpeg"
alt="Card-Image"
/>
<div class="card__description card__description--margin--top">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</div>
<div class="card__code card__code--margin--top">
Product code: 195434
</div>
<div class="card__rating card__rating--margin--top">
<ul class="stars">
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
</ul>
<div class="card__reviews">Reviews: 5</div>
</div>
<div class="card__priceBlock card__priceBlock--margin--top">
<div class="card__txt">Price:</div>
<div class="card__price">$2,199</div>
</div>
<div data-qa="hover">
<a
class="card__button card__button--margin--top"
href="/"
>
BUY
</a>
</div>
</div>
<div class="card">
<img
class="card__image"
src="/src/images/imac.jpeg"
alt="Card-Image"
/>
<div class="card__description card__description--margin--top">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</div>
<div class="card__code card__code--margin--top">
Product code: 195434
</div>
<div class="card__rating card__rating--margin--top">
<ul class="stars">
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
<li class="stars__area stars__area--size"></li>
</ul>
<div class="card__reviews">Reviews: 5</div>
</div>
<div class="card__priceBlock card__priceBlock--margin--top">
<div class="card__txt">Price:</div>
<div class="card__price">$2,199</div>
</div>
<div data-qa="hover">
<a
class="card__button card__button--margin--top"
href="/"
>
BUY
</a>
</div>
</div>

Choose a reason for hiding this comment

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

The product cards are repeated with identical content. Consider using a templating engine or JavaScript to dynamically generate these cards to avoid redundancy and improve maintainability.

Comment on lines +236 to +238
.card:hover {
transform: scale(1.2);
transition: 0.3s;

Choose a reason for hiding this comment

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

The hover effect on .card uses transform: scale(1.2), which can cause layout shifts and affect the user experience. Consider using a different visual effect that doesn't alter the element's size, such as changing the background color or adding a shadow.

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.

2 participants