Conversation
Ruslan170620
commented
Apr 17, 2026
Ruslan170620
commented
- DEMO LINK
- TEST REPORT LINK
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Changes are needed because several explicit checklist requirements are violated and these will block automated checks from passing.
Critical issues that must be fixed:
- The header is constrained with
max-width: 1200pxandmargin: 0 autoon.header, so it does not stretch full page width; remove these from the header (or move them to an inner wrapper) so the header itself spans 100% width [CHECKLIST ITEM #7, #42]. - You set
height: 60pxon.headerand then use.nav__link { height: 100%; }; instead, remove the fixed height from the header and set the explicit height (from the design) directly on.nav__link(ideally via a variable) so header height is defined in one place for the links [CHECKLIST ITEM #8, #22]. - You use the universal selector
* { ... }and tag-based selectors likeul { ... }anda { ... }; replace these with more specific selectors such ashtml, body { ... },.nav__list { ... }, and.nav__link { ... }, or use a:where()pattern for resets [CHECKLIST ITEM #38, #39]. - The blue color
#00acdcis hardcoded in multiple places; define a CSS variable (e.g.:root { --color-blue: #00acdc; }) and usevar(--color-blue)wherever that blue is needed [CHECKLIST ITEM #26]. - The logo image uses
alt="logo", which is too generic; change it to a descriptive text likealt="MOYO logo"or similar so it clearly describes the image content [CHECKLIST ITEM #35].
Once these issues are corrected, your solution should align well with the task requirements, since the overall structure, flexbox layout, active link behavior, and typography setup are already on the right track.
✨ 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
| max-width: 1200px; | ||
| margin: 0 auto; |
There was a problem hiding this comment.
This violates checklist item #7: "The header should stretch the full page width (don't use a horizontal margin)." and checklist item #42: "Don't fix container size (if there is no such a requirement)." You set a max-width and center the header with margin: 0 auto which constrains the header to a centered container instead of stretching the full page width. Remove the max-width and margin or move the max-width to an inner wrapper if needed.
| display: flex; | ||
| align-items: center; | ||
| justify-content: space-between; /* Лого зліва, nav справа */ | ||
| height: 60px; /* Було 64px */ |
There was a problem hiding this comment.
This violates checklist item #8: "The height should be set for nav links (not the header), take it from the design." and checklist item #22: "Header height is set in 1 place (for the links)" — you set the height on .header (60px). Instead set the explicit height on the nav links (e.g. .nav__link { height: 64px; }) and let the header size itself.
| @@ -1,3 +1,97 @@ | |||
| * { | |||
There was a problem hiding this comment.
This violates checklist item #38: "Don't use the * selector (it impacts performance)." You use * { ... } to reset styles. Use a safer, class/element-specific reset (for example html, body plus selective element resets) or the :where() pattern instead of the universal selector.
| ul { | ||
| margin: 0; | ||
| padding: 0; | ||
| list-style: none; |
There was a problem hiding this comment.
This violates checklist item #39: "Don't use tag names for styling (except html and body). Style all elements using .class ..." You are styling ul directly here; prefer .nav__list { ... } (class-based selectors).
| a { | ||
| text-decoration: none; |
There was a problem hiding this comment.
This violates checklist item #39 as well — you're styling a directly. Replace this with a class-based selector (for example .nav__link { text-decoration: none; }) to avoid tag-based styling.
| href="#" | ||
| class="logo" |
There was a problem hiding this comment.
This violates checklist item #39: "Don't use tag names for styling (except html and body)". Styling a directly is not allowed — use class-based selectors like .nav__link for link defaults instead.
| /> | ||
| </a> |
There was a problem hiding this comment.
| <ul class="nav__list"> | ||
| <li class="nav__item"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
This violates checklist item #8: "The height should be set for nav links (not the header), take it from the design." and checklist item #22: "Header height is set in 1 place (for the links)". You set height: 60px on .header; instead remove header height and set the required height (from the design) on the nav links (e.g. .nav__link) — ideally via a single CSS variable.
| class="nav__link" | ||
| > | ||
| Photo | ||
| </a> | ||
| </li> | ||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| Video | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| </nav> |
There was a problem hiding this comment.
This violates checklist item #26: "CSS Variable is used for a blue color." You hardcode #00acdc in several places. Define a CSS variable (e.g. --accent-blue: #00acdc;) and use var(--accent-blue) for link color and the active underline (color and background-color).
| > | ||
| <img | ||
| src="./images/logo.png" | ||
| alt="logo" |
There was a problem hiding this comment.
This violates checklist item #35: "alt attribute should describe the image content." alt="logo" is too generic; provide a descriptive alt (for example alt="MOYO logo" or alt="MOYO brand logo").
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Changes are needed before this can be accepted because several core checklist requirements are still not met and will cause automated checks to fail. The header height is currently set directly on .header (e.g. .header { height: 60px; } in src/style.css / index.html), but the requirement says the fixed height must be set only on the nav links, so you should remove the header height and instead define something like --nav-height: 60px; and apply height: var(--nav-height); on .nav__link. The blue color #00acdc is hardcoded in multiple rules (hover, .is-active, and the ::after underline); per the checklist you need a CSS variable such as :root { --color-blue: #00acdc; } and then use var(--color-blue) everywhere that blue is used. Your CSS still styles bare tags ul { ... } and a { ... }, which violates the “no tag selectors except html/body” rule, so please replace these with class-based selectors like .nav__list and .nav__link for margins, list reset, and text-decoration removal. The nav links correctly have padding: 0, but their text is not explicitly centered horizontally; add centering (for example display: flex; justify-content: center; align-items: center; on .nav__link, or text-align: center if they are block-level) so the text sits centered within each clickable area. Apart from these issues, the overall HTML structure, semantics, and many other requirements look good, so once you address these specific points you should be in a strong position to pass the automated checks.
✨ 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
| display: flex; | ||
| align-items: center; | ||
| justify-content: space-between; /* Лого зліва, nav справа */ | ||
| height: 60px; /* Було 64px */ |
There was a problem hiding this comment.
This violates checklist item "Header height is set in 1 place (for the links)". You set a fixed height on .header (height: 60px). Remove the height from the header and set the navigation height in one place (for example a CSS variable used on .nav__link, e.g. --nav-height: 60px; .nav__link { height: var(--nav-height); }).
| color: #00acdc; | ||
| } | ||
|
|
||
| .nav__link.is-active { | ||
| color: #00acdc; | ||
| } | ||
|
|
||
| .nav__link.is-active::after { | ||
| content: ''; | ||
| position: absolute; | ||
| left: 0; | ||
| right: 0; | ||
| bottom: 0; | ||
| height: 4px; | ||
| background-color: #00acdc; |
There was a problem hiding this comment.
This violates checklist item "CSS Variable is used for a blue color". The blue #00acdc is hardcoded in multiple places. Define a CSS variable (for example :root { --color-blue: #00acdc; }) and replace hardcoded usages with var(--color-blue).
| ul { | ||
| margin: 0; | ||
| padding: 0; | ||
| list-style: none; |
There was a problem hiding this comment.
This violates checklist item "Don't use tag names for styling (except html and body)". You are styling the ul element directly here. Replace this with a class-based selector (e.g. .nav__list { ... }) instead of ul { ... }.
| a { | ||
| text-decoration: none; |
There was a problem hiding this comment.
This violates checklist item "Don't use tag names for styling (except html and body)". You are styling the a tag directly to remove decoration. Prefer class-based selectors (e.g. .nav__link, .logo/.logo__img) and avoid global a {} rules.
| .nav__link { | ||
| position: relative; | ||
| display: flex; | ||
| align-items: center; | ||
| padding: 0; /* Прибрано внутрішні відступи */ | ||
| height: 100%; | ||
| font-size: 12px; /* Було 13px */ | ||
| text-transform: uppercase; | ||
| color: #000; | ||
| transition: color 0.2s; |
There was a problem hiding this comment.
This does not meet the requirement "Nav Links should not have any padding but have the text centered". .nav__link has padding: 0 (good) but no horizontal centering. Center the link text (for example add justify-content: center; when using display:flex, or text-align: center; if using block-level links) so the text is centered inside the nav link area.
| </a> | ||
| <nav class="nav"> | ||
| <ul class="nav__list"> | ||
| <li class="nav__item"> |
There was a problem hiding this comment.
This violates the requirement: "The height should be set for nav links (not the header), take it from the design" and the checklist item: "Header height is set in 1 place (for the links)". You set height: 60px on .header (line shown here). Remove the fixed height from .header and set the explicit height on the navigation links (for example .nav__link) so header height is defined from link height in one place.
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > |
There was a problem hiding this comment.
This violates the checklist item: "Nav Links should not have any padding but have the text centered". .nav__link has padding: 0 (good) but lacks horizontal centering. Add horizontal centering (for example justify-content: center on .nav__link or text-align: center) so link text is centered inside each link.
| <title>Moyo header</title> | ||
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap" | ||
| rel="stylesheet" |
There was a problem hiding this comment.
This violates the Code Style rule from the checklist: "Don't use tag names for styling (except html and body)". You use a global ul { ... } selector here — replace it with the class-based selector (for example .nav__list { ... }) to follow the style rules and avoid styling all ul elements.
| <h1>Moyo header</h1> | ||
| <header class="header"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
This also violates the checklist rule: "Don't use tag names for styling (except html and body)". You use a global a { ... } selector — change this to a class-based selector (e.g. .nav__link for link styles) so you don't style all anchors globally.
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| Photo | ||
| </a> | ||
| </li> | ||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| Video | ||
| </a> | ||
| </li> |
There was a problem hiding this comment.
This violates the checklist item: "- [ ] CSS Variable is used for a blue color". The blue #00acdc is hardcoded in multiple places (hover, active color, pseudo-element). Define a CSS variable, for example :root { --color-blue: #00acdc; } and replace #00acdc with var(--color-blue) for hover color, .is-active color and the ::after background so the color is controlled from one place.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
2 participants
