Develop #3887

lLiashko · 2023-12-16T18:17:32Z

VitaliyBondarenko1982

Good job.
Please check comments

VitaliyBondarenko1982 · 2023-12-16T18:38:43Z

src/index.html

-    <h1>Search bar airbnb</h1>
+  <body data-qa="hover">
+    <!--First bar-->
+    <div class="search-bar search-bar__big" data-qa="big">


Suggested change

<div class="search-bar search-bar__big" data-qa="big">

<form class="search-bar" data-qa="big">

use form tag instead div
search-bar__big element is redundant.
set all styles for search-bar and just change necessary for small form in search-bar__small element.
same for other elements

VitaliyBondarenko1982 · 2023-12-16T18:39:19Z

src/style.css

+  padding: 0;
+  border: 0;


Suggested change

padding: 0;

border: 0;

redundant. body doesn't have default paddings or magins.

VitaliyBondarenko1982 · 2023-12-16T18:39:48Z

src/style.css

+input {
+  width: 100%;
+}


style by class instead by tag

Suggested change

input {

width: 100%;

}

VitaliyBondarenko1982 · 2023-12-16T18:41:09Z

src/style.css

+  width: 100%;
+}
+
+input::placeholder {


Suggested change

input::placeholder {

.search-bar__text::placeholder {

VitaliyBondarenko1982 · 2023-12-16T18:42:43Z

src/style.css

+  color: #3d4e61;
+  font-family: Avenir, sans-serif;
+  font-weight: 400;
+  line-height: normal;


instead width 100% you can make this element block. it will allow to set height, and width will be 100% by default

Suggested change

line-height: normal;

line-height: normal;

display: block;

height: 100%;

Still not fixed

VitaliyBondarenko1982 · 2023-12-16T18:43:28Z

src/style.css

+.search-bar__text-big {
+  margin-left: 60px;
+  padding-right: 29px;
+  font-size: 16px;
+  width: 129px;
+  height: 22px;


Suggested change

.search-bar__text-big {

margin-left: 60px;

padding-right: 29px;

font-size: 16px;

width: 129px;

height: 22px;

.search-bar__text-big {

padding-left: 60px;

padding-right: 29px;

font-size: 16px;

VitaliyBondarenko1982 · 2023-12-16T18:45:10Z

src/style.css

+.search-bar__text-small {
+  justify-content: flex-end;
+  font-size: 14px;
+  width: 126.29px;
+  height: 20px;
+  padding-left: 33px;
+}


Suggested change

.search-bar__text-small {

justify-content: flex-end;

font-size: 14px;

width: 126.29px;

height: 20px;

padding-left: 33px;

}

.search-bar__text-small {

justify-content: flex-end;

font-size: 14px;

height: 20px;

padding-left: 33px;

}

do not hardcode width. it will make visible placeholder and will be possible to write text on full input

Thank you for comments! I've changed my code

volodymyr-soltys97

Good job 👍
Let's improve your code
The search must span the entire width of the browser window

volodymyr-soltys97 · 2023-12-17T07:30:46Z

src/index.html

-  <body>
-    <h1>Search bar airbnb</h1>
+  <body data-qa="hover">
+    <!--First bar-->


Comment is redundant here

Suggested change

They help me to find right bar quicker

volodymyr-soltys97 · 2023-12-17T07:30:59Z

src/index.html

+      <input
+        class="search-bar__text"
+        type="text"
+        placeholder="Try &quot;Los Angeles&quot;"
+        data-qa="keypress">


Suggested change

<input

class="search-bar__text"

type="text"

placeholder="Try "Los Angeles""

data-qa="keypress">

<input

class="search-bar__text"

type="text"

placeholder="Try "Los Angeles""

data-qa="keypress"

>

That is correct formatting - it's better to stick to it

volodymyr-soltys97 · 2023-12-17T07:31:13Z

src/index.html

+        placeholder="Try &quot;Los Angeles&quot;"
+        data-qa="keypress">
+    </form>
+    <!--Second bar-->


Suggested change

volodymyr-soltys97 · 2023-12-17T07:31:34Z

src/index.html

+        data-qa="keypress">
+    </form>
+    <!--Second bar-->
+    <div class="search-bar search-bar__small" data-qa="small">


Use a semantic tag form here instead of div

volodymyr-soltys97 · 2023-12-17T07:32:34Z

src/style.css

+  font-family: Avenir;
+  src: url("./fonts/Avenir-Book.ttf") format("truetype");
+  font-weight: 300;
+  font-style: normal;


Default value, so you can just remove it everywhere

Suggested change

font-style: normal;

volodymyr-soltys97 · 2023-12-17T07:32:47Z

src/style.css

@@ -1 +1,107 @@
 /* add styles here */
+@font-face {
+  font-family: Avenir;
+  src: url("./fonts/Avenir-Book.ttf") format("truetype");


Suggested change

src: url("./fonts/Avenir-Book.ttf") format("truetype");

src: url("./fonts/Avenir-Book.ttf");

Another mentor in chat sent me this example, so I don't really understand which variant is right

you may google what does it mean

generally, it's needed to explicitly set the format of the font file:
https://css-tricks.com/snippets/css/using-font-face-in-css/

but it could work w/o it

so you may keep any variant

volodymyr-soltys97 · 2023-12-17T07:33:07Z

src/style.css

+  color: #3d4e61;
+  font-family: Avenir, sans-serif;
+  font-weight: 400;
+  line-height: normal;


Still not fixed

volodymyr-soltys97 · 2023-12-17T07:33:26Z

src/style.css

+/* #endregion */
+
+/* #region text */


Remove comments everywhere

Suggested change

/* #endregion */

/* #region text */

Could you please explain why? In lecture videos they are used almost everywhere and it really helps

yes, it could be helpful during the development (when you write code), but it's not ok to push it (in case this code goes to production)
Generally, it's not critical here, but at least FYI - we don't want to push such code in production in real life

In the future you will divide a separate blocks in separate files anyway 🙂

thank you foR the explanation!

TarasHoliuk

GJ 👍
Some fixes are still needed to make it better)

TarasHoliuk · 2023-12-17T10:27:57Z

src/index.html

+      <input
+        class="search-bar__text"
+        type="text"
+        placeholder="Try &quot;Los Angeles&quot;"
+        data-qa="keypress">


That is correct formatting - it's better to stick to it

TarasHoliuk · 2023-12-17T10:29:50Z

src/style.css

+  background: linear-gradient(180deg, #FFF 0%, #F6F6F7 100%);
+}
+
+.search-bar input[type="text"] {


do not use such selectors (combined)
you have 1 class for it

Suggested change

.search-bar input[type="text"] {

.search-bar-text {

TarasHoliuk · 2023-12-17T10:30:27Z

src/style.css

+  color: #3d4e61;
+  font-family: Avenir, sans-serif;
+  font-weight: 400;
+  line-height: normal;


agree - no need to add default browser styles:

TarasHoliuk · 2023-12-17T10:32:39Z

src/style.css

+  padding-right: 75%;
+}
+
+.search-bar__text:focus {


you should add a gradient to the input, and make it 100% width

also typed text should have a different font-weight - 800 (Avenir-Heavy)

TarasHoliuk · 2023-12-17T10:32:51Z

src/style.css

+  justify-content: flex-end;
+  font-size: 14px;
+  padding-left: 33px;
+  padding-right: 75%;


TarasHoliuk · 2023-12-17T10:33:33Z

src/style.css

+}
+
+.search-bar:hover {
+  background: #FFF;


redundant
it might overwrite gradient when input is focused

anastasiiavorobiova

Great work! Almost done

anastasiiavorobiova · 2023-12-17T12:12:43Z

src/index.html

+        type="text"
+        placeholder="Try &quot;Los Angeles&quot;"
+        data-qa="keypress"
+        >


Suggested change

>

>

anastasiiavorobiova · 2023-12-17T12:19:16Z

src/style.css

+  font-family: Avenir, sans-serif;
+  font-weight: 400;
+  padding-left: 62px;
+  padding-right: 68%;


Suggested change

padding-right: 68%;

padding-right: 62px;

src/style.css

+  border: none;
+  outline: none;
+  display: block;
+  height: 100%;


anastasiiavorobiova

Great work!

lLiashko added 5 commits December 14, 2023 19:18

not ready yet

dd21e09

not ready yet

e3eb105

updated

6ede30f

almost done

677076d

finished task

775faab

VitaliyBondarenko1982 suggested changes Dec 16, 2023

View reviewed changes

lLiashko added 2 commits December 16, 2023 19:52

done

ce2c65f

improved

44f2935

lLiashko requested a review from VitaliyBondarenko1982 December 16, 2023 19:21

volodymyr-soltys97 suggested changes Dec 17, 2023

View reviewed changes

lLiashko added 2 commits December 17, 2023 09:02

updated

690f831

updated

836ef35

lLiashko requested a review from volodymyr-soltys97 December 17, 2023 08:12

TarasHoliuk suggested changes Dec 17, 2023

View reviewed changes

updeted

af22887

lLiashko requested a review from TarasHoliuk December 17, 2023 11:55

anastasiiavorobiova suggested changes Dec 17, 2023

View reviewed changes

updated

4527b49

lLiashko requested a review from anastasiiavorobiova December 17, 2023 12:36

anastasiiavorobiova approved these changes Dec 17, 2023

View reviewed changes

	<div class="search-bar search-bar__big" data-qa="big">
	<form class="search-bar" data-qa="big">

	src: url("./fonts/Avenir-Book.ttf") format("truetype");
	src: url("./fonts/Avenir-Book.ttf");

Develop #3887

Are you sure you want to change the base?

Develop #3887

Conversation

lLiashko commented Dec 16, 2023

VitaliyBondarenko1982 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

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

TarasHoliuk 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

anastasiiavorobiova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

anastasiiavorobiova left a comment

Choose a reason for hiding this comment