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

Jump to bottom

task solution #5381

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

task solution #5381

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
700bef2
task solution
D0dgerJ Sep 18, 2024
897bdf5
corrections
D0dgerJ Sep 18, 2024
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
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://D0dgerJ.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://D0dgerJ.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
100 changes: 94 additions & 6 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
<!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:ital,wght@0,100;0,300;0,400;0,500;0,700;0,900;1,100;1,300;1,400;1,500;1,700;1,900&display=swap"
rel="stylesheet"
/>
<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"
/>
<title>Moyo header</title>
<link
Expand All @@ -17,6 +25,86 @@
/>
</head>
<body>
<h1>Moyo header</h1>
<header class="header">
<a
href="/"
class="logo-link"
>
<img
src="images/logo.png"
alt="MOYO Logo"
class="logo"
/>
Copy link

Choose a reason for hiding this comment

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

There should be an empty line between multiline sibling blocks of HTML. Please add an empty line after the closing </a> tag and before the <nav> tag to improve readability.

Copy link

Choose a reason for hiding this comment

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

Ensure that the alt attribute of the <img> tag describes the content of the image. 'MOYO Logo' is a good description, but if 'MOYO' has a specific meaning or is a brand, including that context could be beneficial (e.g., 'MOYO Electronics Brand Logo').

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

Choose a reason for hiding this comment

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

Avoid using spaces in the href attribute of the <a> tag. If this is meant to be a placeholder, consider using '#apple' to indicate the intended target section.

</a>
</li>
<li class="nav__item">
<a
href="#"
class="nav__link"
>
Samsung
Copy link

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. Use '#samsung' as the value if this link is meant to direct to a Samsung section.

</a>
</li>
<li class="nav__item">
<a
href="#"
class="nav__link"
>
Smartphones
Copy link

Choose a reason for hiding this comment

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

Similar to the previous comments, ensure the href attribute does not contain spaces. Use '#smartphones' for better clarity.

</a>
</li>
<li class="nav__item">
<a
href="#"
class="nav__link"
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.

Don't use spaces in <a> tag's href property. The href for 'Laptops & Computers' contains a space, which should be replaced with a hyphen or underscore (e.g., href="#laptops-computers").

>
Laptops & Computers
Copy link

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 have spaces. Use '#laptops-computers' for the href value.

</a>
</li>
<li class="nav__item">
<a
href="#"
class="nav__link"
>
Gadgets
Copy link

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 include spaces. Use '#gadgets' as the href value.

</a>
</li>
<li class="nav__item">
<a
href="#"
class="nav__link"
>
Tablets
Copy link

Choose a reason for hiding this comment

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

Ensure the href attribute does not contain spaces. Use '#tablets' as the href value.

</a>
</li>
<li class="nav__item">
<a
href="#"
class="nav__link"
>
Photo
Copy link

Choose a reason for hiding this comment

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

The href attribute should be free of spaces. Use '#photo' as the href value.

</a>
</li>
<li class="nav__item">
<a
href="#"
class="nav__link"
>
Video
Copy link

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. Use '#video' as the href value.

</a>
</li>
</ul>
</nav>
</header>
</body>
</html>
71 changes: 70 additions & 1 deletion src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,72 @@
body {
:root {
--main-color: #00acdc;
}

* {
margin: 0;
padding: 0;
box-sizing: border-box;
}
Copy link

Choose a reason for hiding this comment

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

Don't use * selector as it impacts performance. It's better to set styles only for elements that require them. If you need to reset default margins and paddings, consider using a more specific reset CSS or applying these styles only to the elements that need them.


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

Choose a reason for hiding this comment

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

Remember to use fallback fonts - alternative font-family in case the main one doesn't work. For instance, you could specify a generic font family like 'sans-serif' as a fallback.


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

.nav__item {
margin-right: 20px;
}
Copy link

Choose a reason for hiding this comment

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

Be consistent with your vertical margins. It's recommended to add only top or only bottom margin, but not both, to prevent issues with margin collapse. In this case, consider using only margin-bottom for .nav__item.


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

Comment on lines +38 to +44
Copy link

Choose a reason for hiding this comment

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

Be consistent with your vertical margins. You're adding right margin to .nav__item which is horizontal, but make sure you apply the same principle to vertical margins as well, by adding only top or only bottom margin but not both.

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

.nav__list {
list-style: none;
display: flex;
}

.nav__link {
height: 60px;
display: flex;
justify-content: center;
align-items: center;
text-align: center;
font-weight: 500;
font-size: 12px;
text-transform: uppercase;
color: #000;
text-decoration: none;
}

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

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

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