Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
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
8 changes: 4 additions & 4 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://github.com/maykonfox/layout_moyo-header-fox)
- [TEST REPORT LINK](https://https://github.com/mate-academy/layout_moyo-header/pull/7481)

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

Expand All @@ -39,5 +39,5 @@ The page should match the design Pixel Perfect: all the sizes, colors and distan
- [ ] **CSS Variable** is used for a blue color
- [ ] Pseudo-element is used for a blue line below the active link
- [ ] Code follows all the [Code Style Rules ❗️](./checklist.md)
- [ ] The Google Fonts Configuration follows requirements.
![alt text](./assets/image.png)
- [ ] The Google Fonts Configuration follows requirements.
![alt text](./assets/image.png)
64 changes: 52 additions & 12 deletions src/index.html
Original file line number Diff line number Diff line change
@@ -1,22 +1,62 @@
<!doctype html>
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<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"
content="width=device-width, initial-scale=1.0"
/>
<meta http-equiv="X-UA-Compatible" content="ie=edge" />
<title>Moyo header</title>
<link
rel="stylesheet"
href="./style.css"
/>
<link rel="preconnect" href="https://fonts.googleapis.com">
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap"
rel="stylesheet">
<link rel="stylesheet" href="./style.css"/>
</head>
<body>
<h1>Moyo header</h1>
<header class="header">
<a href="#" class="logo">
<img class="img"

Choose a reason for hiding this comment

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

Class names should be meaningful and not replicate tag names. A class like logo__image would be more descriptive and align better with the BEM-like naming convention you're using for navigation elements. This also relates to the checklist item 'Class names represent the meaning of the content (not the styles or tag names)'.

Copy link
Author

Choose a reason for hiding this comment

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

ok corrigido

src="images/logo.png"
alt="logo"

Choose a reason for hiding this comment

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

The alt text should be more descriptive. For screen readers and SEO, something like "Moyo company logo" would be more informative.

Copy link
Author

Choose a reason for hiding this comment

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

ok corrigido

>
</a>
<nav class="nav">
<ul class="nav_list">
<li class="nav_item">
<a href="#" class="nav_link">Apple</a>
</li>
<li class="nav_item">
<a href="#" class="nav_link">Samsung</a>
</li>
<li class="nav_item">
<a href="#" class="nav_link">Smartphones</a>
</li>
<li class="nav_item">
<a
href="#"
Comment on lines +71 to +72

Choose a reason for hiding this comment

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

According to the Figma mockup, the "Smartphones" link should be the active one. To match the design perfectly, please move the is-active class and the aria-current="page" attribute from this link to the <a> tag for "Smartphones". The data-qa="hover" attribute should remain on this link as required.

class="nav_link is-active"
aria-current="page"
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">Photos</a>
</li>
<li class="nav_item">
<a href="#" class="nav_link">Video</a>
</li>
</ul>
</nav>
</header>
</body>
</html>
79 changes: 79 additions & 0 deletions src/style.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,82 @@
:root {
--blue: #00acdc;
--nav-height: 56px;
}
body {
margin: 0;
font-family: 'Roboto', Arial, sans-serif;
background-color: rgb(201, 204, 204);
}
.header {
display: flex;
align-items: center;
justify-content: space-between;
font-weight: 500;
padding:0 40px;
background-color: #fff;
}
.img {
display: block;
width: 40px;
height: 40px;
}
.nav_list {
display: flex;
list-style: none;
Comment on lines +28 to +30

Choose a reason for hiding this comment

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

The <ul> element has default browser padding that needs to be reset for correct alignment. Please add padding: 0; to this rule to match the Figma design. It's also good practice to add margin: 0; to reset any default margins on the list.

Copy link
Author

Choose a reason for hiding this comment

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

ok corrigido

}

.nav_item + .nav_item {
margin-left: 24px;
}

.nav_link {
text-transform: uppercase;
display: inline-flex;
align-items: center;
justify-content: center;
position: relative;
text-decoration: none;
cursor: pointer;
height: var(--nav-height);
color: #000;
font-size: 14px;
font-weight: 500;
transition: color 0.3s;
}

.nav_link:hover {
color: var(--blue);
}
.nav_link.is-active {
position: relative;

Choose a reason for hiding this comment

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

This position: relative; declaration is redundant, as it's already defined for all links in the .nav_link class on line 37. You can remove this line for cleaner code.

Copy link
Author

Choose a reason for hiding this comment

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

ok corrigido

color: var(--blue);
}
.nav_link::after {
content: '';
position: absolute;
width: 100%;
left: 50%;
bottom: 2px;
height: 4px;
background-color: var(--blue);
border-radius: 8px;
opacity: 0;
transform-origin: center;
transition: opacity 0.2s ease, transform 0.2s ease;
transform: translateX(-50%) scaleX(0);
}
.nav_link:focus-visible{
color: var(--blue);
}
.nav_link:focus-visible::after{
transform: translateX(-50%) scaleX(1);
opacity: 1;
}
.nav_link:hover::after {
transform: translateX(-50%) scaleX(1);
opacity: 1;
}
.nav_link.is-active::after {
transform: translateX(-50%) scaleX(1);
opacity: 1;
}
Loading