Skip to content
Open
Show file tree
Hide file tree
Changes from all 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://mark-chep.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://mark-chep.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 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)
96 changes: 95 additions & 1 deletion src/index.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
<!doctype html>
<html lang="en">
<head>
<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:wght@500&display=swap"
rel="stylesheet"
/>

<meta charset="UTF-8" />
<meta
name="viewport"
Expand All @@ -17,6 +30,87 @@
/>
</head>
<body>
<h1>Moyo header</h1>
<header>
<a
href="https://www.moyo.ua/ua/"
class="logo"
>
<img
class="logo_img"
src="images/logo.png"
alt="website logo"

Choose a reason for hiding this comment

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

The alt attribute should be more descriptive. While 'website logo' is acceptable, something more specific like 'Moyo website logo' would better align with best practices for accessibility.

/>
</a>

<nav class="nav">
<ul class="nav__list">
<li class="nav__item">
<a
href="apple"
class="nav__link is-active"
>
apple
</a>
</li>
<li class="nav__item">
<a
href="samsung"
class="nav__link"
>
samsung
</a>
</li>
<li class="nav__item">
<a
href="smartphones"
class="nav__link"
>
smartphones
</a>
</li>
<li class="nav__item">
<a
href="laptops & computers"

Choose a reason for hiding this comment

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

The href attribute should not contain spaces. This violates the code style rules. Consider replacing spaces with hyphens, like laptops-and-computers.

class="nav__link"
data-qa="hover"
>
laptops & computers
</a>
</li>
<li class="nav__item">
<a
href="gadgets"
class="nav__link"
>
gadgets
</a>
</li>
<li class="nav__item">
<a
href="tablets"
class="nav__link"
>
tablets
</a>
</li>
<li class="nav__item">
<a
href="photo"
class="nav__link"
>
photo
</a>
</li>
<li class="nav__item">
<a
href="video"
class="nav__link"
>
video
</a>
</li>
</ul>
</nav>
</header>
</body>
</html>
68 changes: 68 additions & 0 deletions src/style.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,71 @@
:root {
--main-color: #00acdc;
}

body {
margin: 0;
padding: 0;
}

header {

Choose a reason for hiding this comment

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

According to the task requirements, you should avoid styling elements by their tag name (except for html and body). Please add a class to the <header> element in your HTML and use that class for styling.

font-family: Roboto, sans-serif;
font-size: 12px;

display: flex;
justify-content: space-between;
align-items: center;
padding: 0 50px;
}

.logo_img {
display: block;
height: 40px;
width: 40px;
margin: 10px 0;

Choose a reason for hiding this comment

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

This margin contributes to the header's height, but the requirement is that the height should be set in one place, on the navigation links. Also, using both top and bottom margins (margin: 10px 0) is inconsistent with the code style guideline to use only top or only bottom margins.

}

.nav__list {
display: flex;
padding: 0;
margin: 0;
}

.nav__item {
list-style: none;
text-decoration: none;
justify-content: center;
Comment on lines +35 to +36

Choose a reason for hiding this comment

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

These CSS properties have no effect here. text-decoration applies to text-containing elements like <a>, not <li>. justify-content applies to flex containers, but .nav__item is a flex item, not a container. These lines can be removed.

margin-left: 20px;
}

.nav__item:first-child {
margin-left: 0;
}

.nav__link {
display: inline-block;
text-decoration: none;
text-transform: uppercase;
color: #000;

padding: 22.5px 0;

Choose a reason for hiding this comment

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

This violates the requirement that 'Nav Links should not have any padding'. Instead, you should set a specific height on the links (as per the Figma design) and then center the text vertically. This approach also violates checklist item #1: 'Header height is set in 1 place (for the links)', as the header's height is currently influenced by both this padding and the logo's margin.

}

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

.is-active {
position: relative;
color: var(--main-color);
}

.nav__link.is-active::after {
content: '';
position: absolute;
bottom: 0;
left: 0;
width: 100%;
height: 4px;
border-radius: 4px;
background-color: var(--main-color);
}
Loading