Develop #2312

chiper9 · 2022-12-11T15:49:57Z

svitjojo

Good job, make changes and delete the folder with tests, run it again, maybe it will help. Good luck)

svitjojo · 2022-12-11T21:16:50Z

src/index.html

+          <div class="stars__star"></div>
+          <div class="stars__star"></div>
+        </div>
+        <div class="rate-rewiev">Reviews: 5</div>


Use semantic tag "p"

haven't fixed

svitjojo · 2022-12-11T21:17:19Z

src/index.html

+        <div class="word-price">Price:</div>
+        <div class="price-value">$2,199</div>


Also better to put p instead div

here as well

svitjojo · 2022-12-11T21:18:31Z

src/styles/card.css

+  padding: 32px 16px 16px;
+  display: flex;
+  flex-direction: column;
+  width: 199px;


width is supposed to be 200px

I meant 199 general width + 1 border width

But you are using border-box property, so it includes border too. You can check it with developer tools in google chrome

CHECKLIST point 7, and I agree it should be 200px

svitjojo · 2022-12-11T21:21:53Z

src/styles/card.css

+  height: 134px;
+  width: 160px;
+  margin: 0 auto 40px;
+  padding: 0 3px;


I think you can remove padding, it doesn't change anything

svitjojo · 2022-12-11T21:23:39Z

src/styles/card.css

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


remove redundant properties

svitjojo · 2022-12-11T21:26:37Z

src/styles/card.css

+  height: 40px;
+  width: 166px;
+  background: #00acdc;
+  border-radius: 5px;
+  text-align: center;
+  text-transform: uppercase;
+  line-height: 40px;
+  text-decoration: none;
+  color: #fff;
+  font-weight: 700;
+  font-size: 14px;
+  /* margin-bottom: 16px; */


Use paddings instead of height and so big line-height. Also, remove comments at the end.

svitjojo · 2022-12-11T21:27:35Z

src/styles/card.css

+.card__button:hover {
+  color: #00acdc;
+  background: #fff;
+  border: 1px solid #00acdc;


put a border to the default style card__button

Thank you for your help dude. Really appreciate it ;)

chiper9 · 2022-12-11T22:28:25Z

Yeah, your right. I forgot about it. Thanks пн, 12 дек. 2022 г., 00:17 Yevhen ***@***.***>:

…

***@***.**** commented on this pull request. ------------------------------ In src/styles/card.css <#2312 (comment)> : > @@ -0,0 +1,91 @@ +.card { + box-sizing: border-box; + padding: 32px 16px 16px; + display: flex; + flex-direction: column; + width: 199px; But you are using border-box property, so it includes border too. You can check it with developer tools in google chrome — Reply to this email directly, view it on GitHub <#2312 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A2EKJQSBRTHYLKB7EVUDLILWMZHIZANCNFSM6AAAAAAS3BWD6M> . You are receiving this because you authored the thread.Message ID: ***@***.***>

alexandra-protyanova · 2022-12-12T09:28:20Z

src/index.html

+  </head>
+  <body>
+    <div class="card" data-qa="card">
+      <img class="card__image"


if you have more than 3 attributes move them on new line, each

16 line - 2 attributes
17 line - everything ok )

I meant they should start from new line)

alexandra-protyanova · 2022-12-12T09:28:38Z

src/index.html

+          <div class="stars__star"></div>
+          <div class="stars__star"></div>
+        </div>
+        <div class="rate-rewiev">Reviews: 5</div>


haven't fixed

alexandra-protyanova · 2022-12-12T09:28:57Z

src/index.html

+        <div class="word-price">Price:</div>
+        <div class="price-value">$2,199</div>


here as well

alexandra-protyanova · 2022-12-12T09:29:14Z

src/index.html

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


Suggested change

<a class="card__button"

href="#"

data-qa="hover"

>

Buy

</a>

<a

class="card__button"

href="#"

data-qa="hover"

>

Buy

</a>

Do not forget about the rules of correct code style

Sorry, what do you mean?

Amount of indent are incorrect

alexandra-protyanova · 2022-12-12T09:30:26Z

src/styles/card.css

+  padding: 32px 16px 16px;
+  display: flex;
+  flex-direction: column;
+  width: 199px;


