add task solution #4549
add task solution #4549
Conversation
There was a problem hiding this comment.
Great job on implementing the custom fonts and using BEM naming conventions! 🎉 While there are some minor improvements suggested regarding the use of fallback fonts and class naming, they don't impact the core functionality. Keep up the good work and consider these suggestions for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/index.html
Outdated
| <form | ||
| class="form form_big" | ||
| action="search" | ||
| data-qa="big" | ||
| > |
There was a problem hiding this comment.
The class name 'form_big__input' suggests a BEM naming convention, but it should be 'form__input form__input--big' to correctly represent the block and modifier structure.
src/index.html
Outdated
| <form | ||
| class="form form_small" | ||
| action="search" | ||
| data-qa="small" | ||
| > |
There was a problem hiding this comment.
The class name 'form_small__input' suggests a BEM naming convention, but it should be 'form__input form__input--small' to correctly represent the block and modifier structure.
src/style.css
Outdated
| @@ -1 +1,94 @@ | |||
| /* add styles here */ | |||
| @font-face { | |||
| font-family: Avenir, Arial, sans-serif; | |||
There was a problem hiding this comment.
The font-family property should not include multiple fonts separated by commas within the @font-face rule. Instead, specify only the custom font name, like Avenir, and use fallback fonts in the font-family property of other CSS selectors.
src/style.css
Outdated
| } | ||
|
|
||
| @font-face { | ||
| font-family: Avenir-Heavy, Arial, sans-serif; |
There was a problem hiding this comment.
Similar to line 2, the font-family property should only specify the custom font name Avenir-Heavy without fallback fonts. Fallbacks should be used in other CSS selectors.
| align-items: center; | ||
| width: 100%; | ||
| margin-top: 20px; | ||
| font-family: Avenir-Book, Arial, sans-serif; |
There was a problem hiding this comment.
The font-family property should use the custom font name defined in the @font-face rule without fallback fonts. Use Avenir instead of Avenir-Book.
| box-shadow: 0 2px 8px 0 rgba(61, 78, 97, 0.2); | ||
| background: linear-gradient(0deg, #f6f6f7, #fff); | ||
| text-shadow: 0 4px 4px rgba(0, 0, 0, 0.25); | ||
| font-family: Avenir-Heavy, Arial, sans-serif; |
There was a problem hiding this comment.
The font-family property should use the custom font name defined in the @font-face rule without fallback fonts. Use Avenir-Heavy instead of Avenir-Heavy, Arial, sans-serif.
Typical MistakesfromBEMlesson theory are checked.