commit message #4613

Humenyuk1991 · 2024-11-25T20:08:07Z

mateacademy-ai-mentor

Thank you for your hard work on this project! 😊 While you've made a great effort, there are a few critical issues that need to be addressed to meet the core requirements of the task:

Font Usage: The task specifies using @font-face for fonts, but the current implementation uses Google Fonts for Roboto. This needs to be corrected to align with the task requirements.
HTML Structure: The data-qa attribute is incorrectly set. It should be keypress for the input in the big search form, but it's currently set to hover in the small search form.
CSS Class Naming: The class names search-input--big and search-input--small do not follow BEM naming conventions, which is a requirement.
Image Usage: The task specifies using images from src/images, but an external image URL is used for the background image. This needs to be corrected.
File Structure: Incorrect file paths for fonts and images may cause loading issues. Ensure that these resources are correctly referenced.

These issues are critical as they affect the core requirements and functionality of the task. Please address these points, and feel free to reach out if you need any help. Keep up the great work, and I'm confident you'll nail it! 🚀

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2024-11-25T20:09:29Z

src/style.css

+  box-sizing: border-box;
+  transition: all 0.3s ease;
+  box-shadow: 0 4px 4px 0 #00000040;
+  gap: 20px;


The gap property is used here, but it doesn't apply to non-flex or non-grid containers. Consider removing it or applying it to a parent element that uses flex or grid layout.

mateacademy-ai-mentor · 2024-11-25T20:09:29Z

src/style.css

+  padding-bottom: 26px;
+  width: 100%;
+  height: 70px;
+  top: 71px;


The top and left properties are used here, but they will only have an effect if the element is positioned (e.g., relative, absolute, fixed). Ensure that the .search-input--big class or its parent has a positioning context.

mateacademy-ai-mentor · 2024-11-25T20:09:30Z

src/style.css

+  margin-top: 20px;
+  width: 100%;
+  height: 42px;
+  top: 403px;


Similar to the previous comment, the top and left properties require a positioning context. Ensure that the .search-input--small class or its parent is positioned.

mateacademy-ai-mentor · 2024-11-25T20:09:30Z

src/style.css

+  top: 283px;
+  left: 48px;
+  gap: 20px;
+  opacity: 1px;


The opacity property should not have a unit. It should be a value between 0 and 1. Consider changing opacity: 1px; to opacity: 1;.

mateacademy-ai-mentor

Review Summary

Great effort on your submission! 🎉 While your code is on the right track, there are a few critical issues that need to be addressed to fully meet the task requirements:

Font Usage:
- The Roboto font should be included using @font-face, similar to Avenir. Ensure all fonts are defined this way.
- The src attribute in the @font-face rule should use a relative path to the font file within the project directory.
HTML Structure:
- The data-qa attribute for the small search form is incorrectly set to hover. It should be small.
File Paths:
- Ensure all file paths, including those for fonts and stylesheets, are relative and correctly point to the files within your project directory.
- The background-image URL in your CSS should also be a relative path.
- The href attribute for the preconnect link should be a URL to a domain, not a local path.
CSS Class Naming:
- Your use of BEM naming conventions for class names like search__input--big and search__input--small is correct. Keep this consistency across all class names.

These adjustments are crucial for the functionality and deployment of your project. Addressing these issues will enhance your code quality and ensure it meets the task requirements. Keep up the good work and continue learning from these experiences. Your progress is important, and tackling these challenges will only make you a better developer. 🚀

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

mateacademy-ai-mentor · 2024-11-25T20:33:02Z

src/index.html

@@ -1,6 +1,19 @@
 <!doctype html>
 <html lang="en">
  <head>
+    <link
+      rel="preconnect"
+      href="D:\projects\layout_search-bar-airbnb\src\fonts"


The href attribute for the preconnect link is pointing to a local file path. This should be a URL to a domain you want to preconnect to, not a local path.

mateacademy-ai-mentor · 2024-11-25T20:33:02Z

src/index.html

+    />
+    <link
+      rel="preconnect"
+      href="D:\projects\layout_search-bar-airbnb\src\fonts"


The href attribute for the preconnect link is pointing to a local file path. This should be a URL to a domain you want to preconnect to, not a local path.

mateacademy-ai-mentor · 2024-11-25T20:33:02Z

src/index.html

+      crossorigin="anonymous"
+    />
+    <link
+      href="D:\projects\layout_search-bar-airbnb\src\fonts"


The href attribute for the stylesheet link is pointing to a local file path. Ensure that the path is correct and accessible from the HTML file.

mateacademy-ai-mentor · 2024-11-25T20:33:02Z

src/index.html

@@ -16,17 +29,31 @@
      href="style.css"


The href attribute for the stylesheet link should be a relative path to the CSS file. Ensure that the path is correct and accessible from the HTML file.

mateacademy-ai-mentor · 2024-11-25T20:33:03Z

src/index.html

+    <div class="search-panel">
+      <form
+        class="search-form big"
+        data-qa="keypress"


The data-qa attribute is correctly set to keypress for the big search form, which aligns with the task requirements.

mateacademy-ai-mentor · 2024-11-25T20:33:03Z

