Skip to content

Develop #7279

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
wants to merge 2 commits into
base: master
Choose a base branch
from
+162 −1
Open

Develop #7279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd14051
added header with links
Nov 24, 2025
836f146
deleted h1
Nov 24, 2025
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
53 changes: 52 additions & 1 deletion src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,64 @@
http-equiv="X-UA-Compatible"
content="ie=edge"
/>
<link
rel="preconnect"
href="https://fonts.googleapis.com"
/>
<link
rel="preconnect"
href="https://fonts.gstatic.com"
/>
<link
href="https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,100..900;1,100..900&display=swap"
rel="stylesheet"
/>
Comment on lines +21 to +24
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

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 Google Fonts Configuration follows requirements.' You have imported a wide range of font weights and styles, but the task requires only the roman style with medium (500) weight. Please adjust the font URL to load only the specific font variant needed.

<title>Moyo header</title>
<link
rel="stylesheet"
href="./style.css"
/>
</head>

<header class="header">
<a
href="index.html"
class="logo"
>
<img
src="images/logo.png"
alt="Moyo"
/>
</a>
<nav>
<ul class="nav-list">
<li>
<a
href="#"
class="is-active nav-item"
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

The nav-item class is applied to this link, but it's missing from most other navigation links. This will lead to inconsistent styling. For consistent styling, a common class should be applied to all navigation links (or their parent <li> elements).

>
Apple
</a>
</li>
<li><a href="#">Samsung</a></li>
<li><a href="#">Smartphones</a></li>
<li>
<a
href="#"
data-qa="hover nav-item"
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

The value of the data-qa attribute should be just hover. The additional nav-item text here is incorrect and will likely cause automated tests to fail.

>
Laptops & Computers
</a>
</li>
<li><a href="#">Gadgets</a></li>
<li><a href="#">Tablets</a></li>
<li><a href="#">Photo</a></li>
<li><a href="#">Video</a></li>
</ul>

<div class="nav-line"></div>
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

This <div> element is unnecessary. The requirement specifies using a CSS pseudo-element to create the blue line under the active navigation link, so this extra HTML element should be removed.

</nav>
</header>
Comment on lines +32 to +70
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

The <header> element should be placed inside the <body> element. Currently, it is a sibling to <body>, which is structurally incorrect for an HTML document.

<body>
<h1>Moyo header</h1>
</body>
</html>
110 changes: 110 additions & 0 deletions src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,113 @@
body {
/* margin: 0;
padding: 0;
box-sizing: border-box; */
height: 100%;

font-family: Roboto, sans-serif;
font-weight: 400;

--blue: #0070f3;
}

* {
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

Using the universal selector * to reset styles is not allowed by the project's code style rules. It's better to apply reset styles to body or specific elements that need them.

margin: 0;
padding: 0;
box-sizing: border-box;
}

.header {
display: flex;
align-items: center;
justify-content: space-between;

width: 100%;
margin: 0;
padding: 0 20px;
}

.logo img {
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

The code style rules forbid using tag names for styling (except for html and body). You should add a class to the img element and use that class to apply styles.

display: block;
margin: 10px 50px auto;
width: auto;
height: 40px;
}

.nav-list {
display: flex;
align-items: center;
list-style: none;
margin: 0;
padding: 0;
text-transform: uppercase;
}

.nav-item {
position: relative;
margin-right: 20px;
}

.nav-item:first-child::before {
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

This violates checklist item #6: 'Pseudo-element is used for a blue line below the active link'.
The implementation for the active link's underline is incorrect. The requirements specify using the ::after pseudo-element and targeting the .is-active class. This rule incorrectly uses ::before and targets :first-child.

content: '';
position: absolute;
bottom: -20px;
left: 0;
width: 100%;
height: 4px;
background-color: var(--blue);
border-radius: 8px;
}

.nav-list a {
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

Styling elements by tag name (a) is against the code style rules. All navigation links should have a common class for styling.

display: inline-block;
list-style: none;
height: 15px;
line-height: 15px;
text-align: center;
text-decoration: none;
position: relative;
padding: 0;
font-weight: 500;
color: #222;
white-space: nowrap;
}

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

.nav-list li:first-child {
margin-left: 0;
}

.nav-list li:last-child {
margin-right: 0;
}

.nav-list a:hover::after {
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

The requirement is that the link color should change on hover. This rule only adds a blue line using ::after, but it doesn't change the text color of the link. You should add a color property to a .nav-item:hover rule.

content: '';
position: absolute;
bottom: 0;
left: 0;
width: 100%;
height: 4px;
background-color: var(--blue);
}

.nav-list li:first-child a,
.nav-list li:nth-child(4) a {
color: var(--blue) !important;
Comment on lines +97 to +99
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Nov 24, 2025

Choose a reason for hiding this comment

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

Styling elements based on their position with :first-child or :nth-child() is not the correct approach here. The active link's style should be applied using the .is-active class. The color for the fourth link seems to be for demonstrating a hover state, which should be handled by a :hover pseudo-class instead.

}

@media (max-width: 480px) {
.nav-list {
flex-wrap: wrap;
}
.nav-list > li + li {
margin-left: 12px;
}
.nav-list a {
height: 15px;
line-height: 15px;
}
}
Loading