add task solution#5409
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still violations of explicit checklist requirements, which are treated as critical blockers for this task. In src/style.css and related CSS you use the universal selector * and tag selectors like form, input, and searchbar input for styling, which violates the rules “Don’t use * selector” and “Don’t use tag names for styling (except html and body)” [CHECKLIST ITEM #4, #5]; these selectors need to be refactored to class-based selectors (e.g., .searchbar__form, .searchbar__input) and the HTML updated to match. Both form elements in src/index.html are missing action and method attributes, which the checklist describes as “inputs inside form with correctly passed attributes” — add appropriate values such as action="#" and method="get" so forms are structurally correct. The linter also fails on src/index.html because the placeholder attributes at lines 38 and 55 are not double-quoted; update those attributes to use double quotes to satisfy the HTML lint rule. Additionally, the extra data-qa="hover" on a div is not required by the spec and should be removed to avoid confusion, and you should verify that the search icon path ./images/Search.svg matches your actual src/images structure.
Once you replace the disallowed selectors, add proper form attributes, and fix the placeholder quoting (plus the minor data-qa cleanup and path verification), your implementation should fully satisfy the task and pass linting. You’ve already covered the main functional requirements well (two search bars, background image icon, correct data-qa on big/small forms and keypress input, font-weight 300), so you’re very close; these changes will bring your solution fully in line with the project constraints.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| user-scalable=no, | ||
| initial-scale=1.0, | ||
| maximum-scale=1.0, | ||
| minimum-scale=1.0" | ||
| /> |
There was a problem hiding this comment.
The stylesheet uses the universal selector * which violates the checklist item: "Don't use * selector (it impacts performance)". Remove * { ... } and target elements/classes specifically (e.g., set box-sizing on html, body or use a .root container).
| <div class="searchbar__glass large"></div> | ||
| <input | ||
| class="searchInput searchInput--large" |
There was a problem hiding this comment.
The CSS and HTML use tag selectors (form, input) for styling. The checklist requires: "Don't use tag names for styling (except html and body)". Move these rules to class-based selectors (e.g., .search-form, .search-input) or scope via the existing classes.
| <div class="searchbar__div"> | ||
| <div class="searchbar__glass large"></div> | ||
| <input | ||
| class="searchInput searchInput--large" | ||
| type="text" | ||
| data-qa="keypress" | ||
| placeholder='Try "Los Angeles"' | ||
| /> |
There was a problem hiding this comment.
Checklist item: "Icon implemented using background-image CSS property" — the icon uses background: url('./images/Search.svg') which is correct, but the path uses ./images/Search.svg. The requirement says use images from src/images. If CSS is in src/ and images are in src/images, the path should be ./images/Search.svg only if served accordingly. Confirm project structure; if CSS is in src/ this is fine. No change if path matches project layout.
| <form | ||
| class="searchbar searchbar--large" | ||
| data-qa="big" | ||
| > |
There was a problem hiding this comment.
Checklist item: "Inputs are written inside of 'form' tag with correctly passed attributes" — inputs are inside form elements and the big input has data-qa="keypress". However, the forms lack action and method attributes. The checklist phrasing expects "correctly passed attributes" — add at least action="#" and method="get" (or appropriate) to each form to ensure attributes are present.
| <div | ||
| class="searchbar__div" | ||
| data-qa="hover" | ||
| > |
There was a problem hiding this comment.
HTML uses data-qa="hover" on a div — data-qa attributes are required only for big (data-qa="big"), small (data-qa="small") forms and data-qa="keypress" on big input. Having extra data-qa with value hover is unnecessary and may violate the spec. Remove or adjust if not required by task.
| border-radius: 4px; | ||
| box-sizing: border-box; | ||
| max-width: 1008px; | ||
| width: 100%; |
There was a problem hiding this comment.
The CSS uses the universal selector * on line 8 which violates checklist item #4: "Don't use * selector". Remove or replace it with a class-based rule (e.g., apply box-sizing to html or specific container classes).
| background: url('./images/Search.svg') no-repeat; | ||
| background-position: -1px -1px; | ||
| background-size: contain; | ||
| margin-right: 17px; | ||
| margin-left: 26px; |
There was a problem hiding this comment.
Multiple rules target tag names for styling: form (line 33) and input (line 37). This violates checklist item #5: "Don't use tag names for styling (except html and body)". Use class selectors instead (e.g., .search-form, .search-input).
| .searchbar { | ||
| margin: 20px auto 0; | ||
| border: 1px solid #e1e7ed; | ||
| box-shadow: 0 1px 8px 0 #3d4e611a; | ||
| border-radius: 4px; |
There was a problem hiding this comment.
The @font-face font-family name is Avenir but there's no fallback font-family declaration for the font file itself; however more importantly, the project requires using @font-face (satisfied) and using fallback fonts for font-family. In style declarations (body and input) you do include fallbacks, but the font-face uses the real font name without quotes — it's OK. No direct violation here.
| max-width: 704px; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Default font-weight must be 300 according to requirements — you set font-weight: 300 on body (line 16) which satisfies the requirement. No change needed.
| } | ||
|
|
||
| .searchbar__glass { | ||
| background: url('./images/Search.svg') no-repeat; |
There was a problem hiding this comment.
Icons must be implemented using background-image CSS property. You use background: url('./images/Search.svg') no-repeat; in searchbar.css (line 33) which satisfies checklist item #1. No change needed.
| .searchbar--small { | ||
| height: 42px; | ||
| } | ||
|
|
||
| .searchbar__glass { | ||
| background: url('./images/Search.svg') no-repeat; | ||
| background-position: -1px -1px; | ||
| background-size: contain; | ||
| margin-right: 17px; | ||
| margin-left: 26px; | ||
| color: #687c94; | ||
| } | ||
|
|
||
| .searchbar__glass.large { |
There was a problem hiding this comment.
Inputs must be inside form with correctly passed attributes (checklist item #2). You have both inputs wrapped in <form> elements and added required data-qa attributes (data-qa="big" on the large form and data-qa="small" on the small form; data-qa="keypress" is present on the big input). This satisfies the checklist item. No change needed.
| background: linear-gradient(180deg, #fff 0%, #f6f6f7 100%); | ||
| } | ||
|
|
||
| .searchbar input { |
There was a problem hiding this comment.
The HTML uses tag selectors in CSS via searchbar input rules in searchbar.css (line 61) — this is also styling by tag within a class but the selector includes a tag input. The checklist forbids using tag names for styling (except html/body). This violates checklist item #5. Change selectors to class-based (e.g., .searchbar__input).
| width: 11px; | ||
| height: 11px; | ||
| margin: 0 9px 0 13px; |
There was a problem hiding this comment.
The HTML includes an extra data attribute data-qa="hover" on a div (line 49). The requirements mention semantic tags when possible; using div here is acceptable. No violation detected for semantics, but consider using meaningful class names rather than searchbar__div if desired.
| width: 100%; | ||
| } | ||
|
|
||
| @media (max-width: 720px) { | ||
| .searchbar { | ||
| max-width: 704px; | ||
| } | ||
| } | ||
|
|
||
| .searchbar__div { | ||
| width: 100%; | ||
| height: 100%; | ||
| display: flex; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .searchbar--large { | ||
| height: 70px; | ||
| } | ||
|
|
||
| .searchbar--small { | ||
| height: 42px; | ||
| } | ||
|
|
||
| .searchbar__glass { | ||
| background: url('./images/Search.svg') no-repeat; | ||
| background-position: -1px -1px; | ||
| background-size: contain; | ||
| margin-right: 17px; | ||
| margin-left: 26px; | ||
| color: #687c94; | ||
| } | ||
|
|
||
| .searchbar__glass.large { | ||
| width: 19px; | ||
| height: 19px; | ||
| } | ||
|
|
||
| .searchbar__glass.small { | ||
| width: 11px; | ||
| height: 11px; | ||
| margin: 0 9px 0 13px; | ||
| } | ||
|
|
||
| .searchbar:hover { |
| * { | ||
| box-sizing: border-box; | ||
| margin: 0; | ||
| padding: 0; | ||
| } |
There was a problem hiding this comment.
This violates checklist item #4: "Don't use * selector" — the rule is violated at style.css line 8 where * { ... } is used. Replace global selector with targeted classes or reset on html/body only.
| form { | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
This violates checklist item #5: "Don't use tag names for styling (except html and body)" — several rules target tag selectors: form at style.css line 33 and input at line 37. Use classes or attribute selectors instead.
| @font-face { | ||
| font-family: Avenir; | ||
| src: url('./fonts/Avenir.ttc') format('ttc'); | ||
| font-weight: 300; | ||
| font-style: normal; | ||
| } |
There was a problem hiding this comment.
This violates checklist item #3: "Remember to use fallback fonts - alternative font-family in case the main one doesn't work." — while font-family lists fallbacks in multiple places, the @font-face declared font-family name Avenir may conflict; ensure fallback is present where Avenir is used and that @font-face provides proper formats (woff/woff2). Also the src uses format('ttc') which is uncommon and may not load; provide woff/woff2 for broader support.
| flex-direction: column; | ||
| } /* add styles here */ | ||
|
|
||
| form { |
There was a problem hiding this comment.
Checklist item #1: "Icon implemented using background-image CSS property" requires using images from src/images. The CSS uses background: url('./images/Search.svg') which is correct path-wise, but ensure the image exists at that path. If the file is missing, add it. (Checked at searchbar.css line 33)
| 'Segoe UI Symbol'; | ||
| display: flex; | ||
| flex-direction: column; | ||
| } /* add styles here */ | ||
|
|
||
| form { | ||
| width: 100%; | ||
| } | ||
|
|
||
| input { | ||
| outline: none; | ||
| box-shadow: none; | ||
| font-family: | ||
| Avenir, | ||
| -apple-system, | ||
| BlinkMacSystemFont, | ||
| 'Segoe UI', | ||
| Roboto, | ||
| Helvetica, |
There was a problem hiding this comment.
This violates checklist item #2: "Inputs are written inside of 'form' tag with correctly passed attributes" — the inputs are inside form tags, but form elements lack action and method attributes. The checklist asks for inputs inside form with correctly passed attributes; add appropriate action and method attributes to both forms (e.g., action="/" method="get"). See index.html lines 28-31 and 43-46.
| * { | ||
| box-sizing: border-box; | ||
| margin: 0; | ||
| padding: 0; | ||
| } |
There was a problem hiding this comment.
This violates requirement: "Don't use * selector." (duplicate emphasis) The global reset should be removed — use html, body or specific class resets instead (style.css lines 8-12).
| width: 100%; | ||
| } | ||
|
|
||
| input { | ||
| outline: none; | ||
| box-shadow: none; |
There was a problem hiding this comment.
This violates checklist/code-style: "Don't use spaces in links" — in index.html the placeholder attribute uses double quotes with inner double quotes (placeholder='Try "Los Angeles"') which is acceptable, but ensure anchor hrefs (none present) have no spaces. No direct violation found otherwise.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation still contains a critical violation of checklist items #4 and #5 in src/searchbar.css. Lines 69, 82, and 87 use tag selectors (.searchbar input, .searchbar input:focus, .searchbar input::placeholder) which combine a class with the input tag name. Per the explicit checklist rule "Don't use tag names for styling," these must be refactored to class-based selectors. Your inputs already have appropriate classes (searchInput, searchInput--large, searchInput--small), so update the CSS to use .searchInput, .searchInput:focus, and .searchInput::placeholder instead of descendant selectors with tag names. This is a blocker that prevents the linter from passing. The visual regression test differences (1.27-2.37%) are marginal and likely due to rendering variations rather than actual styling issues, so those don't require code changes on your end.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| box-shadow: 0 3px 8px 0 #3d4e6133; | ||
| background: linear-gradient(180deg, #fff 0%, #f6f6f7 100%); | ||
| } | ||
|
|
There was a problem hiding this comment.
This violates checklist item #5: 'Don't use tag names for styling'. Replace .searchbar input with a class-based selector like .searchbar__input and update the HTML to use this class on the input elements.
| padding: 0; | ||
| } | ||
|
|
||
| .searchbar input:focus { |
There was a problem hiding this comment.
Same tag selector violation - uses input tag name. Apply the same class-based selector fix mentioned for line 69.
| outline: none; | ||
| font-weight: 900; | ||
| } | ||
|
|
There was a problem hiding this comment.
Same tag selector violation - uses input tag name. Apply the same class-based selector fix mentioned for line 69.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved with an educational note to help you progress. You've done an excellent job refactoring all tag-based selectors to class-based selectors in src/searchbar.css, which resolved the previous checklist violations [CHECKLIST ITEMS #4, #5]. The HTML structure with forms, data attributes, and the icon implementation using background-image are all correct. However, there's one spacing issue remaining: the distance between the two search bars is 0px, but the requirement specifies a 20px gap. In src/style.css, the .searchbar--large class has margin: 20px auto 0 with no bottom margin. Adding margin-bottom: 20px to this class will create the required gap between the search bars, addressing checklist item "Be consistent with margins." The visual regression test failures are environment-related (font rendering differences in CI) and not code issues—these can be resolved by regenerating reference screenshots. Keep up the good work on the clean CSS refactoring!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| 'Segoe UI Symbol'; | ||
| display: flex; | ||
| flex-direction: column; |
There was a problem hiding this comment.
Add margin-bottom: 20px to create the required 20px gap between search bars. Currently, the first search bar has margin: 20px auto 0 with no bottom margin, resulting in 0px gap between the two forms.
Anton-Kuchmasov
left a comment
Anton-Kuchmasov
left a comment
There was a problem hiding this comment.
Well done!
But please - in futher tasks, don't forget to deploy last version of your solution before opening a new PR
3 participants
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 MistakesfromBEMlesson theory are checked.Code follows all the Code Style Rules ❗️