added solution #4491
added solution #4491
Conversation
There was a problem hiding this comment.
Great job on the Pull Request! There are a few minor issues to work on in the future, like adjusting font-weight changes, adding fallback fonts, and being consistent with margins. Also, pay attention to indentation, empty lines for readability, and attribute formatting. These are not critical, but addressing them will make your code even better! Keep it up! 👍💻
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| placeholder="Try “Los Angeles“" | ||
| /> | ||
| </div> | ||
| </form> |
There was a problem hiding this comment.
Add empty lines between multiline sibling blocks of HTML. There should be an empty line between the two form elements to improve readability.
| action="#" | ||
| data-qa="big" | ||
| > | ||
| <div class="searchbar__container"> |
There was a problem hiding this comment.
Remember about correct indentation between parent and child elements. The div element should be indented two spaces inside the form element.
| action="#" | ||
| data-qa="small" | ||
| > | ||
| <div class="searchbar__container"> |
There was a problem hiding this comment.
Remember about correct indentation between parent and child elements. The div element should be indented two spaces inside the form element.
| .form:not(:last-child) { | ||
| margin-bottom: 20px; | ||
| margin-top: 20px; |
There was a problem hiding this comment.
Be consistent with your margins (Add only top or bottom, but not both). This helps to avoid potential margin collapse and keeps the styling consistent.
| background-image: url('./images/Search.svg'); | ||
| background-repeat: no-repeat; | ||
| box-sizing: border-box; | ||
| font-family: Avenir, Arial, 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 that you have 'Arial, sans-serif' as fallbacks, but ensure they are correctly ordered according to their similarity to the primary font.
| .searchbar--small:focus, | ||
| .searchbar--big:focus { | ||
| border-color: rgb(225, 231, 237); | ||
| font-weight: 900; |
There was a problem hiding this comment.
Avoid changing font-weight on focus as it can cause layout shifts. Consider using other styles like border or color changes to indicate focus.
❗️ Copy this
Checklistto thePull Requestdescription after links, and put- [x]before each point after you checked it.Typical MistakesfromBEMlesson theory are checked.