Solution #4501
Solution #4501
Conversation
There was a problem hiding this comment.
Great job on ordering the import statements correctly and structuring the variables.css file! Keep up the good work with following best practices. 👍 Remember, every improvement counts, and attention to detail will make your code shine even brighter! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| outline: none; | ||
| font-weight: 900; |
There was a problem hiding this comment.
The use of font-weight: 900; on focus might lead to a text reflow which can be distracting for users. Consider maintaining consistent font weight or using other visual cues for focus state that do not affect the layout or text flow.
| background: linear-gradient( | ||
| var(--first-gradient-form-color), | ||
| var(--second-gradient-form-color) | ||
| ); |
There was a problem hiding this comment.
Ensure that the background gradient on focus enhances the usability and accessibility of the input field. If the gradient makes the text hard to read or the focus state less discernible, consider using a simpler background color change or a border to indicate focus.
| @@ -0,0 +1,7 @@ | |||
| .main { | |||
| margin: 0 8px; | |||
There was a problem hiding this comment.
Be consistent with your margins (Add only top or bottom, but not both). In this case, since the .main__form class already has a margin-top, you should consider removing the margin from the .main class or ensure that the vertical margins are only applied in one direction to avoid potential margin collapse issues.
| background: center no-repeat url(../images/Search.svg); | ||
| background-size: cover; |
There was a problem hiding this comment.
The background-size: cover; property on the .search-form__search-icon class might not be appropriate for an icon. The cover value scales the background image to cover the element's entire area, which could distort the icon if its aspect ratio doesn't match the element's. Consider using contain to ensure the entire icon is visible, or set explicit width and height values that match the icon's dimensions.
| .search-form--large:hover, | ||
| .search-form:has(.search-form__input--large:focus) { | ||
| box-shadow: 0 4px 4px var(--box-large-hover-shadow-color); |
There was a problem hiding this comment.
The selector .search-form:has(.search-form__input--large:focus) uses the :has() pseudo-class, which is not widely supported and may not work in all browsers. Consider using JavaScript to add a class to the .search-form element when the .search-form__input--large element is focused, and style that class instead.
| <input | ||
| type="text" | ||
| data-qa="keypress" | ||
| placeholder="Try “Los Angeles“" | ||
| class="search-form__input search-form__input--large" | ||
| /> |
There was a problem hiding this comment.
The placeholder text 'Try “Los Angeles“' should not include quotes as they are unnecessary in this context and can be confusing for users. Consider simplifying it to just 'Try Los Angeles'.
| <form | ||
| class="search-form search-form--small" | ||
| data-qa="small" | ||
| > | ||
| <a | ||
| href="#" | ||
| class="search-form__search-icon search-form__search-icon--small" | ||
| ></a> | ||
|
|
||
| <input | ||
| type="text" | ||
| data-qa="hover" | ||
| placeholder="Try “Los Angeles“" | ||
| class="search-form__input search-form__input--small" | ||
| /> |
There was a problem hiding this comment.
The attributes within the
and tags should start on a new line with 2-space indentation relative to the tag, and the tag’s closing bracket should be on the same level as the opening one, according to the checklist.| * { | ||
| box-sizing: border-box; | ||
| } |
There was a problem hiding this comment.
Don't use the * selector as it impacts performance. Set styles only for elements that require them.
|
|
||
| html, | ||
| input { | ||
| font-family: Avenir, Arial, 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. While you have added Arial and Helvetica as fallbacks, it's also a good practice to include a generic family name such as 'sans-serif'.
| input { | ||
| font-family: Avenir, Arial, Helvetica, sans-serif; | ||
| color: var(--text-color); | ||
| font-weight: 300; |
There was a problem hiding this comment.
Be consistent with your font-weight usage. If there is no specific design requirement for a lighter font-weight, it's common to use the default weight of 400 for readability.
DEMO LINK