Add search-bar #4505
Add search-bar #4505
Conversation
There was a problem hiding this comment.
Great job on ordering the CSS imports correctly and ensuring no issues were found! Your attention to detail is commendable. Keep up the good work, and always strive to maintain this level of code quality. 👍💼 Remember to keep an eye on the little details mentioned in the feedback for future improvements!
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <input | ||
| type="text" | ||
| data-qa="keypress" | ||
| placeholder="Try “Los Angeles“" | ||
| class="search-bar__input" | ||
| /> |
There was a problem hiding this comment.
The attributes of the input element should each start on a new line with 2-space indentation relative to the tag, as there are more than two attributes. The tag’s closing bracket should be on the same level as the opening one. This will improve readability as per the checklist guidelines.
| <input | ||
| type="text" | ||
| placeholder="Try “Los Angeles“" | ||
| class="search-bar__input" | ||
| /> |
There was a problem hiding this comment.
Same as the previous comment, the attributes of this input element should be formatted correctly, with each attribute starting on a new line and proper indentation.
| font-family: Avenir; | ||
| src: url(../../fonts/Avenir.ttc); | ||
| font-weight: 400; |
There was a problem hiding this comment.
Remember to use fallback fonts - alternative font-family in case the main one doesn't work. You can specify a list of comma-separated fonts to be used, ending with a generic font family, in case none of the specified fonts are available. For example: font-family: Avenir, Arial, sans-serif;.
| @@ -0,0 +1,15 @@ | |||
| .search-bar--big { | |||
| height: 70px; | |||
There was a problem hiding this comment.
Avoid fixing the height of elements if there is no specific requirement for it. This can lead to content overflow or unexpected layout issues when the content changes or scales. Let the content dictate the size of its container.
| @@ -0,0 +1,14 @@ | |||
| .search-bar--small { | |||
| height: 42px; | |||
There was a problem hiding this comment.
Avoid fixing the height of elements if there is no specific requirement for it. Let the content size dictate the container's size to prevent overflow or accidental scrollbars.
|
|
||
| .search-bar--small .search-bar__input { | ||
| height: 20px; | ||
| margin-left: 33px; |
There was a problem hiding this comment.
Be consistent with your margins to avoid potential margin collapse. Prefer to add margin on one side (either top or bottom) rather than on both sides.
| border: 1px solid rgb(225, 231, 237); | ||
| border-radius: 4px; | ||
| box-shadow: 0 1px 8px 0 rgba(61, 78, 97, 0.1); | ||
| margin-top: 20px; |
There was a problem hiding this comment.
Be consistent with your margins. It's better to add only top or bottom margins to avoid potential margin collapse issues. If the design allows it, consider using only margin-bottom or only margin-top for spacing between elements.
| padding: 0; | ||
| border: 0; | ||
|
|
||
| font-family: Avenir, Arial, 'Helvetica Neue', Helvetica, 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. This ensures that if the primary font fails to load for some reason, the browser has alternative fonts to fall back on. For example, you could specify a generic font family as a fallback: font-family: Avenir, Helvetica, Arial, sans-serif;
DEMO LINK
TEST REPORT LINK
Icon implemented using background-image CSS property
Inputs are written inside of 'form' tag with correctly passed attributes
All
Typical MistakesfromBEMlesson theory are checked.Code follows all the Code Style Rules ❗️