Conversation
VovaShmeriga
commented
Apr 17, 2026
- DEMO LINK
- TEST REPORT LINK
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Changes are needed before this solution can be accepted, because several mandatory checklist items and test expectations are not met. Structurally, the header is missing a semantic <header> wrapper and the nav links are not inside a <ul>/<li> list, which breaks the requirement to use these specific semantic tags and also causes Backstop to fail when looking for header and list-based navigation [CHECKLIST ITEM #11]. The logo <img> is not wrapped in a link and its alt is too generic (alt="logo"); it must be an image from src/images wrapped in an <a> outside the <nav>, with a descriptive alt such as alt="MOYO logo" [CHECKLIST ITEMS #20, #32]. Your active link uses the class active instead of the required is-active, and the ::after rule targets .nav-link.active::after instead of .nav-link.is-active::after, so you need to rename the class in HTML and CSS and ensure the active link is position: relative [CHECKLIST ITEMS #21, #22]. The header height is currently set on the header container (e.g. .head { height: 60px; }); instead, move this height to the nav link rule so there is a single source of truth on the links and remove the fixed header height [CHECKLIST ITEMS #4, #19]. Presentation-wise, the link text is typed in uppercase in HTML (e.g. APPLE, SAMSUNG) rather than using text-transform: uppercase in CSS, there is no :hover color change for links, Roboto is used without adding the proper Google Fonts import for roman 500, and the required --color-blue CSS variable is not defined and reused [CHECKLIST ITEMS #7, #8, #10, #41]. Finally, the 4th link (Laptops & computers) is missing data-qa="hover", and some layout choices like justify-content: space-around in the nav lead to extra space before the first and after the last item, which conflicts with the requirement that list items not create outer margins [CHECKLIST ITEMS #23, #24]. Once you address these critical items in src/index.html and src/style.css, your header should both pass the automated tests and align with the specified design and checklist; the existing flex layout and basic structure are a good foundation to build on.
✨ 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
| <div class="head"> | ||
| <img src="images/logo.png" alt="logo" | ||
| class="logo"/> | ||
| <nav class="nav"> |
There was a problem hiding this comment.
This violates checklist item #11: "Use semantic tags specifically including <header>, <img>, <nav>, <ul>, <li> and <a>." You use <div class="head"> instead of a <header> and anchors are direct children of <nav> instead of being inside a <ul> with <li> elements. Replace the wrapper with a semantic <header> and structure nav links as a list.
| <a href="/" class="nav-link active">APPLE</a> | ||
| <a href="/" class="nav-link">SAMSUNG</a> | ||
| <a href="/" class="nav-link">SMARTPHONES</a> | ||
| <a href="/" class="nav-link">LAPTOPS & COMPUTERS</a> | ||
| <a href="/" class="nav-link">GADGETS</a> | ||
| <a href="/" class="nav-link">TABLETS</a> | ||
| <a href="/" class="nav-link">PHOTO</a> | ||
| <a href="/" class="nav-link">VIDEO</a> |
There was a problem hiding this comment.
This violates checklist item #8: "CSS must be used to show all letters in uppercase; uppercase letters must not be typed directly in HTML." The anchor texts are typed in uppercase in HTML (e.g. APPLE, SAMSUNG). Put the link text in normal case and use CSS text-transform: uppercase for presentation.
| <a href="/" class="nav-link active">APPLE</a> | ||
| <a href="/" class="nav-link">SAMSUNG</a> | ||
| <a href="/" class="nav-link">SMARTPHONES</a> | ||
| <a href="/" class="nav-link">LAPTOPS & COMPUTERS</a> |
There was a problem hiding this comment.
This violates checklist item #23: "Add data-qa="hover" attribute to the 4th link (Laptops & computers) for testing." The 4th anchor (Laptops & computers) does not have data-qa="hover".
| <img src="images/logo.png" alt="logo" | ||
| class="logo"/> |
There was a problem hiding this comment.
This violates checklist item #20: "Implement the logo as an image wrapped with a link, using an image from src/images, and ensure this logo link is not part of the <nav>." The <img> is not wrapped in an <a> link. Also this violates checklist item #32: "The alt attribute for images must describe the image content." alt="logo" is too generic — make it descriptive (for example: alt="MOYO logo").
| rel="stylesheet" | ||
| href="./style.css" | ||
| /> | ||
| <link rel="stylesheet" href="./style.css"/> |
There was a problem hiding this comment.
This violates checklist item #10: "Use the Roboto font... selecting ONLY roman style, medium (500) weight and normal width for embedding." There is no Google Fonts <link> (or @import) in the document head that loads Roboto with the required axes/weight. Add the proper Google Fonts import in the head and load only the specified style/weight.
|
|
||
| .head { | ||
| width: 100%; | ||
| background-color: #fff; |
There was a problem hiding this comment.
This violates checklist item #10: "Use the Roboto font, selecting only roman style, medium (500) weight and normal width for embedding." You set font-family: Roboto in CSS but you did not include the Google Fonts embed/config in the HTML head with the required selection. Add the correct Google Fonts <link> (roman, weight 500) in the head.
| align-items: center; | ||
| } | ||
|
|
||
| .nav-link { |
There was a problem hiding this comment.
This violates checklist item #7: "Link color must change on the :hover state." There is no :hover rule for .nav-link. Add a hover style (e.g. .nav-link:hover { color: var(--color-blue); }).
| width: 40px; | ||
| height: 40px; | ||
| align-self: center; | ||
| } |
There was a problem hiding this comment.
This violates checklist item #23: "Add data-qa="hover" attribute to the 4th link (Laptops & computers) for testing." The 4th link (LAPTOPS & COMPUTERS) is missing data-qa="hover" attribute — add it to that <a>.
| } | ||
|
|
||
| .nav-link.active { | ||
| color: #00ACDC; |
There was a problem hiding this comment.
This violates checklist item #41: "Use a CSS variable for the blue color." You use #00ACDC directly in multiple places (color: #00ACDC / background-color: #00ACDC). Define a CSS variable (for example --color-blue: #00ACDC;) and use it instead.
| color: #00ACDC; | ||
| } | ||
|
|
||
| .nav-link.active::after { |
There was a problem hiding this comment.
This violates checklist item #22: "Use the ::after pseudo-element and position it relative to the link with the is-active class to create the blue line below the active link." Your selector uses .nav-link.active::after; the task requires the is-active class name (.is-active) to be present and targeted. Update the HTML class and the selector to use .is-active (and ensure the link element is position: relative).
There was a problem hiding this comment.
what`s wrong in the fixed bugs commit?
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I’m requesting changes because there are still several critical checklist violations that will cause tests or visual checks to fail. You must remove all uses of the flex gap property (e.g., on the nav list in CSS and any inline styles in index.html) and replace the spacing with margins on list items, such as .nav-list li + li { margin-left: 20px; }, ensuring there is no extra space before the first or after the last item [CHECKLIST ITEM #X]. The body currently has background-color: lightblue (in both CSS and HTML), which breaks the pixel-perfect color requirement; please change it to the exact design color or remove the override so the design background is used. The nav links also need their text centered horizontally with no padding, so update .nav-link to center the text (for example using justify-content: center or text-align: center) while keeping padding: 0 as required. Several elements with three or more attributes (the main <img> and nav <a> tags, including the one with data-qa="hover") must be reformatted so each attribute starts on its own line with a 2-space indent and the closing > aligned with the tag name, following the HTML formatting rule [CHECKLIST ITEM #X]. Your npm test is failing due to stylelint in src/style.css at line 7, column 16 (unexpected quotes around the font family), so change font-family: "Roboto", sans-serif; to font-family: Roboto, sans-serif; and re-run the tests, and also make sure your PR description includes the required checklist with - [x] items. Once these points are addressed, your otherwise solid header implementation should fully meet the task requirements.
✨ 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
| margin: 0; | ||
| padding: 0; | ||
| display: flex; | ||
| gap: 20px; |
There was a problem hiding this comment.
This uses the flex gap property for spacing: "don't use the flex gap property for indents. It's not supported in tests, use margin instead". Replace gap with margins between list items (for example using .nav-list li + li { margin-left: 20px; }) and ensure there are no margins before the first and after the last item.
| text-decoration: none; | ||
| color: black; | ||
| text-transform: uppercase; | ||
| display: flex; |
There was a problem hiding this comment.
The nav links don't center their text horizontally. The checklist requires: "Nav Links should not have any padding but have the text centered." Add horizontal centering to the .nav-link (for example justify-content: center) while keeping padding at 0.
| margin: 0; | ||
| font-family: Roboto, sans-serif; | ||
| font-weight: 500; | ||
| background-color: lightblue; |
There was a problem hiding this comment.
The page must match the design Pixel Perfect: "all the sizes, colors and distances MUST be the same as on the design." Setting background-color: lightblue on body will most likely break pixel-perfect color requirements — remove or change it to match the design (use the design background color).
| <a data-qa="hover" href="/" | ||
| class="nav-link">Laptops & computers</a> | ||
| </li> | ||
| <li> |
There was a problem hiding this comment.
This uses the flex gap property which is forbidden by the task. The description explicitly says: "don't use the flex gap property for indents. It's not supported in tests, use margin instead" — replace gap with margins on the list items (and ensure you don't add outer margins before the first or after the last item).
| href="./style.css" | ||
| /> | ||
| <link href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap" rel="stylesheet"> | ||
| <link rel="stylesheet" href="./style.css"/> |
There was a problem hiding this comment.
The page must match the design Pixel Perfect: "all the sizes, colors and distances MUST be the same as on the design." Setting background-color: lightblue; on the body will likely fail visual tests. Remove or change this to the design color (or omit body background so the default/design background is used).
| <img src="images/logo.png" alt="MOYO logo" | ||
| class="logo"/> |
There was a problem hiding this comment.
HTML attributes formatting must follow the checklist: "If the HTML-element has long attribute values or number of attributes is more than 2 - start each one, including the first, on the new line with 2-space indentation related to tag. Tag’s closing bracket should be on the same level as opening one." The <img> has 3 attributes but attributes are not each started on their own indented line — please reformat this element (each attribute on its own line with 2-space indentation).
| <a data-qa="hover" | ||
| href="/" class="nav-link is-active">Apple</a> |
There was a problem hiding this comment.
Anchor elements with more than 2 attributes must follow the same formatting rule: "If the HTML-element has long attribute values or number of attributes is more than 2 - start each one, including the first, on the new line with 2-space indentation related to tag. Tag’s closing bracket should be on the same level as opening one." The first nav <a> has data-qa, href and class but attributes are not formatted as required — please reformat all such anchors.
| <a data-qa="hover" href="/" | ||
| class="nav-link">Samsung</a> |
There was a problem hiding this comment.
Another nav link showing the same attribute-formatting issue. Reformat anchors with 3+ attributes so each attribute starts on its own line (2-space indent) as required: "If the HTML-element has long attribute values or number of attributes is more than 2 - start each one..."
| <a data-qa="hover" href="/" | ||
| class="nav-link">Laptops & computers</a> |
There was a problem hiding this comment.
The 4th nav link (Laptops & computers) must have the test attribute data-qa="hover" (it is present — good) but its attributes also need to follow the formatting rule: "If the HTML-element has long attribute values or number of attributes is more than 2 - start each one..." Please reformat this anchor too.