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 4 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
126 changes: 112 additions & 14 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,120 @@
<!doctype html>
<!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
rel="stylesheet"
href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap"
>

<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>

<nav class="nav">
<ul class="nav__list">
<li class="nav__item">
<a
href="#"
class="nav__link is-active"
>
Apple
</a>
</li>

<li class="nav__item">
<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 class="nav__item">
    <a
    href="#"
    class="nav__link"
    >
    Smartphones
    </a>
    </li>

    <li class="nav__item">
    <a
    href="#"
    class="nav__link"
    data-qa="hover"
    >
    Laptops & computers
    </a>
    </li>

    <li class="nav__item">
    <a
    href="#"
    class="nav__link"
    >
    Gadgets
    </a>
    </li>

    <li class="nav__item">
    <a
    href="#"
    class="nav__link"
    >
    Tablets
    </a>
    </li>

    <li class="nav__item">
    <a
    href="#"
    class="nav__link"
    >
    Photo
    </a>
    </li>

    <li class="nav__item">
    <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;
    }

    :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;
    margin: 0;
    padding: 0;
    list-style: none;
    }

    .nav__list > .nav__item + .nav__item {
    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;
    font-size: 12px;
    font-weight: 500;
    line-height: 14px;
    }

    .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