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

add task solution #4545

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

add task solution #4545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2c717d
add task solution
veronikamaliar Oct 16, 2024
f994f5d
add task solution
veronikamaliar Oct 16, 2024
045114e
add task solution
veronikamaliar Oct 16, 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
4 changes: 2 additions & 2 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ ___

❗️ 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://veronikamaliar.github.io/layout_search-bar-airbnb/)
- [TEST REPORT LINK](https://veronikamaliar.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.

Expand Down
42 changes: 30 additions & 12 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,41 @@
http-equiv="X-UA-Compatible"
content="ie=edge"
/>
<title>Document</title>
<title>Search-bar</title>

<link
rel="stylesheet"
href="style.css"
/>
</head>
<body>
<input
type="text"
data-qa="keypress"
placeholder="Try “Los Angeles“"
/>

<input
type="text"
data-qa="hover"
placeholder="Try “Los Angeles“"
/>
<form
action="#"
data-qa="big"
>
<label for="search-input">
Copy link

Choose a reason for hiding this comment

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

The 'for' attribute of the label element must be equal to the id of a non-hidden form control. Please add an 'id' attribute to the input and make sure it matches with 'for' attribute of the label. This is an accessibility issue.

<input
id="search-input"
class="search-bar search-bar--big"
type="text"
data-qa="keypress"
placeholder="Try “Los Angeles“"
/>
Comment on lines +26 to +32
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. If the HTML-element has long attribute values or number of attributes is more than 2 - start each one, including the first, on the new line with 2-space indentation related to tag. Tag’s closing bracket should be on the same level as opening one.

</label>
Copy link

Choose a reason for hiding this comment

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

As per HTML Content checklist, it's recommended to use semantic tags where possible. In this case, 'label' tag is used without a 'for' attribute and without any text, which doesn't provide any semantic value. Consider removing the 'label' tag if it's not necessary, or provide a 'for' attribute linking to the 'id' of the input field.

Comment on lines +25 to +33
Copy link

Choose a reason for hiding this comment

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

Remember about correct indentation between parent and child elements. Each level of nesting, including text, contained inside the element, requires 2-space offset. Also blank line shouldn't be between parent and child elements.

</form>
Comment on lines +21 to +34
Copy link

Choose a reason for hiding this comment

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

According to the checklist, you should add empty lines between multiline sibling blocks of HTML. This will improve readability.

<form
action="#"
data-qa="small"
>
<label for="search-input2">
<input
id="search-input2"
class="search-bar search-bar--small"
type="text"
data-qa="hover"
placeholder="Try “Los Angeles“"
/>
</label>
Copy link

Choose a reason for hiding this comment

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

Same issue here as in line 32. Please revise your usage of 'label' tag.

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

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

body {
font-family: Avenir, Arial, Helvetica, sans-serif;
}
Copy link

Choose a reason for hiding this comment

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

Remember to use fallback fonts - alternative font-family in case the main one doesn't work. You can add 'Arial, Helvetica, sans-serif' as fallback fonts for 'Avenir'.


.search-bar {
font-family: Avenir, sans-serif;
width: 100%;
Copy link

Choose a reason for hiding this comment

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

You have defined the font-family for the class 'search-bar' as 'Avenir'. However, you have not defined a font-face for 'Avenir'. You should either define a font-face for 'Avenir' or use one of the fonts you have defined, like 'Avenir Book' or 'Avenir Heavy'.

Copy link

Choose a reason for hiding this comment

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

Remember to use fallback fonts - alternative font-family in case the main one doesn't work. You can add 'Arial, Helvetica, sans-serif' as fallback fonts for 'Avenir'.

display: block;
Copy link

Choose a reason for hiding this comment

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

Avoid fixing container size if there is no such requirement. Let the content size dictate it. To avoid overflow or accidental scroll bar.

margin-top: 20px;
background: #fff;
Copy link

Choose a reason for hiding this comment

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

You have added a top margin to the 'search-bar' class. This is not a good practice as per the checklist. Instead of adding margins to the top and bottom, it is recommended to add them to either the top or the bottom to avoid potential margin collapse.

border: 1px solid #e1e7ed;
border-radius: 5px;
box-shadow: 0 1px 8px 0 #3d4e611a;
box-sizing: border-box;
}

.search-bar--big {
height: 70px;
font-size: 16px;
Copy link

Choose a reason for hiding this comment

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

You have fixed the height of the 'search-bar__big' class. Unless there is a requirement, it is recommended not to fix the container size. Let the content size dictate it to avoid overflow or accidental scroll bars.

padding-left: 62px;
background-image: url(images/Search.svg);
background-repeat: no-repeat;
background-size: 19px;
background-position: left 26px center;
background-color: #fff;
}

.search-bar--small {
height: 42px;
font-size: 14px;
Copy link

Choose a reason for hiding this comment

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

Similar to the 'search-bar__big' class, you have also fixed the height of the 'search-bar__small' class. It is recommended not to fix the container size unless there is a requirement.

padding-left: 33px;
background-image: url(images/Search.svg);
background-repeat: no-repeat;
background-size: 11px;
background-position: left 13px center;
background-color: #fff;
}

.search-bar:hover {
box-shadow: 0 3px 8px 0 #3d4e6133;
}

.search-bar--big:focus {
font-weight: bold;
background:
url('./images/Search.svg') no-repeat,
linear-gradient(180deg, transparent, #f6f6f7);
background-size: 19px;
background-position: left 26px center;
border: 1px solid #e1e7ed;
box-shadow: 0 4px 4px 0 #00000040;
outline: none;
}

.search-bar--small:focus {
font-weight: bold;
background:
url('./images/Search.svg') no-repeat,
linear-gradient(180deg, transparent, #f6f6f7);
background-size: 11px;
background-position: left 13px center;
border: 1px solid #e1e7ed;
box-shadow: 0 4px 4px 0 #00000040;
outline: none;
}

.search-bar::placeholder {
color: #3d4e61;
}
Loading