Skip to content

moyo header solution#7738

Open
kgordiychuk23-bit wants to merge 15 commits intomate-academy:masterfrom
kgordiychuk23-bit:my-feature
Open

moyo header solution#7738
kgordiychuk23-bit wants to merge 15 commits intomate-academy:masterfrom
kgordiychuk23-bit:my-feature

Conversation

@kgordiychuk23-bit
Copy link
Copy Markdown

No description provided.

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 the current src/index.html is missing the core header and navigation structure required by the task, which is causing both checklist violations and automated test failures. The page currently only shows an <h1> and text, but you must add a semantic <header> containing a logo link (<a> wrapping an <img> from src/images with a descriptive alt) placed outside the <nav>, and then a <nav><ul><li><a> list of links, including one link with class="is-active" and the 4th link with data-qa="hover" ([CHECKLIST ITEM #1], [#3], [#7]–[#9], [#11], [#15], [#18], [#19], [#20], [#33], [#34]). In the <head>, you also need to include the correct Google Fonts link that loads only Roboto, roman, weight 500, plus link your stylesheet where you reset default margins and set a font-family with fallback fonts ([CHECKLIST ITEM #21], [#22], [#23], [#24]). In your CSS, define a --blue variable, set the nav link height in a single place, vertically center header content using flexbox, transform link text to uppercase via CSS (not HTML), add a hover color, and implement .is-active::after as a pseudo-element positioned relative to the active link ([CHECKLIST ITEM #4], [#5], [#12], [#13], [#14], [#17], [#31], [#35]). These missing structures and styles are why BackstopJS cannot find header, nav, .is-active, or [data-qa="hover"] and why the visual diffs are so large, so updating the markup and CSS to match the spec is required before the task can be considered complete. As a minor clean-up, also remove the blank line between <body> and its first child to align with the formatting checklist ([CHECKLIST ITEM #40]). Once you implement the header/nav markup and the required CSS and font configuration, re-run npm test and use the Backstop HTML report to fine-tune spacing, alignment, and colors until the scenarios pass.


✨ 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/index.html Outdated
/>
</head>
<body>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing header/navigation and logo markup. This file currently lacks the semantic header and navigation required by the task. Violated checklist items (quote exact items):

  • "[CHECKLIST ITEM Task #1] Create an HTML page with the header using flexbox based on the Figma mockup."
  • "[CHECKLIST ITEM MOYO header by Kate #3] The header should stretch the full page width (don't use a horizontal margin)."
  • "[CHECKLIST ITEM solution #33] Use semantic tags: <header>, <img>, <nav>, <ul>, <li> and <a>."
  • "[CHECKLIST ITEM Solution #34] Use semantic tags where possible (like header, section, article, p."
  • "[CHECKLIST ITEM Fs on apr19 valeraonisko #7] The logo should be a link with an image inside (from src/images)."
  • "[CHECKLIST ITEM Solution #8] The logo should not be a part of the <nav>."
  • "[CHECKLIST ITEM Fs on apr19 oleksandr #9] Logo is an image wrapped with a link."
  • "[CHECKLIST ITEM solution #11] The blue link with a line below should have a class is-active in addition to any other classes you add."
  • "[CHECKLIST ITEM Fs apr19 ivan #15] Add data-qa="hover" attribute to the 4th link for testing (Laptops & computers)."
  • "[CHECKLIST ITEM Oleh_Rudko #18] alt attribute should describe the image content."
  • "[CHECKLIST ITEM Anna Kondratiuk #19] Don't use spaces in <a> tag's href property."
  • "[CHECKLIST ITEM Yevhen Kryvdiuk #20] Anchor links starts with the # symbol."

What to do: Add a semantic

containing the logo (an wrapping an

from src/images with a descriptive alt) outside the , then a with an
    and
  • / items. Ensure the 4th link has data-qa="hover" and the active link has class is-active.

Comment thread src/index.html Outdated
<body>

<h1>Moyo header</h1>
Hello, world!
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Head and styling setup missing required items. Violated checklist items (quote exact items):

What to do: In add the Google Fonts link that requests Roboto roman, weight 500 only (and no other styles), and add your stylesheet that resets default margins. In CSS define a --blue variable, set text-transform: uppercase for nav links (via class), set the link hover color, implement ::after for .is-active (positioned relative), and set the nav link height in one place (not the header).

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

There are no files to review. Please make sure that you have added files to the pull request. Some files and directories may be ignored from the review or failed to load.
[CODE: NFR]

Additional info:
Ignored files list:

  • .github/workflows/test.yml-template

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

There are no files to review. Please make sure that you have added files to the pull request. Some files and directories may be ignored from the review or failed to load.
[CODE: NFR]

Additional info:
Ignored files list:

  • .github/workflows/test.yml-template

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

There are no files to review. Please make sure that you have added files to the pull request. Some files and directories may be ignored from the review or failed to load.
[CODE: NFR]

Additional info:
Ignored files list:

  • .github/workflows/test.yml-template

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

There are no files to review. Please make sure that you have added files to the pull request. Some files and directories may be ignored from the review or failed to load.
[CODE: NFR]

Additional info:
Ignored files list:

  • .github/workflows/test.yml-template

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

@brespect brespect left a comment

Choose a reason for hiding this comment

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

Good progress, but I didn't see any changes from src directory, also you need to pass all tests and pass DEMO LINK to the Pull Request description

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