Skip to content

done header #7746

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.

Sign up for GitHub

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

Open
lilbodyn wants to merge 5 commits into
base: master
Choose a base branch
from
Open

done header #7746

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a98e184
done header
lilbodyn Apr 16, 2026
17a563c
Fix review comments
lilbodyn Apr 16, 2026
3a3b328
Fix checklist and style issues
lilbodyn Apr 16, 2026
7a2bc60
Fix final review comments
lilbodyn Apr 16, 2026
6c77bda
Add report
lilbodyn Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 99 additions & 13 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,108 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta
name="viewport"
content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0"
/>
<meta
http-equiv="X-UA-Compatible"
content="ie=edge"
/>
<title>Moyo header</title>
<meta charset="UTF-8">
<title>Moyo Header</title>
<link
rel="preconnect"
href="https://fonts.googleapis.com"
>
<link
rel="preconnect"
href="https://fonts.gstatic.com"
Comment on lines +12 to +14
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 16, 2026

Choose a reason for hiding this comment

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

This CSS rule uses a tag selector for ul. The checklist requires styling by classes (only html and body may use tag selectors). Quote from the checklist: "Don't use tag names for styling (except html and body)". Move these resets to the .nav__list class (for example, .nav__list { margin: 0; padding: 0; }) and remove the ul { ... } rule.

crossorigin="anonymous"
>
<link
href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap"
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 16, 2026

Choose a reason for hiding this comment

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

This HTML line is longer than 80 characters. The checklist rule states: "Lines of code have 80 chars max". The Google Fonts <link> href value is very long (line 16). Please reformat so no line exceeds 80 characters (for example, ensure each attribute is on its own line and adjust the wrapping to keep lines <= 80 chars) while keeping the correct Google Fonts configuration.

rel="stylesheet"
>
<link
rel="stylesheet"
href="./style.css"
/>
>
</head>
<body>
<h1>Moyo header</h1>
<header class="header">
<a
href="#"
class="logo"
>
<img
src="./images/logo.png"
alt="Moyo logo"
class="logo__img"
>
</a>

Comment thread
lilbodyn marked this conversation as resolved.
<nav class="nav">
<ul class="nav__list">
<li>
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 16, 2026

Choose a reason for hiding this comment

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

Your

  • elements (starting here) do not have a class (e.g. nav__item). Because of the rule "Don't use tag names for styling (except html and body)", add a semantic class to every
  • and update your CSS to target that class instead of the li tag.

    •

    <a
    href="#"
    class="nav__link is-active"
    >
    Apple
    </a>
    </li>
    <li>
    <a
    href="#"
    Copy link
    Copy Markdown

    @mateacademy-ai-mentor mateacademy-ai-mentor Apr 16, 2026

    Choose a reason for hiding this comment

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

    The selector .nav__list > li + li uses the li tag name in CSS. The project rules forbid tag-name styling: "Don't use tag names for styling (except html and body)". Add a semantic class to each

  • (for example nav__item) and change the selector to use that class (e.g. .nav__list > .nav__item + .nav__item).

    •

    class="nav__link"
    >
    Samsung
    </a>
    </li>
    <li>
    <a
    href="#"
    class="nav__link"
    >
    Smartphones
    </a>
    </li>
    <li>
    <a
    href="#"
    class="nav__link"
    data-qa="hover"
    >
    Laptops & computers
    </a>
    </li>
    <li>
    <a
    href="#"
    class="nav__link"
    >
    Gadgets
    </a>
    </li>
    <li>
    <a
    href="#"
    class="nav__link"
    >
    Tablets
    </a>
    </li>
    <li>
    <a
    href="#"
    class="nav__link"
    >
    Photo
    </a>
    </li>
    <li>
    <a
    href="#"
    class="nav__link"
    >
    Video
    </a>
    </li>
    </ul>
    </nav>
    </header>
    </body>
    </html>
    </html>
    70 changes: 70 additions & 0 deletions src/style.css
    View file
    Open in desktop
    Original file line number Diff line number Diff line change
    @@ -1,3 +1,73 @@
    html {
    box-sizing: border-box;
    }

    body {
    margin: 0;
    font-family: Roboto, sans-serif;
    }

    ul {
    margin: 0;
    padding: 0;
    Copy link
    Copy Markdown

    @mateacademy-ai-mentor mateacademy-ai-mentor Apr 16, 2026

    Choose a reason for hiding this comment

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

    This CSS block uses the ul tag selector which violates the checklist rule: "Don't use tag names for styling (except html and body)". Target the nav list with its class (for example .nav__list { margin: 0; padding: 0; }) instead of styling the ul tag directly so you comply with [CHECKLIST ITEM #17].

    }

    :root {
    --blue: #00acdc;
    }

    .header {
    display: flex;
    justify-content: space-between;
    align-items: center;
    padding: 0 50px;
    Comment on lines +14 to +18
    Copy link
    Copy Markdown

    @mateacademy-ai-mentor mateacademy-ai-mentor Apr 16, 2026

    Choose a reason for hiding this comment

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

    This violates checklist item #4: "The height must be set for the nav links (not for the header), and this height must be taken from the design." You set height: 60px; on .header. Move the height declaration to the nav links (only) and remove it from the header so the header height is determined by its content.

    }

    .logo {
    display: flex;
    align-items: center;
    }

    .logo__img {
    display: block;
    height: 40px;
    }

    .nav__list {
    display: flex;
    list-style: none;
    }

    .nav__list > li + li {
    margin-left: 20px;
    }

    .nav__link {
    display: flex;
    align-items: center;
    justify-content: center;
    height: 60px;
    color: #000;
    text-decoration: none;
    text-transform: uppercase;
    position: relative;
    }

    .nav__link:hover {
    color: var(--blue);
    }

    .is-active {
    color: var(--blue);
    }

    .is-active::after {
    content: "";
    position: absolute;
    right: 0;
    bottom: 0;
    left: 0;
    height: 4px;
    background-color: var(--blue);
    border-radius: 8px;
    }
    Loading