src/index.html

+        data-qa="small"
+      >
+        <input
+          class="search__input search__input--small"


The class name search__input--small follows BEM naming conventions, which is correct. Ensure consistency across all class names.

mateacademy-ai-mentor · 2024-11-25T20:33:03Z

src/style.css

-/* add styles here */
+@font-face {
+  font-family: Avenir;
+  src: url(fonts/Avenir.ttc);


The src attribute in the @font-face rule should use a relative path to the font file within the project directory. Ensure that the path is correct and accessible from the CSS file.

mateacademy-ai-mentor · 2024-11-25T20:33:03Z

src/style.css

+}
+
+.body {
+  font-family: Roboto, serif;


The font-family Roboto is used here, but according to the task requirements, it should be included using @font-face. Ensure that you define Roboto using @font-face if it's not already included.

mateacademy-ai-mentor · 2024-11-25T20:33:03Z

src/style.css

+}
+
+.search__input--big {
+  background-image: url(images/Search.svg);


The background-image URL should be a relative path to the image within the project directory. Ensure that the path is correct and accessible from the CSS file.

mateacademy-ai-mentor · 2024-11-25T20:33:03Z

src/style.css

+}
+
+.search__input--small {
+  background-image: url(images/Search.svg);


The background-image URL should be a relative path to the image within the project directory. Ensure that the path is correct and accessible from the CSS file.

SanyaBratashchuk

all tests are failed. Try to pass them if you have any difficulties do not hesitate to ask in chat

TarasHoliuk · 2024-11-28T18:43:50Z

Now tests result seems outdated:

Page:

Re-run tests again and redeploy (it will also update test report link).

TarasHoliuk

GJ 👍
A few more suggestions to improve your code

Also, redeploy tests results

TarasHoliuk · 2024-11-28T18:45:46Z

src/style.css

+  padding-top: 25px;
+  padding-left: 62px;
+  padding-bottom: 26px;


Do you need top and bottom paddings? Should work without them

TarasHoliuk · 2024-11-28T18:46:09Z

src/style.css

+  padding-top: 12px;
+  font-size: 14px;
+  padding-left: 34px;
+  font-weight: 300px;


Suggested change

font-weight: 300px;

font-weight: 300;

px is wrong unit for font-weight

TarasHoliuk · 2024-11-28T18:47:45Z

src/style.css

+  width: 100%;
+  gap: 20px;
+  opacity: 1;


Do you need it here? Seem redundant

TarasHoliuk · 2024-11-28T18:48:30Z

src/style.css

+.search__input:focus {
+  background-image: url(images/Search.svg),
+  linear-gradient(180deg, #FFF 0%, #F6F6F7 100%);
+  border: 1px solid #E1E7ED;


you've set it by default - redundant here and in :hover

volodymyr-soltys97

Great work! 🔥

volodymyr-soltys97 · 2024-11-29T06:53:48Z

src/index.html

-      placeholder="Try “Los Angeles“"
-    />
+  <body class="body">
+    <div class="search-panel">


Redundant wrapper

Suggested change

<div class="search-panel">

commit message

b348253

mateacademy-ai-mentor suggested changes Nov 25, 2024

View reviewed changes

commit message

120bb5f

Humenyuk1991 requested a review from mateacademy-ai-mentor November 25, 2024 20:31

mateacademy-ai-mentor approved these changes Nov 25, 2024

View reviewed changes

SanyaBratashchuk suggested changes Nov 25, 2024

View reviewed changes

Humenyuk1991 added 2 commits November 28, 2024 17:16

commit message

c41aaa3

commit message

409537b

Humenyuk1991 requested a review from mateacademy-ai-mentor November 28, 2024 15:22

Humenyuk1991 added 4 commits November 28, 2024 19:08

commit message

c583426

commit message

75fb9aa

commit message

608dbc8

commit message

2ca00aa

TarasHoliuk suggested changes Nov 28, 2024

View reviewed changes

Humenyuk1991 added 6 commits November 28, 2024 21:01

commit message

ca25ff4

commit message

384cc5a

commit message

77efde6

commit message

d31bcb6

commit message

f612cd7

commit message

b30f166

Humenyuk1991 requested a review from TarasHoliuk November 28, 2024 21:20

volodymyr-soltys97 approved these changes Nov 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

commit message #4613

commit message #4613

Humenyuk1991 commented Nov 25, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

mateacademy-ai-mentor Nov 25, 2024

SanyaBratashchuk left a comment

TarasHoliuk commented Nov 28, 2024

TarasHoliuk left a comment

TarasHoliuk Nov 28, 2024

TarasHoliuk Nov 28, 2024

TarasHoliuk Nov 28, 2024

TarasHoliuk Nov 28, 2024

volodymyr-soltys97 left a comment

volodymyr-soltys97 Nov 29, 2024

commit message #4613

Are you sure you want to change the base?

commit message #4613

Conversation

Humenyuk1991 commented Nov 25, 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

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

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

Choose a reason for hiding this comment

SanyaBratashchuk left a comment

Choose a reason for hiding this comment

TarasHoliuk commented Nov 28, 2024

TarasHoliuk 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

volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment