-
Notifications
You must be signed in to change notification settings - Fork 6k
added header #7274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
added header #7274
Conversation
TetianaSobolieva
commented
Nov 24, 2025
- DEMO LINK
- TEST REPORT LINK
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
Your HTML structure is well done, but changes are needed in src/style.css to meet several core requirements. Please address the following issues:
- Incorrect Font: The task requires embedding the Roboto font with only the 'medium (500)' weight, but a variable font file was used. Please use the correct static font file. [CHECKLIST ITEM #8]
- Forbidden Selector: The universal selector (
*) is used, which is explicitly forbidden by the project's style guide. [CHECKLIST ITEM #7] - Missing CSS Variable: The blue color
#00acdcis hardcoded. It must be defined as a CSS variable and applied usingvar(). [CHECKLIST ITEM #5] - Padding on Nav Links: The requirement "Nav Links should not have any padding" was violated. Please remove the
paddingand use a fixedheightwith flexbox properties for vertical alignment.
Addressing these points will bring your solution in line with the project's specifications.
✨ 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
src/style.css
Outdated
| @@ -1,3 +1,75 @@ | |||
| @font-face { | |||
| font-family: Roboto; | |||
| src: url(fonts/Roboto/Roboto-VariableFont_wdth\,wght.ttf); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates checklist item #8. The task requires embedding the Roboto font with only the 'roman' style, 'medium (500)' weight, and 'normal' width. The variable font file you are using includes all weights and styles. Please follow the font configuration instructions precisely.
src/style.css
Outdated
| src: url(fonts/Roboto/Roboto-VariableFont_wdth\,wght.ttf); | ||
| } | ||
|
|
||
| * { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates checklist item #7: "Don't use * selector". The requirements prohibit its use. Please remove this universal selector and apply styles to specific elements as needed.
src/style.css
Outdated
| position: relative; | ||
| display: flex; | ||
| align-items: center; | ||
| padding: 23px 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This padding violates two requirements. First, the task explicitly states, "Nav Links should not have any padding". Second, the vertical padding value creates a total height that doesn't match the 86px from the Figma design. You should set a fixed height on the links and use flex properties to center the text instead.
src/style.css
Outdated
|
|
||
| .nav__link:hover, | ||
| .nav__link.is-active { | ||
| color: #00acdc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates checklist item #5: "CSS Variable is used for a blue color". The color #00acdc is hardcoded here and for the ::after pseudo-element. You should declare it as a CSS variable (e.g., in a :root selector) and use var() to apply it in both places.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
Excellent work on this update! You have successfully addressed all the previous feedback, and the solution now fully complies with all project requirements. The HTML is well-structured with correct semantic tags, and the CSS is clean, using variables and flexbox as specified. Your solution is approved. Great job!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