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.

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

moyo header task solution #3407

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

moyo header task solution #3407

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6b821cc
add test solution
arturgorniak Jun 28, 2023
5dadb73
add test solution
arturgorniak Jun 28, 2023
e43245b
final changes
arturgorniak Jun 28, 2023
e71aa9f
readme
arturgorniak Jun 28, 2023
14f50a8
implementing requested changes
arturgorniak Jun 29, 2023
a3a21b8
final changes
arturgorniak Jun 30, 2023
52d8e65
add box shadow
arturgorniak Jun 30, 2023
661b3df
add box shadow
arturgorniak Jun 30, 2023
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Moyo header
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://arturgorniak.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://arturgorniak.github.io/layout_moyo-header/report/html_report/)

> Follow [these instructions](https://mate-academy.github.io/layout_task-guideline/#how-to-solve-the-layout-tasks-on-github)

Expand Down
37 changes: 36 additions & 1 deletion src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,41 @@
<link rel="stylesheet" href="./style.css">
</head>
<body>
<h1>Moyo header</h1>
<header class="header">
<a href="#home" class="logo_link">
Copy link

Choose a reason for hiding this comment

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

Following the BEM naming convention please add a double underscore instead of one for block elements styles class="logo__link"

Please change that in the whole project HTML and CSS :)

<img
src="./images/logo.png"
alt="Moyo Header Logo"
>
</a>
Copy link

Choose a reason for hiding this comment

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

Here should be an empty space, this a tag is a sibling to nav, generally the rule is (apart from the exception with repeated tags) that the siblings should have an empty space between them:

<div>
  <span></span>

  <p></p>

  <div>
    <ul>
      <li></li>
      <li></li>
    </ul>
  </div>
</div>

<nav class="nav">
Copy link

Choose a reason for hiding this comment

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

When you have repeated tags there is no need for empty lines between them, for example:

<ul>
  <li></li>
  <li></li>
  <li></li>
  <li></li>
</ul>

<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" data-qa="hover">
Copy link

Choose a reason for hiding this comment

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

The 'data-qa' attribute has been added to the 'li' element, whereas it should be added to the 4th 'a' link as per the task description.

<a href="#laptops" class="nav_link">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>
76 changes: 76 additions & 0 deletions src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,79 @@
@import url("https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap");

:root {
--light-blue: #00acdc;
--black: #000;
}

html {
font-family: "Roboto", sans-serif;
}

body {
margin: 0;
}

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

.nav {
display: flex;
}

.logo_link {
Copy link

Choose a reason for hiding this comment

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

This should be enough here, rest is redundant:

.logo_link { 
  display: flex;
}

Copy link

Choose a reason for hiding this comment

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

Bump this Radek mention, you can refactor these styles as suggested

display: block;
width: 40px;
height: 40px;
text-decoration: none;
}

.nav_list {
display: flex;
list-style: none;
margin: 0;
padding: 0;
}

.nav_item {
margin-right: 20px;
}

.nav_item:last-child {
margin-right: 0;
}

.nav_link {
position: relative;
display: flex;
align-items: center;
font-size: 12px;
line-height: 15px;
padding: 23px 0;
Copy link

Choose a reason for hiding this comment

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

Use rounded values divisible by 5 as it makes any calculations and positioning much easier.

Copy link

Choose a reason for hiding this comment

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

Bump this also, please set this padding value to 22px or 24px

text-transform: uppercase;
color: var(--black);
text-decoration: none;
}

.nav_link:hover {
color: var(--light-blue);
}

.nav_link.is-active {
color: var(--light-blue);
}

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