add solution #4497
add solution #4497
Conversation
There was a problem hiding this comment.
Great job on the Pull Request! There are a few minor suggestions regarding class naming and including fallback fonts, which aren't critical issues but can improve the code quality. Keep it up and consider these tips for future work! 😊👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| data-qa="hover" | ||
| placeholder="Try “Los Angeles“" | ||
| /> | ||
| <body class="position"> |
There was a problem hiding this comment.
Use semantic tags where possible. The class name 'position' does not indicate the purpose of the content. Consider using a more descriptive class name or semantic HTML5 elements to improve SEO and accessibility.
| <form | ||
| action="#" | ||
| data-qa="big" | ||
| class="form form__big" |
There was a problem hiding this comment.
The class name 'form__big' seems to represent the style or size of the form rather than its content. Class names should represent the meaning of the content, not the styles or tag names.
| <input | ||
| type="text" | ||
| data-qa="keypress" | ||
| class="big__input" | ||
| placeholder="Try “Los Angeles“" |
There was a problem hiding this comment.
The class name 'big__input' suggests a style or size which is not ideal. Class names should describe the content's meaning and not its appearance.
| <form | ||
| action="#" | ||
| data-qa="small" | ||
| class="form form__small" |
There was a problem hiding this comment.
The class name 'form__small' has the same issue as 'form__big' - it represents style rather than content. Consider renaming it to reflect the purpose or content of the form.
| <input | ||
| type="text" | ||
| class="small__input" | ||
| placeholder="Try “Los Angeles“" |
There was a problem hiding this comment.
Similar to 'big__input', the class 'small__input' should be renamed to better represent the content's meaning rather than its style.
| border: 1px solid rgba(225, 231, 237, 1); | ||
| box-shadow: 0 1 8 0 rgba(61, 78, 97, 0.1); | ||
| border-radius: 4px; | ||
| font-family: Avenir, 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. You should include a generic font family as a fallback. For example: font-family: Avenir, Arial, sans-serif;.
| background-size: 11px; | ||
| border: 1px solid rgba(225, 231, 237, 1); | ||
| box-shadow: 0 1px 8px 0 rgba(61, 78, 97, 0.1); | ||
| font-family: Avenir, sans-serif; |
There was a problem hiding this comment.
As with the previous font-family declaration, remember to include a generic fallback font. For example: font-family: Avenir, Arial, sans-serif;.
No description provided.