Skip to content

solution: complete moyo header layout#7747

Open
xundseth wants to merge 3 commits intomate-academy:masterfrom
xundseth:master
Open

solution: complete moyo header layout#7747
xundseth wants to merge 3 commits intomate-academy:masterfrom
xundseth:master

Conversation

@xundseth
Copy link
Copy Markdown

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Changes are needed before this can be considered complete because one of the explicit checklist requirements is not yet satisfied. In src/style.css, in the .nav__link rule, you currently have display: flex; and align-items: center; (which correctly centers the content vertically), but there is no horizontal centering; per [CHECKLIST ITEM #10], navigation links must have no padding and their text must be centered inside the link area, so you should add something like justify-content: center; to that same rule to horizontally center the link text. Additionally, the task requires that the full checklist be copied into the Pull Request description and marked with - [x] for completed items; please update your PR description to include the provided checklist and mark the items you’ve completed there [CHECKLIST ITEM #26]. Once these two points are addressed, your implementation will align with both the behavioral requirement for nav link alignment and the process requirement for the PR metadata. Everything else in your header (semantic structure, flex layout, active state underline, hover color, and typography) looks solid and very close to the intended design. After making these adjustments, your solution should be ready to move forward.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/style.css
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because a couple of explicit checklist requirements are still not met, even though the header layout and behavior in your HTML/CSS look correct. In your Pull Request description, you have not copied the full checklist from the task and marked each completed item with - [x] after the DEMO and TEST REPORT links; this is a mandatory requirement and must be added to the PR description (see it as [CHECKLIST ITEM #21 / #22]). To fix this, open the task description, copy the provided Checklist block, paste it under your DEMO/TEST REPORT links in the PR description, and prefix each completed point with - [x]. Also, several lines in src/index.html exceed the 80‑character limit (for example, the long Google Fonts <link> tag near the top and some <a> elements with multiple attributes), which violates the rule that every code line must be at most 80 characters. You can fix this by wrapping long tags so that each attribute is on its own indented line and no physical line goes over 80 characters. As a minor improvement, double‑check your spacing, font sizes, and underline thickness against the Figma to ensure pixel‑perfect alignment, and consider only keeping explicit line-height values if they are required by the design. Once you address the PR checklist and line‑length issues, your solution should be in great shape.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

@xundseth
Copy link
Copy Markdown
Author

  • Header height is set in 1 place (for the links)
  • Content is vertically centered (for any header height)
  • CSS is used to show all letters in Uppercase (don't type them in HTML)
  • Logo is an image wrapped with a link
  • CSS Variable is used for a blue color
  • Pseudo-element is used for a blue line below the active link
  • Code follows all the Code Style Rules ❗️
  • The Google Fonts Configuration follows requirements.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a 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.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a 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.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a 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.

@xundseth xundseth marked this pull request as draft April 16, 2026 13:40
@xundseth xundseth closed this Apr 16, 2026
@xundseth xundseth reopened this Apr 16, 2026
@xundseth xundseth marked this pull request as ready for review April 16, 2026 14:13
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution as an educational override so you can progress, but there are a couple of important polish points to fix in your next iteration.

You did a solid job on the header structure: using flex on the header and navigation, keeping the logo as an image inside a link outside the <nav>, setting a fixed height on the nav links, using .is-active with a ::after underline, and loading Roboto 500 from Google Fonts all match the task requirements well.

The main HTML issue is line length: in src/index.html, the Google Fonts <link> tag near the top currently has all attributes on one long line that exceeds 80 characters, which violates [CHECKLIST ITEM #39]; split that tag so each attribute is on its own indented line and ensure no physical line goes over 80 characters.

On the repository side, your Pull Request description still needs the required metadata: copy in the full Checklist block from the task, and in your PR body put your actual DEMO and TEST REPORT links with your GitHub username (replacing <your_account>), then mark each completed item with - [x] as required by [CHECKLIST ITEM #22] and [CHECKLIST ITEM #23].

For cleaner HTML formatting, also check that there is a blank line between multiline sibling <li> blocks in your navigation list, as required by [CHECKLIST ITEM #35].

Overall, your layout and behavior are in good shape; once you tighten up the formatting and PR description, your submission will be fully aligned with the project standards—nice work so far, and keep refining these details.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread src/index.html
Comment on lines +19 to +21
<link
href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap"
rel="stylesheet"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item: "Lines of code have 80 chars max". The Google Fonts block is too long on the single attribute line. Also follow the checklist guidance: "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." Wrap/split the long tag (or its attributes) so no physical line exceeds 80 characters (for example put each attribute on its own indented line and ensure the long URL line stays under 80 chars).

Comment thread src/style.css

body {
margin: 0;
font-family: Roboto, sans-serif;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description must include your demo/test links and the copied Checklist block. Please add and complete these exactly as required. Quoting the checklist items that are not satisfied in the PR description:

  • "Replace <your_account> with your GitHub username in the links."
  • "Copy the DEMO LINK (https://<your_account>.github.io/layout_moyo-header/) to the Pull Request description (with <your_account> replaced)."
  • "Copy the TEST REPORT LINK (https://<your_account>.github.io/layout_moyo-header/report/html_report/) to the Pull Request description (with <your_account> replaced)."
  • "Copy the provided Checklist block to the Pull Request description after the links."
  • "Put - [x] before each checklist point after you checked it."

Action: update your Pull Request description — include the DEMO and TEST REPORT links with your GitHub username in place of <your_account>, then paste the full Checklist block from the task and mark each completed item with - [x].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants