add task solution #4490
add task solution #4490
Conversation
Ga1dar
commented
Sep 26, 2024
•
Ga1dar
commented
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Hey there! You've done a great job so far, but there are a few critical issues that need your attention before we can move forward 🛠️. The pseudo-classes need to be fixed to ensure proper functionality, and there are some important corrections to be made regarding CSS properties and values. Also, let's make sure the @font-face rule is set up correctly for different font weights and styles. Keep up the good work, and with these adjustments, your code will be even better! 💪
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/index.html
Outdated
| <form | ||
| class="search-container" | ||
| action="#" | ||
| method="post" | ||
| data-qa="keypress" |
There was a problem hiding this comment.
Keep your attributes correctly formatted. Since the <form> element has more than two attributes, start each one on a new line with 2-space indentation related to the tag.
src/index.html
Outdated
| <div class="search-field large"> | ||
| <input | ||
| type="text" | ||
| placeholder="Try “Los Angeles“" | ||
| data-qa="big" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Remember about correct indentation between parent and child elements. The <input> tag should be indented two spaces from the <div> tag.
src/index.html
Outdated
| <div class="search-field small"> | ||
| <input | ||
| type="text" | ||
| data-qa="small" | ||
| placeholder="Try “Los Angeles“" | ||
| /> |
There was a problem hiding this comment.
Remember about correct indentation between parent and child elements. The <input> tag should be indented two spaces from the <div> tag.
src/style.css
Outdated
| html { | ||
| font-family: Avenir, sans-serif; | ||
| font-size: 16px; | ||
| font-weight: 300px; |
There was a problem hiding this comment.
The font-weight property should be specified using numeric values, not pixels. For example, font-weight: 300;.
src/style.css
Outdated
| font-family: Avenir, sans-serif; | ||
| font-size: 16px; | ||
| font-weight: 300px; | ||
| line-height: 21.86px; |
There was a problem hiding this comment.
The line-height property typically does not require a unit unless it's relative to the font size, in which case 'em' or unitless values are commonly used. For example, line-height: 1.35;.
src/style.css
Outdated
| font-family: Avenir, sans-serif; | ||
| padding: 0; | ||
| width: 100vw; /* Полная ширина окна */ | ||
| height: 100vh; /* Полная высота окна */ |
There was a problem hiding this comment.
Setting height to 100vh on the body tag is not recommended as it can lead to unwanted overflow issues with the content. Use min-height if you want to ensure the body takes at least the viewport height.
src/style.css
Outdated
| height: 70px; /* Высота для большой строки поиска */ | ||
| box-sizing: border-box; | ||
| font-size: 18px; /* Размер текста побольше */ | ||
| margin-bottom: 20px; /* Отступ между строками */ |
There was a problem hiding this comment.
The margin-bottom property is being set on the input element. According to the checklist, it's recommended to use either margin-top or margin-bottom but not both to avoid potential margin collapse issues.
src/style.css
Outdated
| height: 50px; /* Меньшая высота для второй строки поиска */ | ||
| box-sizing: border-box; | ||
| font-size: 16px; /* Меньший размер текста */ | ||
| box-shadow: 5px 5px 5px 5px rgba(0, 0, 0, 0.25); |
There was a problem hiding this comment.
The box-shadow property here is using pixel units for blur and spread radius, which is fine, but ensure consistency with the box-shadow in .search-field.large input.
src/style.css
Outdated
| box-shadow: 5px 5px 5px 5px rgba(0, 0, 0, 0.25); | ||
| } | ||
|
|
||
| .search-field :hover { |
There was a problem hiding this comment.
The :hover pseudo-class should be attached to the class without a space (e.g., .search-field:hover) to ensure it is applied when the user hovers over the .search-field element.
src/style.css
Outdated
| box-shadow: 1px 5px 5px 1px rgba(0, 0, 0, 0.25); | ||
| } | ||
|
|
||
| .search-field :focus { |
There was a problem hiding this comment.
The :focus pseudo-class should be attached to the class without a space (e.g., .search-field:focus) to ensure it is applied when the .search-field element is focused.
There was a problem hiding this comment.
Great job on the HTML structure! 🌟 There are a few formatting issues to iron out, like correcting the font-weight value and ensuring consistent margins, but they don't significantly impact the functionality. Remember to use the correct selectors for HTML elements and keep your code tidy for better readability. Keep it up! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <html | ||
| lang="en" | ||
| class="html" | ||
| > |
There was a problem hiding this comment.
The attributes of the <html> tag should be indented with 2 spaces. The closing bracket should be aligned with the opening tag.
| <link | ||
| rel="stylesheet" | ||
| href="style.css" | ||
| href="/src/style.css" | ||
| /> |
There was a problem hiding this comment.
The attributes of the <link> tag should be indented with 2 spaces. The closing bracket should be aligned with the opening tag.
| type="text" | ||
| data-qa="keypress" | ||
| placeholder="Try “Los Angeles“" | ||
| class="form__searchBar form__searchBar--big" |
There was a problem hiding this comment.
Avoid using multiple classes that modify the same element in a way that their purpose overlaps, such as form__searchBar and form__searchBar--big. If --big is a modifier, ensure it only adds modifications to form__searchBar rather than redefining styles.
| type="text" | ||
| data-qa="hover" | ||
| placeholder="Try “Los Angeles“" | ||
| class="form__searchBar form__searchBar--small" |
There was a problem hiding this comment.
Similar to the previous comment, ensure that form__searchBar--small is only a modifier and does not conflict with the base form__searchBar class.
| .html { | ||
| font-family: Avenir, sans-serif; |
There was a problem hiding this comment.
The selector '.html' seems to be a mistake. If you intended to style the <html> element, you should use the html selector without the dot.
| .body { | ||
| font-family: Avenir, sans-serif; |
There was a problem hiding this comment.
The selector '.body' seems to be a mistake. If you intended to style the <body> element, you should use the body selector without the dot.
| margin-top: 0; | ||
| margin-bottom: 0; |
There was a problem hiding this comment.
According to the checklist, be consistent with your margins. It's recommended to add only top or bottom margin, but not both to avoid potential margin collapse issues.
|
|
||
| .form__searchBar--big { | ||
| height: 70px; | ||
| font-weight: 16px; |
There was a problem hiding this comment.
The .form__searchBar--big class has a font-weight property with a value of 16px, which is incorrect. The font-weight property should be a keyword or number without a unit, such as bold or 400.
