-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Develop #4352
base: master
Are you sure you want to change the base?
Develop #4352
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.
- [DEMO LINK](https://Miranna472.github.io/layout_search-bar-airbnb/) | ||
- [TEST REPORT LINK](https://Miranna472.github.io/layout_search-bar-airbnb/report/html_report/) |
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
data-qa="hover" | ||
placeholder="Try “Los Angeles“" | ||
/> | ||
<div class="search-bar-container"> |
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.
Redundant
<div class="search-bar-container"> |
} | ||
|
||
body { | ||
font-family: Avenir, Arial, Helvetica, sans-serif; |
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.
Move font styles to the html selector
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.
when I move everything to html, all the fonts fly off. That's why I left it as it was
src/style.css
Outdated
box-sizing: border-box; | ||
font-size: 16px; | ||
text-align: left; | ||
font-weight: 300; | ||
font-style: normal; | ||
margin-top: 0; |
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.
Redundant
box-sizing: border-box; | |
font-size: 16px; | |
text-align: left; | |
font-weight: 300; | |
font-style: normal; | |
margin-top: 0; |
src/style.css
Outdated
margin-top: 20px; | ||
} | ||
|
||
.search-bar.big 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 only class selector instead of input, fix it everywhere
src/style.css
Outdated
.search-bar.big input:focus { | ||
font-size: 16px; | ||
font-weight: 900; | ||
line-height: 21.86px; |
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.
line-height: 21.86px; | |
line-height: 22px; |
src/style.css
Outdated
font-size: 16px; | ||
font-weight: 900; | ||
line-height: 21.86px; | ||
text-align: left; |
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.
Default value, you can just remove it everywhere
text-align: left; |
|
|
||
body { | ||
font-family: Avenir, Arial, Helvetica, sans-serif; | ||
box-sizing: border-box; |
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 common practice to apply same box-sizing for all elements
No description provided.