CHECKLIST point 7, and I agree it should be 200px

alexandra-protyanova · 2022-12-12T09:30:39Z

src/styles/card.css

@@ -0,0 +1,88 @@
+.card {
+  box-sizing: border-box;


move to * selector

haven't fixed

in my case if I do it everything stops working at all

U should move only box-sizing to *

alexandra-protyanova · 2022-12-12T09:31:03Z

src/styles/card.css

+  font-size: 12px;
+  line-height: 18px;
+  color: #060b35;
+  margin: 0 0 4px;


why isn't it margin-bottom?

alexandra-protyanova · 2022-12-12T09:32:10Z

src/styles/card.css

+
+.card__button {
+  height: 40px;
+  width: 166px;


Suggested change

width: 166px;

width: 100%;

alexandra-protyanova · 2022-12-12T09:33:03Z

src/styles/stars.css

+  background-image: url(../images/star-active.svg);
+}
+
+.stars__star:last-child {


CHECKLIST point 4

what about this comment?

alexandra-protyanova · 2022-12-12T09:33:46Z

src/index.html

+      <img class="card__image"
+        src="./images/imac.jpeg"
+        alt="Apple A1419 iMac">
+      <h2 class="card__name">


CHECKLIST point 3

Thanks a lot ;)

alexandra-protyanova

Fix all the comments, if you have any questions feel free to ask them in slack)

alexandra-protyanova · 2022-12-12T16:44:52Z

src/index.html

+  </head>
+  <body>
+    <div class="card" data-qa="card">
+      <img class="card__image"


I meant they should start from new line)

alexandra-protyanova · 2022-12-12T16:45:16Z

src/index.html

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


Amount of indent are incorrect

alexandra-protyanova · 2022-12-12T16:45:28Z

src/styles/card.css

@@ -0,0 +1,88 @@
+.card {
+  box-sizing: border-box;


haven't fixed

alexandra-protyanova

Not all the comments were fixed, got to files changed and check all the comments, if you have any questions fell free to ask them in slack 😊

alexandra-protyanova · 2022-12-13T09:03:31Z

src/index.html

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


Suggested change

<a class="card__button"

href="#"

data-qa="hover"

>

Buy

</a>

<a

class="card__button"

href="#"

data-qa="hover"

>

Buy

</a>

alexandra-protyanova · 2022-12-13T13:05:15Z

src/styles/card.css

@@ -0,0 +1,88 @@
+.card {
+  box-sizing: border-box;


U should move only box-sizing to *

alexandra-protyanova · 2022-12-13T13:05:35Z

src/styles/card.css

+  display: flex;
+  flex-direction: column;
+  width: 200px;
+  background: #fff;


default color for background

alexandra-protyanova · 2022-12-13T13:05:51Z

src/styles/stars.css

+  background-image: url(../images/star-active.svg);
+}
+
+.stars__star:last-child {


what about this comment?

chiper9 added 4 commits December 11, 2022 17:20

product card added

66eff5c

product card

2b23da7

product card

961bf0b

word sizes were changed

4ce769f

svitjojo suggested changes Dec 11, 2022

View reviewed changes

mistakes were removed

a790079

alexandra-protyanova suggested changes Dec 12, 2022

View reviewed changes

mistakes were removed

444c137

alexandra-protyanova suggested changes Dec 12, 2022

View reviewed changes

chiper9 mentioned this pull request Dec 12, 2022

haven't fixed #2325

Open

alexandra-protyanova suggested changes Dec 13, 2022

View reviewed changes

problems were fixed

b91e700

chiper9 force-pushed the develop branch from efb7aaa to b91e700 Compare December 13, 2022 12:01

alexandra-protyanova suggested changes Dec 13, 2022

View reviewed changes

stars problem was solved

400fb69

alexandra-protyanova approved these changes Dec 14, 2022

View reviewed changes

		<div class="word-price">Price:</div>
		<div class="price-value">$2,199</div>

Develop #2312

Are you sure you want to change the base?

Develop #2312

Conversation

chiper9 commented Dec 11, 2022

svitjojo 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

chiper9 commented Dec 11, 2022 via email

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

chiper9 Dec 13, 2022 • edited Loading

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

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

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

chiper9 Dec 13, 2022 •

edited

Loading