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.

Sign up for GitHub

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

Jump to bottom

Solution #4499

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

Solution #4499

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
376a7a9
Update style.css
cassiaqueiroz Sep 30, 2024
3281e81
Update style.css
cassiaqueiroz Sep 30, 2024
f77207f
add solution
cassiaqueiroz Sep 30, 2024
ecf1950
add solution
cassiaqueiroz Sep 30, 2024
5c76928
add solution
cassiaqueiroz Sep 30, 2024
8df2641
Update index.html
cassiaqueiroz Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ ___

❗️ Replace `<your_account>` with your Github username and copy the links to `Pull Request` description:

- [DEMO LINK](https://<your_account>.github.io/layout_search-bar-airbnb/)
- [TEST REPORT LINK](https://<your_account>.github.io/layout_search-bar-airbnb/report/html_report/)
- [DEMO LINK](https://cassiaqueiroz.github.io/layout_search-bar-airbnb/)
- [TEST REPORT LINK](https://cassiaqueiroz.github.io/layout_search-bar-airbnb/report/html_report/)

❗️ Copy this `Checklist` to the `Pull Request` description after links, and put `- [x]` before each point after you checked it.

- [ ] Icon implemented using background-image CSS property
- [ ] Inputs are written inside of 'form' tag with correctly passed attributes
- [ ] All `Typical Mistakes` from `BEM` lesson theory are checked.
- [ ] Code follows all the [Code Style Rules ❗️](./checklist.md)
- [x] Icon implemented using background-image CSS property
- [x] Inputs are written inside of 'form' tag with correctly passed attributes
- [x] All `Typical Mistakes` from `BEM` lesson theory are checked.
- [x] Code follows all the [Code Style Rules ❗️](./checklist.md)
37 changes: 24 additions & 13 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<!doctype html>
<html lang="en">
<head>
<head class="header">
cassiaqueiroz marked this conversation as resolved.
Show resolved Hide resolved
<meta charset="UTF-8" />
<meta
name="viewport"
Expand All @@ -10,23 +10,34 @@
http-equiv="X-UA-Compatible"
content="ie=edge"
/>
<title>Document</title>
<title>Airbnb Search Bar</title>
<link
rel="stylesheet"
href="style.css"
cassiaqueiroz marked this conversation as resolved.
Show resolved Hide resolved
/>
</head>
<body>
<input
type="text"
data-qa="keypress"
placeholder="Try “Los Angeles“"
/>

<input
type="text"
data-qa="hover"
placeholder="Try “Los Angeles“"
/>
<body class="page__body">
cassiaqueiroz marked this conversation as resolved.
Show resolved Hide resolved
<form
class="search-bar"
data-qa="big"
>
<input
class="search-bar__input"
data-qa="keypress"
type="text"
placeholder="Try “Los Angeles”"
Copy link

Choose a reason for hiding this comment

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

Use semantic tags where possible. Consider wrapping the <input> elements in a semantic tag like <section> or <div> to indicate a block of related content.

/>
Comment on lines +25 to +30
Copy link

Choose a reason for hiding this comment

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

Keep your attributes correctly formatted. Since the input element has more than two attributes, each attribute should start on a new line with 2-space indentation relative to the tag.

Comment on lines +21 to +30
Copy link

Choose a reason for hiding this comment

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

Keep your attributes correctly formatted. Since the <input> element on lines 25-30 has more than two attributes, each attribute should start on a new line with 2-space indentation relative to the tag.

</form>
<form
class="search-bar"
data-qa="small"
>
<input
class="search-bar__input search-bar__input--small"
type="text"
placeholder="Try “Los Angeles”"
Comment on lines +36 to +39
Copy link

Choose a reason for hiding this comment

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

Keep your attributes correctly formatted. Since the input element has more than two attributes, each attribute should start on a new line with 2-space indentation relative to the tag.

/>
Comment on lines +32 to +40
Copy link

Choose a reason for hiding this comment

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

Keep your attributes correctly formatted. Since the <input> element on lines 36-40 has more than two attributes, each attribute should start on a new line with 2-space indentation relative to the tag.

</form>
</body>
</html>
85 changes: 84 additions & 1 deletion src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,84 @@
/* add styles here */
@font-face {
font-family: Avenir;
src: url('fonts/Avenir.ttc');
font-weight: 300;
}

@font-face {
font-family: Avenir;
src: url('fonts/Avenir.ttc');
font-weight: bold;
}
Comment on lines +1 to +10
Copy link

Choose a reason for hiding this comment

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

Consider adding a fallback font family in your @font-face rule to ensure text remains visible when the custom font is loading or if it fails to load. For example, you could modify the font-family property to include a generic font family like sans-serif as a backup.


:root {
--font-family: Avenir, Arial, sans-serif;
--font-color: #3d4e61;
--background-color: #fff;
--border-color: #e1e7ed;
--box-shadow: rgba(61, 78, 97, 0.1);
--hover-box-shadow: rgba(61, 78, 97, 0.2);
--placeholder-color: #3d4e61;
--search-icon-size: 19px;
--search-icon-small-size: 11px;
--search-bar-height: 70px;
--search-bar-small-height: 42px;
--font-size: 16px;
--font-size-small: 14px;
}

cassiaqueiroz marked this conversation as resolved.
Show resolved Hide resolved
.page__body {
padding: 0;
}
Copy link

Choose a reason for hiding this comment

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

It's a good practice to set margin: 0; along with padding: 0; on the body element to reset any default margins provided by the browser.


.search-bar {
margin-top: 20px;
Copy link

Choose a reason for hiding this comment

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

Consider adding a bottom margin to the .search-bar instead of a top margin to be consistent with your margins, as per the checklist item 'Be consistent with your margins (Add only top or bottom, but not both)'.

}
Copy link

Choose a reason for hiding this comment

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

Be consistent with your margins. Since you're adding a margin to the top, ensure that you're not adding margin to the bottom of any elements within .search-bar to avoid potential margin collapse issues.


Comment on lines +33 to +35
Copy link

Choose a reason for hiding this comment

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

Be consistent with your margins. It's recommended to add only top or bottom margins, but not both, to avoid potential margin collapse issues.

.search-bar__input {
box-sizing: border-box;
font-family: var(--font-family);
font-size: var(--font-size);
color: var(--font-color);
background-color: var(--background-color);
border: 1px solid var(--border-color);
border-radius: 4px;
background-image: url(./images/Search.svg);
background-repeat: no-repeat;
background-size: var(--search-icon-size);
background-position: 25px;
width: 100%;
height: var(--search-bar-height);
Copy link

Choose a reason for hiding this comment

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

Don't fix container size (if there is no such a requirement). The width of the .search-bar__input is set to 100%, which is good as it allows the container to be flexible. However, ensure that this is not causing any overflow or accidental scrollbar, especially when padding or borders are added.

padding-left: 62px;
box-shadow: 0 1px 8px var(--box-shadow);
Copy link

Choose a reason for hiding this comment

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

Avoid fixing the padding size. Instead, consider using relative units or a variable if the design allows for flexibility. This can make the design more responsive and easier to maintain.

outline: none;
}

.search-bar__input::placeholder {
color: var(--placeholder-color);
}

.search-bar__input:hover {
box-shadow: 0 3px 8px var(--hover-box-shadow);
}

.search-bar__input:focus {
font-weight: bold;
background-position: 25px;
box-shadow: 0 3px 8px var(--hover-box-shadow);
text-shadow: 0 4px 4px rgba(0, 0, 0, 0.25);
}

.search-bar__input--small {
background-size: var(--search-icon-small-size);
Copy link

Choose a reason for hiding this comment

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

The .search-bar__input--small class correctly overrides the necessary properties for the smaller search bar variant, maintaining consistency in the design. However, make sure that the font-weight: 400; is necessary here, as the normal font weight is usually the default and might not need to be specified unless it's overriding a previous setting.

background-position: 12px;
height: var(--search-bar-small-height);
font-size: var(--font-size-small);
font-weight: 400;
padding-left: 33px;
}

.search-bar__input--small:focus {
background-position: 12px;
background-size: var(--search-icon-small-size);
text-shadow: none;
}
Loading