add solution #4543

LiliiaVol · 2024-10-16T09:22:49Z

mateacademy-ai-mentor

Great job so far! 😊 However, there are a few areas that need to be corrected. Please fix the typo in the class names and avoid using the '*' selector due to its impact on performance. Also, consider using semantic HTML5 elements and a more descriptive document title. Don't forget to correct the indentation as well. Keep up the good work! 💪

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-10-16T09:23:18Z

src/index.html

- placeholder="Try “Los Angeles“"
- />
+ <form
+ class="search-form search-form_big"


Remember about correct indentation between parent and child elements. Each level of nesting, including text, contained inside the element, requires 2-space offset. But here, it seems like you used 4-space offset. Please fix it.

mateacademy-ai-mentor · 2024-10-16T09:23:18Z

src/index.html

- placeholder="Try “Los Angeles“"
- />
+ <form
+ class="search-form search-form_small"


Here again, the indentation is not correct. Please use 2-space offset for the nested elements.

mateacademy-ai-mentor · 2024-10-16T09:23:18Z

src/index.html

+ class="search-form__input search-form__input_small"
+ />
+ </label>
+ </form>
 </body>


The semantic structure of the HTML document could be improved. Consider using semantic HTML5 elements like <header>, <main>, <section>, <article>, <aside>, <footer>, etc. These elements provide additional information about your document to both the browser and the developer.

mateacademy-ai-mentor · 2024-10-16T09:23:19Z

src/style.css

+ margin: 0;
+ padding: 0;
+ box-sizing: border-box;
+}


You're using the '*' selector here. This is generally not recommended because it can impact performance. It would be better to apply these styles to the specific elements that need them.

mateacademy-ai-mentor · 2024-10-16T09:23:19Z

src/style.css

