test solution #5040

KrawczykDamiann · 2024-12-04T15:09:42Z

mateacademy-ai-mentor

Thank you for your hard work on this project! 😊 While your code is mostly on the right track, there are a few critical issues that need to be addressed to fully meet the task requirements. Specifically, the BEM blocks need proper definition, and the SCSS nesting and variable usage should be improved for consistency and maintainability. Additionally, the stars--N modifiers need to be implemented correctly, and the unnecessary index.css file should be removed. Please make these changes, and your solution will be in great shape! Keep up the good work! 🚀

✨ 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.

mateacademy-ai-mentor · 2024-12-04T15:11:19Z

src/index.html

+    <link
+      rel="preconnect"
+      href="https://fonts.googleapis.com"
+    />
+    <link
+      rel="preconnect"
+      href="https://fonts.gstatic.com"
+      crossorigin="use-credentials"
+    />


There are duplicate preconnect links for 'https://fonts.googleapis.com' and 'https://fonts.gstatic.com'. It's redundant to have multiple preconnect links for the same URL, as it can lead to unnecessary network requests. Consider removing the duplicates to optimize performance.

natalia-klonowska · 2024-12-04T15:12:59Z

src/index.html

@@ -9,10 +31,47 @@
    <title>Product cards</title>
    <link
      rel="stylesheet"
-      href="./styles/index.scss"
+      href="./styles/index.css"


Suggested change

href="./styles/index.css"

href="./styles/index.scss"

natalia-klonowska · 2024-12-04T15:15:30Z

src/index.html

+      <div class="title">
+        <div class="title__text">
+          APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
+        </div>
+        <div class="title__product-code">Product code:195434</div>
+      </div>


title can be element of card block or create separate file for title styles (because you should have separate file for each block)

natalia-klonowska · 2024-12-04T15:16:37Z

src/styles/_button.scss

+@use './variables' as *;
+@use './mixins' as *;


Suggested change

@use './variables' as *;

@use './mixins' as *;

natalia-klonowska · 2024-12-04T15:16:49Z

src/styles/_card.scss

+@use './variables' as *;
+@use './mixins' as *;
+@use './stars' as *;


Suggested change

@use './variables' as *;

@use './mixins' as *;

@use './stars' as *;

natalia-klonowska · 2024-12-04T15:17:02Z

src/styles/_price.scss

+@use './variables' as *;
+@use './mixins' as *;


Suggested change

@use './variables' as *;

@use './mixins' as *;

natalia-klonowska · 2024-12-04T15:57:21Z

src/styles/_card.scss

+    width: 98px;
+    height: 14px;
+    top: 246px;


Suggested change

width: 98px;

height: 14px;

top: 246px;

natalia-klonowska · 2024-12-04T15:57:38Z

src/styles/_card.scss

+    width: $primary-width;
+    height: 36px;
+    top: 206px;


Suggested change

width: $primary-width;

height: 36px;

top: 206px;

natalia-klonowska · 2024-12-04T15:57:51Z

src/styles/_card.scss

+
+.title {
+  display: inline;
+  left: 16px;


Suggested change

left: 16px;

natalia-klonowska · 2024-12-04T15:58:05Z

src/styles/_card.scss

+  top: 32px;
+  left: 20px;


Suggested change

top: 32px;

left: 20px;

src/index.html

natalia-klonowska · 2024-12-12T14:04:08Z

src/index.html

+      </div>
+      <div class="card__stars">
+        <div class="card__stars-star"></div>
+        <div class="card__stars-star"></div>
+        <div class="card__stars-star"></div>
+        <div class="card__stars-star"></div>
+        <div class="card__stars-star"></div>
+        <div class="card__stars-reviews">Reviews: 5</div>
+      </div>


if you have separate file stars then you should have stars block and you need container for star elements so you could use space-between

Suggested change

</div>

<div class="card__stars">

<div class="card__stars-star"></div>

<div class="card__stars-star"></div>

<div class="card__stars-star"></div>

<div class="card__stars-star"></div>

<div class="card__stars-star"></div>

<div class="card__stars-reviews">Reviews: 5</div>

</div>

</div>

<div class="card__stars stars">

<div class="stars__list">

<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>

<div class="stars__reviews">Reviews: 5</div>

</div>

natalia-klonowska · 2024-12-12T14:04:38Z

src/index.html

+      <div class="card__price">
+        <div class="card__price-text">Price:</div>
+        <div class="card__price-sum">$2,199</div>
+      </div>


Suggested change

<div class="card__price">

<div class="card__price-text">Price:</div>

<div class="card__price-sum">$2,199</div>

</div>

<div class="card__price price">

<div class="price__text">Price:</div>

<div class="price__sum">$2,199</div>

</div>

natalia-klonowska · 2024-12-12T14:06:49Z

src/index.html

+      <a
+        class="card__button"
+        href="#"
+        data-qa="hover"
+      >


Suggested change

<a

class="card__button"

href="#"

data-qa="hover"

>

<a

class="card__button button"

href="#"

data-qa="hover"

>

natalia-klonowska · 2024-12-12T14:09:09Z

src/styles/_button.scss

@@ -0,0 +1,23 @@
+.card__button {


Suggested change

.card__button {

.button {

natalia-klonowska · 2024-12-12T14:09:53Z

src/styles/_button.scss

+  width: $primary-width;
+  margin-top: 15px;


Suggested change

width: $primary-width;

margin-top: 15px;

width: 100%;

margin-top: 16px;

src/styles/_stars.scss

natalia-klonowska · 2024-12-12T14:20:53Z

src/styles/_stars.scss

+    background-repeat: no-repeat;
+  }
+
+  &-star:nth-child(n + 5) {


Suggested change

&-star:nth-child(n + 5) {

&__star:nth-child(n + 5) {

src/styles/_stars.scss

natalia-klonowska · 2024-12-12T14:22:34Z

src/styles/_variables.scss

@@ -0,0 +1 @@
+$primary-width: 166px;


create variables for colors

Suggested change

$primary-width: 166px;

natalia-klonowska · 2024-12-12T14:23:14Z

src/styles/index.scss

 body {
+  @include center;


Suggested change

@include center;

natalia-klonowska

resolve all comments
index.html:
https://github.com/mate-academy/layout_product-cards/pull/5040/files#r1882190599
https://github.com/mate-academy/layout_product-cards/pull/5040/files#r1882191391
https://github.com/mate-academy/layout_product-cards/pull/5040/files#r1882194957

button.scss:
https://github.com/mate-academy/layout_product-cards/pull/5040/files#r1882201056

price.scss:
https://github.com/mate-academy/layout_product-cards/pull/5040/files#r1882214053

variables.scss:
https://github.com/mate-academy/layout_product-cards/pull/5040/files#r1882234428

index.scss:
https://github.com/mate-academy/layout_product-cards/pull/5040/files#r1882235682

natalia-klonowska · 2024-12-12T17:04:21Z

src/index.html

+      <div class="card__stars stars">
+        <div class="stars__list">
+          <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 class="stars__reviews">Reviews: 5</div>
+        </div>
+      </div>


Suggested change

<div class="card__stars stars">

<div class="stars__list">

<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 class="stars__reviews">Reviews: 5</div>

</div>

</div>

<div class="card__stars stars">

<div class="stars__list">

<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>

<div class="stars__reviews">Reviews: 5</div>

</div>

natalia-klonowska · 2024-12-12T17:05:03Z

src/styles/_stars.scss

+
+  line-height: 16px;
+  width: 100%;
+  margin-top: 17px;


Suggested change

margin-top: 17px;

margin-top: 16px;

display: flex;

justify-content: space-between;

align-items: flex-end;

&__list {

@include center;

}

natalia-klonowska · 2024-12-12T17:07:20Z

src/styles/_stars.scss

+  &__star {
+    width: 16px;
+    height: 16px;
+    margin-right: 4px;
+    justify-content: space-between;
+    background-image: url('../images/star-active.svg');
+    background-repeat: no-repeat;
+  }


add background-position: center;

Suggested change

&__star {

width: 16px;

height: 16px;

margin-right: 4px;

justify-content: space-between;

background-image: url('../images/star-active.svg');

background-repeat: no-repeat;

}

&__star {

width: 16px;

height: 16px;

margin-right: 4px;

justify-content: space-between;

background-image: url('../images/star-active.svg');

background-repeat: no-repeat;

background-position: center;

}

test solution

5bc9acb

mateacademy-ai-mentor suggested changes Dec 4, 2024

View reviewed changes

natalia-klonowska suggested changes Dec 4, 2024

View reviewed changes

KrawczykDamiann added 4 commits December 4, 2024 19:52

test solution 2

86632c9

test task solution

2eaa6b0

task solution2

7e7285a

task solution3

2fca7ca

natalia-klonowska suggested changes Dec 12, 2024

View reviewed changes

task solution4

8eb04d6

KrawczykDamiann requested a review from natalia-klonowska December 12, 2024 15:47

natalia-klonowska suggested changes Dec 12, 2024

View reviewed changes

task solution-1

0262f01

KrawczykDamiann requested a review from natalia-klonowska December 12, 2024 16:54

natalia-klonowska suggested changes Dec 12, 2024

View reviewed changes

final solution

fe37e20

KrawczykDamiann requested review from mateacademy-ai-mentor and natalia-klonowska December 13, 2024 08:13

danon321 approved these changes Dec 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test solution #5040

test solution #5040

KrawczykDamiann commented Dec 4, 2024 •

edited

Loading

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 4, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska left a comment

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

natalia-klonowska Dec 12, 2024

	@use './variables' as *;
	@use './mixins' as *;
	@use './stars' as *;

-  margin-top: 17px;
+  margin-top: 16px;
+  display: flex;
+  justify-content: space-between;
+  align-items: flex-end;
+  &__list {
+    @include center;
+  }

test solution #5040

Are you sure you want to change the base?

test solution #5040

Conversation

KrawczykDamiann commented Dec 4, 2024 • edited Loading

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natalia-klonowska left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KrawczykDamiann commented Dec 4, 2024 •

edited

Loading