add search-bar #4512
add search-bar #4512
Conversation
There was a problem hiding this comment.
Hey there! It looks like you've done a great job overall, but there are a couple of things that need your attention before we can move forward. Don't worry, they're easy fixes! 😊 First, remember to specify fallback fonts - it's a safety net that ensures your text remains readable even if the primary font fails to load. Also, let's make sure the alt texts for images are descriptive, this helps with accessibility and SEO. Lastly, using semantic tags like label in forms is a good practice that makes your code cleaner and more meaningful. Keep up the good work, and with these small tweaks, your code will be even better! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <form class="form form__1"> | ||
| <div class="input__wrapper"> | ||
| <img | ||
| src="images/Search.svg" | ||
| alt="search-logo" | ||
| class="input__icon input__icon__1" | ||
| /> | ||
| <input | ||
| class="input input__first" | ||
| type="text" | ||
| data-qa="big keypress" | ||
| placeholder="Try “Los Angeles“" | ||
| /> |
There was a problem hiding this comment.
Use semantic tags where possible. Instead of using div for wrapping input elements, consider using label which is more appropriate for form input elements.
| <img | ||
| src="images/Search.svg" | ||
| alt="search-logo" | ||
| class="input__icon input__icon__1" |
There was a problem hiding this comment.
The alt attribute should describe the image content. 'search-logo' is not descriptive of the image content. It should describe the action or purpose of the image, like 'Search icon'.
| <form class="form form__2"> | ||
| <div class="input__wrapper"> | ||
| <img | ||
| src="images/Search.svg" | ||
| alt="search-logo" | ||
| class="input__icon input__icon__2" | ||
| /> | ||
| <input | ||
| class="input input__second" | ||
| type="text" | ||
| data-qa="small" | ||
| placeholder="Try “Los Angeles“" | ||
| /> |
There was a problem hiding this comment.
Use semantic tags where possible. Similar to the previous form, consider using label for wrapping input elements.
| <img | ||
| src="images/Search.svg" | ||
| alt="search-logo" | ||
| class="input__icon input__icon__2" |
There was a problem hiding this comment.
The alt attribute should describe the image content. 'search-logo' is not descriptive of the image content. It should describe the action or purpose of the image, like 'Search icon'.
| } | ||
|
|
||
| body { | ||
| font-family: Avenir-Book, sans-serif; |
There was a problem hiding this comment.
Remember to use fallback fonts - alternative font-family in case the main one doesn't work. It's good practice to specify a generic family as a fallback, such as sans-serif for Avenir-Book.
| border-radius: 4px; | ||
| background-color: white; | ||
| box-shadow: 0 1px 8px 0 #3d4e611a; | ||
| font-family: Avenir-Book, sans-serif; |
There was a problem hiding this comment.
You've specified the font-family again for .input__first which is redundant since it's already been set for the body. If the intention is to have a consistent font throughout the site, setting it on the body should suffice.
| text-shadow: 0 4px 4px 0 #00000040; | ||
| outline: none; | ||
| color: #3d4e61; | ||
| font-family: Avenir-Heavy, sans-serif; |
There was a problem hiding this comment.
The font-family Avenir-Heavy should also have a fallback font specified, similar to the body's font-family declaration.
| justify-content: center; /* Центрування форми по горизонталі */ | ||
| align-items: center; | ||
| width: 100%; /* Ширина для flex-контейнера */ | ||
| margin-top: 20px; |
There was a problem hiding this comment.
Be consistent with your margins (Add only top or bottom, but not both). Here, you've added a margin-top to .form, which might lead to margin collapse issues if you're not careful.
| border: 1px solid #e1e7ed; | ||
| border-radius: 4px; | ||
| box-shadow: 0 1px 8px 0 #3d4e611a; | ||
| font-family: Avenir-Book, sans-serif; |
There was a problem hiding this comment.
The font-family declaration here is redundant as well since it's the same as the one set for the body. You can remove it unless you need a different font for this specific element.
(дуже розчарований в цьому завданні, макет зроблений дуже погано, не вказані важливі параметри шрифтів, а тести взагалі показують що треба було робити медіа запити, про які в макеті нічого не сказано..)