-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 two search bar #3782
base: master
Are you sure you want to change the base?
add two search bar #3782
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GJ!
let's make some improvements.
- check my comments.
- check input text styles. they are different in figma.
src/style.css
Outdated
margin-left: 26px; | ||
margin-top: 25px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you're using flex you can center elements more easily) use flex properties (e.g. align-items)
src/style.css
Outdated
} | ||
|
||
.search__form:focus-within, | ||
.search__form:focus-within .search__input { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use input:focus instead of form:focus
src/style.css
Outdated
.search__form { | ||
box-sizing: border-box; | ||
display: flex; | ||
height: 70px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is bad practice to hardcode height. use proper padding instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/index.html
Outdated
<img | ||
src="./images/Search.svg" | ||
alt="search" | ||
class="icon"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<img | |
src="./images/Search.svg" | |
alt="search" | |
class="icon"> | |
<img | |
src="./images/Search.svg" | |
alt="search" | |
class="icon" | |
> |
Tests passed without errors, when I used height. But my code was rejected and mentor told me that it is better to use paddings. When i use padding, I have error in code. Chat with mentors could not help) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/style.css
Outdated
} | ||
|
||
.search__input:focus::placeholder { | ||
font-style: normal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant default value
font-style: normal; |
DEMO LINK
TEST REPORT LINK
Icon implemented using background-image CSS property
Inputs are written inside of 'form' tag with correctly passed attributes
All
Typical Mistakes
fromBEM
lesson theory are checked.Code follows all the Code Style Rules ❗️