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

add task solution #6090

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

add task solution #6090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f740c2
add task solution
TomasQ789 Feb 5, 2025
8f93107
add task solution 2
TomasQ789 Feb 5, 2025
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
6 changes: 3 additions & 3 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The page should match the design Pixel Perfect: all the sizes, colors and distan
- reset browser default margins
- use [Roboto font](https://fonts.google.com/specimen/Roboto): select ONLY **roman** style, **medium (500)** weight and **normal** width for embedding
- use semantic tags: `<header>`, `<img>`, `<nav>`, `<ul>`, `<li>` and `<a>`
- the header should stretch the full page width (don't use a horizontal margin)
- header should stretch the full page width (don't use a horizontal margin)
- the height should be set for nav links (not the header), take it from the design
- header content should be vertically centered
- the logo should also be a link with an image inside (from [src/images](src/images)). But it should not be a part of the `<nav>`
Expand All @@ -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://TomasQ789.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://TomasQ789.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
102 changes: 95 additions & 7 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,107 @@
<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"
content="width=device-width, initial-scale=1.0"
/>
<meta
http-equiv="X-UA-Compatible"
content="ie=edge"
<title>Menu</title>
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 title of the page is descriptive and relevant to the content. 'Menu' might not be the most descriptive title for the page.

<link
rel="stylesheet"
href="style.css"
/>
<link
rel="preconnect"
href="https://fonts.googleapis.com"
/>
<link
rel="preconnect"
href="https://fonts.gstatic.com"
/>
<title>Moyo header</title>
<link
href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap"
rel="stylesheet"
href="./style.css"
/>
</head>
<body>
<h1>Moyo header</h1>
<header>
Copy link

Choose a reason for hiding this comment

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

The <header> tag should not contain the logo directly. According to semantic HTML practices, the logo should be placed outside of the <nav> element, possibly within a <div> or <section> to separate it from the navigation.

<a
href="/"
class="logo"
Comment on lines +29 to +31
Copy link

Choose a reason for hiding this comment

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

Consider adding an aria-label attribute to the anchor tag for better accessibility. This will help screen readers understand the purpose of the link.

Copy link

Choose a reason for hiding this comment

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

The class name logo is appropriate, but ensure that it is styled correctly in your CSS file to meet the design requirements.

aria-label="Logo of the company"
>
<img
class="Moyo1"
Copy link

Choose a reason for hiding this comment

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

The class name Moyo1 is not meaningful. Class names should be indicative of the content or purpose, not styles or arbitrary names. Consider renaming it to something more descriptive.

src="images/logo.png"
alt="Logo"
/>
</a>
<nav>
<ul>
<li>
<a
href="#"
class="Apple is-active"
Comment on lines +43 to +45
Copy link

Choose a reason for hiding this comment

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

The class name 'is-active' suggests that this menu item is currently active. Ensure that this class is dynamically updated based on the current page or section to provide accurate context to users.

Copy link
Author

Choose a reason for hiding this comment

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

What exactly do you mean?

Copy link

Choose a reason for hiding this comment

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

The class names Apple, Samsung, etc., are not meaningful in terms of content. Class names should reflect the role or purpose of the element, not the specific content. Consider using more generic names like product-category or similar.

>
Apple
</a>
</li>
<li>
<a
href="#"
class="Samsung"
>
Samsung
</a>
</li>
<li>
<a
href="#"
class="Smartphones"
>
Smartphones
</a>
</li>
<li>
<a
href="#"
class="LaptopsComputers"
>
Laptops & Computers
</a>
</li>
<li>
<a
href="#"
class="Gadgets"
>
Gadgets
</a>
</li>
<li>
<a
href="#"
class="Tablets"
>
Tablets
</a>
</li>
<li>
<a
href="#"
class="Photo"
>
Photo
</a>
</li>
<li>
<a
href="#"
class="Video"
>
Video
</a>
</li>
</ul>
</nav>
</header>
</body>
</html>
77 changes: 77 additions & 0 deletions src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,80 @@
:root {
--blue-color: #00acdc;
}

body {
margin: 0;
padding: 0;
box-sizing: border-box;
font-family: Roboto, Arial, sans-serif;
background: white;
}

header {
display: flex;
justify-content: space-between;
min-width: 1024px;
max-width: 1200px;
Comment on lines +16 to +17
Copy link

Choose a reason for hiding this comment

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

Avoid using fixed widths like min-width: 1024px and max-width: 1200px for the header. Instead, use flexible units like percentages or max-width with width: 100% to ensure better responsiveness across different screen sizes.

height: 60px;
}

.logo {
margin-top: 10px;
margin-left: 50px;
}

.Moyo1 {
Copy link

Choose a reason for hiding this comment

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

The class name .Moyo1 is not meaningful. Consider renaming it to something more descriptive, such as .logo-image, to better convey its purpose.

width: 40px;
height: 40px;
}

nav {
margin-right: 50px;
}

nav ul {
margin: 0;
padding: 0;
list-style: none;
display: inline-flex;
}

li {
padding-right: 20px;
}

li:last-child {
padding-right: 0;
}

nav a {
font-size: 12px;
font-weight: 500;
text-transform: uppercase;
color: black;
text-decoration: none;
display: block;
position: relative;
height: 60px;
text-align: center;
line-height: 60px;
}

nav a:hover {
color: var(--blue-color);
}

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

nav a.is-active::after {
content: '';
position: absolute;
left: 0;
right: 0;
bottom: 0;
height: 4px;
background-color: var(--blue-color);
border-radius: 8px;
}
Loading