Skip to content
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.

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

add task solution #4691

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
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 `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://Pjankusha.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://Pjankusha.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
54 changes: 51 additions & 3 deletions src/index.html
Original file line number Diff line number Diff line change
@@ -1,14 +1,62 @@
<!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>
<link rel="stylesheet" href="./style.css">
<link
rel="stylesheet"
href="/src/style.css">
<link rel="preconnect" href="https://fonts.gstatic.com">
<link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Roboto:wght@500;700&display=swap">

</head>
<body>
<h1>Moyo header</h1>
<header class="header">
<a href="#" class="logo">
<img src="/src/images/logo.png" alt="moyo-logo">
</a>

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

Choose a reason for hiding this comment

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

I believe that you don't need this class

Suggested change
<a href="#" class="nav__link is-active blue-link">Apple</a>
<a href="#" class="nav__link is-active">Apple</a>

Choose a reason for hiding this comment

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

is-active is enough for such case

</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="#"
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>
71 changes: 69 additions & 2 deletions src/style.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,70 @@
body {
margin: 0;
:root {
--main-text-color: #000;
--edit-text-color: #00ACDC;
}

html, body {
width: 100%;
margin: 0;
padding:0;
font-family: Roboto, sans-serif;

Choose a reason for hiding this comment

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

Suggested change
width: 100%;
margin: 0;
padding:0;
font-family: Roboto, sans-serif;
margin: 0;
font-family: Roboto, sans-serif;

}

.header {
display: flex;
justify-content: space-between;
align-items: center;
padding: 0 50px;
font-size: 12px;
}

.logo {
height: 40px;
width: 40px;
}

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

.nav__link {
display: flex;
position: relative;
height: 60px;
justify-content: center;
align-items: center;
flex-direction: column;
text-decoration: none;
color: var(--main-text-color);
}

Choose a reason for hiding this comment

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

Suggested change
.nav__link {
display: flex;
position: relative;
height: 60px;
justify-content: center;
align-items: center;
flex-direction: column;
text-decoration: none;
color: var(--main-text-color);
}
.nav__link {
position: relative;
height: 60px;
text-decoration: none;
color: var(--main-text-color);
}

Copy link
Author

Choose a reason for hiding this comment

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

Hi, unfortunately your suggestion regarding removing flex in nav__link doesn't work.
Screenshot 2024-03-11 at 13 33 38
Screenshot 2024-03-11 at 13 33 10

Choose a reason for hiding this comment

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

You can try just to add text-align: center

Copy link
Author

Choose a reason for hiding this comment

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

Screenshot 2024-03-11 at 17 10 00 text-align: center doesn't work even if I change display to block. Regarding requirements to this task text should be centred, and height set to nav__links, flex in this case make it easy to do. And it is works right.


.nav__item{
margin-left: 20px;
}

.nav__list > :first-child{
margin-left: 0;
}

Choose a reason for hiding this comment

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

you can just use not:first-child

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for suggestion. It is looks better.


.blue-link,
.nav__link:hover {
color: var(--edit-text-color);
}

Choose a reason for hiding this comment

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

Suggested change
.blue-link,
.nav__link:hover {
color: var(--edit-text-color);
}
.is-active,
.nav__link:hover {
color: var(--edit-text-color);
}

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for this suggestion



.is-active::after {
width: 100%;
height: 4px;
content: "";
background-color: var(--edit-text-color);
border-radius: 8px;
position: absolute;
bottom: 0;
}