add solution #4740
add solution #4740
Conversation
MartaKurdylo
commented
Jan 30, 2025
MartaKurdylo
commented
- DEMO LINK
- TEST REPORT LINK
- Icon implemented using background-image CSS property
- Inputs are written inside of 'form' tag with correctly passed attributes
- All Typical Mistakes from BEMlesson theory are checked.
- Code follows all the Code Style Rules (https://github.com/MartaKurdylo/layout_search-bar-airbnb/blob/master/checklist.md)
There was a problem hiding this comment.
Thank you for your hard work on this task! 😊 While your solution meets many of the requirements, there are a few critical issues that need to be addressed before we can approve the pull request:
-
Font Files: Ensure that the font files specified in the
@font-facerules are correctly placed in thefontsdirectory. This is crucial for the fonts to load properly. -
Search Icon: Verify that the path to the
Search.svgimage is correct and that the image exists in the specified directory. This will ensure the search icon appears as expected. -
Placeholder Text: The placeholder text for the input fields should be distinct if they are meant to serve different purposes. Consider differentiating the placeholder text to reflect the intended use of each search bar.
-
Data-QA Attributes: The task description requires the implementation of two search bars with specific data-qa attributes for testing purposes. Ensure that these attributes are correctly set and used in your testing framework.
Please address these issues to ensure full compliance with the task requirements. Once these are resolved, we can proceed with approval. Keep up the great work, and don't hesitate to reach out if you need any help! 🚀
✨ 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.
| <div class="search-container"> | ||
| <form | ||
| class="search-bar search--big" | ||
| data-qa="big" | ||
| > | ||
| <input | ||
| type="text" | ||
| data-qa="keypress" | ||
| placeholder="Try “Los Angeles“" | ||
| /> | ||
| </form> | ||
|
|
||
| <input | ||
| type="text" | ||
| data-qa="hover" | ||
| placeholder="Try “Los Angeles“" | ||
| /> | ||
| <form | ||
| class="search-bar search--small" | ||
| data-qa="small" | ||
| > | ||
| <input | ||
| type="text" | ||
| data-qa="hover" | ||
| placeholder="Try “Los Angeles“" | ||
| /> | ||
| </form> | ||
| </div> |
There was a problem hiding this comment.
The task description requires the implementation of two search bars with specific data-qa attributes for testing purposes. Ensure that these attributes are correctly set and used in your testing framework.
| <input | ||
| type="text" | ||
| data-qa="keypress" | ||
| placeholder="Try “Los Angeles“" | ||
| /> |
There was a problem hiding this comment.
The placeholder text for the input fields should be distinct if they are meant to serve different purposes. Consider differentiating the placeholder text to reflect the intended use of each search bar.
| <input | ||
| type="text" | ||
| data-qa="hover" | ||
| placeholder="Try “Los Angeles“" | ||
| /> |
There was a problem hiding this comment.
Similar to the previous comment, ensure that the placeholder text is distinct if the search bars are intended for different functionalities.
src/style.css
Outdated
| @font-face { | ||
| font-family: Avenir; | ||
| src: url(fonts/Avenir-Book.ttf); | ||
| font-weight: 400; | ||
| font-style: normal; | ||
| } | ||
|
|
||
| @font-face { | ||
| font-family: Avenir; | ||
| src: url(fonts/Avenir-Heavy.ttf); | ||
| font-weight: 900; | ||
| font-style: normal; | ||
| } | ||
|
|
||
| @font-face { | ||
| font-family: Avenir; | ||
| src: url(fonts/Avenir.ttc); | ||
| font-weight: 300; | ||
| font-style: normal; | ||
| } |
There was a problem hiding this comment.
Ensure that the font files specified in the @font-face rules are correctly placed in the fonts directory. This is crucial for the fonts to load properly.
src/style.css
Outdated
|
|
||
| .search-bar::before { | ||
| content: ''; | ||
| background-image: url('../src/images/Search.svg'); |
There was a problem hiding this comment.
Verify that the path to the Search.svg image is correct and that the image exists in the specified directory. This will ensure the search icon appears as expected.
| .search-bar input { | ||
| flex: 1; | ||
| border: none; | ||
| outline: none; | ||
| background-color: #fff; | ||
| padding-left: 50px; | ||
| } |
There was a problem hiding this comment.
The padding-left for the input fields is set to accommodate the search icon. Ensure that this padding aligns well with the icon's position for both search bar sizes.
There was a problem hiding this comment.
Great job on addressing some of the previous issues! 🎉 While there are still a few points to improve, such as ensuring the font files (Avenir-Book.ttf, Avenir-Heavy.ttf, Avenir.ttc) are correctly placed in the fonts directory and verifying the path to the Search.svg image, your progress is commendable. Additionally, make sure the placeholder text for the search bars is distinct to meet the task requirements. Remember, these adjustments are crucial for the functionality and user experience of your project. Keep up the good work, and use this opportunity to refine your skills further! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <input | ||
| type="text" | ||
| data-qa="keypress" | ||
| placeholder="Try “Los Angeles“" |
There was a problem hiding this comment.
The placeholder text for this input field is the same as the one in the other search bar. Consider differentiating the placeholder text to reflect the intended use of each search bar, as per the task requirements.
| <input | ||
| type="text" | ||
| data-qa="hover" | ||
| placeholder="Try “Los Angeles“" |
There was a problem hiding this comment.
The placeholder text for this input field is identical to the one in the other search bar. Ensure that the placeholder text is distinct for each search bar to meet the task requirements.
| /* add styles here */ | ||
| @font-face { | ||
| font-family: Avenir; | ||
| src: url('fonts/Avenir-Book.ttf'); |
There was a problem hiding this comment.
Ensure that the 'Avenir-Book.ttf' font file is correctly placed in the 'fonts' directory. This is crucial for the font to load properly.
|
|
||
| @font-face { | ||
| font-family: Avenir; | ||
| src: url('fonts/Avenir-Heavy.ttf'); |
There was a problem hiding this comment.
Ensure that the 'Avenir-Heavy.ttf' font file is correctly placed in the 'fonts' directory. This is crucial for the font to load properly.
|
|
||
| @font-face { | ||
| font-family: Avenir; | ||
| src: url('fonts/Avenir.ttc'); |
There was a problem hiding this comment.
Ensure that the 'Avenir.ttc' font file is correctly placed in the 'fonts' directory. This is crucial for the font to load properly.
|
|
||
| .search-bar::before { | ||
| content: ''; | ||
| background-image: url('images/Search.svg'); |
There was a problem hiding this comment.
Verify that the path to the 'Search.svg' image is correct and that the image exists in the 'images' directory. This will ensure the search icon appears as expected.