+
+body {
+ margin: 8px;
+ font-family: Avenir, sans-serif;


You've specified a font-family here, but you haven't provided a fallback font. If the Avenir font fails to load for some reason, the browser will use the default font, which may not fit your design. It's a good idea to specify a generic font family that the browser can fall back to. For example: font-family: Avenir, sans-serif;.

mateacademy-ai-mentor · 2024-10-16T09:23:19Z

src/style.css

+}
+
+* {
+ margin: 0;


It's not recommended to use the * selector to set styles for all elements, as it can negatively affect performance. Try to apply these styles specifically to the elements that need them.

mateacademy-ai-mentor · 2024-10-16T09:23:19Z

src/style.css

+ box-shadow: 0 3px 8px rgba(61, 78, 97, 0.2);
+}
+
+.input:focus {


It seems like there is a typo in your class name. You probably meant .search-form__input:focus instead of .input:focus.

mateacademy-ai-mentor · 2024-10-16T09:23:19Z

src/style.css

+ line-height: 19px;
+}
+
+.search-form_input:hover {


It seems like there is a typo in your class name. You probably meant .search-form__input:hover instead of .search-form_input:hover.

mateacademy-ai-mentor

Your code is really good, but there are some minor issues that need your attention. Let's fix them to make your code even better! 😊 Remember to avoid fixing the container size unless it's necessary, be consistent with your margins, use fallback fonts and avoid using the * selector. Keep up the good work! 👍

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-10-16T09:32:55Z

src/style.css

+* {
+ padding: 0;
+ box-sizing: border-box;
+}


According to the checklist, you should not use the * selector as it impacts performance. It's better to set styles only for elements that require them.

mateacademy-ai-mentor · 2024-10-16T09:32:55Z

src/style.css

+
+body {
+ margin: 8px;
+ font-family: Avenir, sans-serif;


Good job on defining the font family. However, remember to use fallback fonts - alternative font-family in case the main one doesn't work. This is to ensure that some sort of font is displayed to the user in case the first one doesn't load for some reason. For example, you could modify this line to: font-family: Avenir, Arial, sans-serif;

mateacademy-ai-mentor · 2024-10-16T09:32:55Z

src/style.css

+}
+
+body {
+ margin: 8px;


Be consistent with your margins. It's recommended to add margins only to the top or bottom of elements, not both, to avoid potential margin collapse. In this case, you're adding margin to all sides of the body element.

mateacademy-ai-mentor · 2024-10-16T09:32:55Z

src/style.css

+ width: 100%;
+ height: 70px;


You've fixed the size of the .search-form_big container. Unless it's a strict requirement, it's better not to fix the container size. This allows the content size to dictate the size of the container, helping to avoid overflow or accidental scroll bars.

mateacademy-ai-mentor · 2024-10-16T09:32:55Z

src/style.css

+ width: 100%;
+ height: 42px;


Similar to the .search-form_big class, you've also fixed the size of the .search-form_small container. Consider letting the content size dictate the size of the container.

mateacademy-ai-mentor

Great job on your pull request! 🎉 You've done well in structuring your HTML and CSS files. However, there are a few areas that need improvement. You should avoid fixing container heights and styling using tag names. Also, be consistent with your margins to avoid potential margin collapse. In your HTML, ensure that label elements have a for attribute for better accessibility, and that your title tag is more descriptive. These are not major issues, but addressing them will make your code cleaner and more efficient. Keep up the good work and happy coding! 💻

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-10-16T09:36:38Z

src/index.html

+ <form class="search-form search-form_big" data-qa="big">
+ <label class="search-form__label search-form__label_big">
+ <input
+ data-qa="keypress"
+ type="text"
+ placeholder="Try “Los Angeles”"
+ class="search-form__input search-form__input_big"
+ />
+ </label>
+ </form>


There should be an empty line between multiline sibling blocks of HTML. Please add an empty line between these two form elements. [HTML Formatting]

mateacademy-ai-mentor · 2024-10-16T09:36:38Z

src/index.html

+ <input
+ data-qa="keypress"
+ type="text"
+ placeholder="Try “Los Angeles”"
+ class="search-form__input search-form__input_big"
+ />


The label element should have a for attribute which refers to the id of the input it labels. This improves accessibility as it allows screen readers to read out the label when the user is focused on the input. [HTML Content]

mateacademy-ai-mentor · 2024-10-16T09:36:38Z

src/index.html

+ <input
+ data-qa="hover"
+ type="text"
+ placeholder="Try “Los Angeles”"
+ class="search-form__input search-form__input_small"
+ />


The label element should have a for attribute which refers to the id of the input it labels. This improves accessibility as it allows screen readers to read out the label when the user is focused on the input. [HTML Content]

mateacademy-ai-mentor · 2024-10-16T09:36:38Z

src/style.css

+body {
+ padding: 0;
+ box-sizing: border-box;
+ font-family: Avenir, Arial, sans-serif;


You've done a good job specifying a fallback font in case the main font doesn't load. This is a good practice as it ensures that some font will be displayed, improving your website's resilience and usability. Keep it up!

mateacademy-ai-mentor · 2024-10-16T09:36:38Z

src/style.css

+.search-form {
+ display: flex;
+ justify-content: center;
+ margin: 20px auto 0;


You are adding both top and bottom margins to this class. According to the checklist, it's better to be consistent with your margins and add either top or bottom, but not both, to avoid potential margin collapse.

mateacademy-ai-mentor · 2024-10-16T09:36:38Z

src/style.css

+
+.search-form_big {
+ width: 100%;
+ height: 70px;


Here you are fixing the height of the container. Unless there is a specific requirement to do so, it's better to let the content size dictate the container size. This helps to avoid overflow or accidental scroll bars.

mateacademy-ai-mentor · 2024-10-16T09:36:39Z

src/style.css

+
+.search-form_small {
+ width: 100%;
+ height: 42px;


The same issue here. You've fixed the height of the container. It's better to let the content size dictate the container size unless there is a specific requirement.

mateacademy-ai-mentor · 2024-10-16T09:36:39Z

src/style.css

+ width: 100%;
+ height: 100%;
+ border: 1px solid #e1e7ed;
+ border-radius: 4px;


Here you are using tag names for styling. According to the checklist, it's better to style all elements using .classes and if needed with :pseudo-classes, pseudo-elements and [attributes].

add solution

63fd72b

mateacademy-ai-mentor suggested changes Oct 16, 2024

View reviewed changes

add solution 2.0

cfdd98c

LiliiaVol requested a review from mateacademy-ai-mentor October 16, 2024 09:32

mateacademy-ai-mentor suggested changes Oct 16, 2024

View reviewed changes

add solution 3.0

d89fbf9

LiliiaVol requested a review from mateacademy-ai-mentor October 16, 2024 09:36

mateacademy-ai-mentor approved these changes Oct 16, 2024

View reviewed changes

add solution 4.0

6e1d715

LiliiaVol requested a review from mateacademy-ai-mentor October 16, 2024 09:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add solution #4543

add solution #4543

LiliiaVol commented Oct 16, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

mateacademy-ai-mentor Oct 16, 2024

add solution #4543

Are you sure you want to change the base?

add solution #4543

Conversation

LiliiaVol commented Oct 16, 2024

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment