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 resolve for search bar #3444

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Anton201220
Copy link

src/style.css Outdated
Comment on lines 49 to 66
.input--big {
height: 70px;
padding-left: 62px;
font-size: 16px;
}

.search-form__input--big::before {
height: 19px;
width: 19px;
top: 25px;
left: 26px;
}

.input--sm {
height: 42px;
font-size: 14px;
padding-left: 33px;
}

Choose a reason for hiding this comment

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

2'nd point in checklist, create base class and one where you will override needed styles

src/index.html Outdated
Comment on lines 19 to 37
<form
action="#"
method="post"
data-qa="big"
class="search-form"
>

<div class="search-form__input search-form__input--big">

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

</div>

</form>

Choose a reason for hiding this comment

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

Suggested change
<form
action="#"
method="post"
data-qa="big"
class="search-form"
>
<div class="search-form__input search-form__input--big">
<input
type="text"
class="input input--big "
data-qa="keypress"
placeholder="Try &quot;Los Angeles&quot;"
>
</div>
</form>
<form
action="#"
method="post"
data-qa="big"
class="search-form"
>
<div class="search-form__input search-form__input--big">
<input
type="text"
class="input input--big "
data-qa="keypress"
placeholder="Try &quot;Los Angeles&quot;"
>
</div>
</form>

src/index.html Outdated
class="search-form"
>

<div class="search-form__input search-form__input--big">

Choose a reason for hiding this comment

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

do you really need this wrapper

src/index.html Outdated
Comment on lines 28 to 33
<input
type="text"
class="input input--big "
data-qa="keypress"
placeholder="Try &quot;Los Angeles&quot;"
>

Choose a reason for hiding this comment

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

add name attribute to inputs

src/style.css Outdated
Comment on lines 1 to 11
@font-face {
font-family: Avenir-Book;
src: url(./fonts/Avenir-Book.ttf);
font-weight: 300;
}

@font-face {
font-family: Avenir-Heavy;
src: url(./fonts/Avenir-Heavy.ttf);
font-weight: 700;
}

Choose a reason for hiding this comment

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

7'th point in checklist

src/style.css Outdated
Comment on lines 19 to 40
.search-form::before {
content: "";
position: absolute;
background-image: url("./images/Search.svg");
background-position: center;
background-size: cover;
background-repeat: no-repeat;
}

.search-form--big::before {
height: 19px;
width: 19px;
top: 25px;
left: 26px;
}

.search-form--sm::before {
width: 11px;
height: 11px;
top: 15px;
left: 13px;
}

Choose a reason for hiding this comment

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

2'nd point in checklist

src/style.css Outdated
Comment on lines 42 to 68
.input {
width: 100%;
box-sizing: border-box;
font-family: "Avenir-Book", Arial, sans-serif;
font-weight: 700;

border-radius: 4px;
border: 1px solid var(--border-color);
background: var(--background-color);
box-shadow: 0 1px 8px 0 rgba(61, 78, 97, 0.1);
}
/* if i*m wrong about fonts. I've confused in this - - Remember that inputs and
other interactive elements don’t inherit font styles by default.
- Remember that placeholder has its own set of styles available using `
::placeholder` pseudo-element. */

.input--big {
height: 70px;
padding-left: 62px;
font-size: 16px;
}

.input--sm {
height: 42px;
font-size: 14px;
padding-left: 33px;
}

Choose a reason for hiding this comment

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

2'nd point in checklist

src/style.css Outdated
Comment on lines 1 to 5
@font-face {
font-family: Avenir-Book;
src: url(./fonts/Avenir-Book.ttf);
font-weight: 300;
}

Choose a reason for hiding this comment

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

you need to connect this font with other font-weight also, check the design

Comment on lines +46 to 48
</form>

</body>

Choose a reason for hiding this comment

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

Suggested change
</form>
</body>
</form>
</body>

don't add spaces between child and parent elements

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