Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 2 additions & 2 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ The page should match the design Pixel Perfect: all the sizes, colors and distan

❗️ Replace `<your_account>` with your GitHub username and copy the links to the `Pull Request` description:

- [DEMO LINK](https://<your_account>.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://<your_account>.github.io/layout_moyo-header/report/html_report/)
- [DEMO LINK](https://<Starkillerr>.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://<Starkillerr>.github.io/layout_moyo-header/report/html_report/)

❗️ Copy this `Checklist` to the `Pull Request` description after links, and put `- [x]` before each point after you checked it.

Expand Down
85 changes: 84 additions & 1 deletion src/index.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
<!doctype html>
<html lang="en">
<head>
<link
rel="stylesheet"
href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap"
/>
<meta charset="UTF-8" />
<meta
name="viewport"
Expand All @@ -17,6 +21,85 @@
/>
</head>
<body>
<h1>Moyo header</h1>
<header class="header">
<a
href="#"
class="logo"
>
<img
src="images/logo.png"
alt="Moyo logo"
/>
</a>

<nav>
<ul class="nav_list">
<li class="nav_item">
<a
class="nav_link active"

Choose a reason for hiding this comment

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

The requirement is to use the class name is-active for the active link, but active is used here. Please update the class name to match the specification.

href="#"
>
Apple
</a>
</li>
<li class="nav_item">
<a
class="nav_link"
href="#"
>
Samsung
</a>
</li>
<li class="nav_item">
<a
class="nav_link"
href="#"
>
Smartphones
</a>
</li>
<li class="nav_item">
<a
class="nav_link"
href="#"
>
Laptops & Computers
</a>
Comment on lines 62 to 68

Choose a reason for hiding this comment

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

This link is missing the required data-qa="hover" attribute. The task description specifies: 'add data-qa="hover" attribute to the 4th link for testing (Laptops & computers)'.

</li>
<li class="nav_item">
<a
class="nav_link"
href="#"
>
Gadgets
</a>
</li>
<li class="nav_item">
<a
class="nav_link"
href="#"
>
tablets
</a>
</li>
<li class="nav_item">
<a
class="nav_link"
href="#"
>
photo
</a>
</li>
<li class="nav_item">
<a
class="nav_link"
href="#"
>
video
</a>
</li>
</ul>
</nav>
</header>
</body>
</html>
62 changes: 62 additions & 0 deletions src/style.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,65 @@
html {
box-sizing: border-box;
}

body {
margin: 0;
padding: 0;
font-family: Roboto, sans-serif;
font-weight: 500;
font-style: normal;
}

.header {
background-color: #fff;
padding: 0 40px;
display: flex;
justify-content: space-between;
align-items: center;

.nav_list {
display: flex;
margin: 0;
padding: 0;
align-items: center;
list-style: none;
}
.nav_item {
margin-right: 30px;
}
.nav_item:last-child {
margin-right: 0;
}
.nav_link {
display: flex;
text-decoration: none;
color: #000;
height: 60px;
text-transform: uppercase;
position: relative;
align-items: center;
}
.logo {
height: 40px;
width: 40px;
}
.nav_link:hover {
color: #00a8ff;

Choose a reason for hiding this comment

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

This violates checklist item #5: 'CSS Variable is used for a blue color'. You should define the color #00a8ff as a CSS variable and use it with var() here and in other places. This improves maintainability.

}
.nav_link.active {

Choose a reason for hiding this comment

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

The task requirements specify that the active link should have the class is-active. Your selector uses .active. Please update this to match the requirements.

color: #00a8ff;
}

.nav_link.nav_link.active::after {

Choose a reason for hiding this comment

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

The selector .nav_link.nav_link.active::after is unnecessarily specific because the .nav_link class is repeated. Using .nav_link.active::after is sufficient and more readable.

content: '';
height: 2px;
width: 100%;
background-color: #00a8ff;
border-bottom: 3.5px solid #00a8ff;

Choose a reason for hiding this comment

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

The implementation of the underline has a couple of issues:

  1. The border-bottom uses a fractional value (3.5px), which goes against the requirement to avoid uneven sizes.
  2. Using height, background-color, and border-bottom together is redundant. A cleaner way to create the line is to use only height and background-color (e.g., height: 4px;).

border-radius: 10px;
position: absolute;
left: 0;
right: 0;
bottom: 0;
}

Choose a reason for hiding this comment

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

This file uses a nested syntax which is not valid in a standard CSS file. This syntax is common in preprocessors like SASS/SCSS, but a browser cannot parse it directly. You need to rewrite these rules without nesting. For example, .nav_list should become a top-level selector like .header .nav_list.

}
Loading