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

Task solved #5900

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

Task solved #5900

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
957ed64
Task solved
anna-poplavska Jan 4, 2025
f14b977
Task solved 2
anna-poplavska Jan 6, 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
18 changes: 9 additions & 9 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ 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://anna-poplavska.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://anna-poplavska.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.

- [ ] Header height is set in 1 place (for the links)
- [ ] Content is vertically centered (for any header height)
- [ ] CSS is used to show all letters in Uppercase (don't type them in HTML)
- [ ] Logo is an image wrapped with a link
- [ ] **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)
- [x] Header height is set in 1 place (for the links)
- [x] Content is vertically centered (for any header height)
- [x] CSS is used to show all letters in Uppercase (don't type them in HTML)
- [x] Logo is an image wrapped with a link
- [x] **CSS Variable** is used for a blue color
- [x] Pseudo-element is used for a blue line below the active link
- [x] Code follows all the [Code Style Rules ❗️](./checklist.md)
95 changes: 94 additions & 1 deletion src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,101 @@
rel="stylesheet"
href="./style.css"
/>
<link
rel="preconnect"
href="https://fonts.googleapis.com"
/>
<link
rel="preconnect"
href="https://fonts.gstatic.com"
crossorigin="anonymous"
/>
<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"
/>
</head>
<body>
<h1>Moyo header</h1>
<header class="header">
Copy link

Choose a reason for hiding this comment

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

Consider using a <nav> element instead of a <header> for the navigation section to improve semantic structure. This change can enhance accessibility by clearly indicating the navigation role to assistive technologies.

<a
href="index.html"
class="logo__link"
>
<img
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 image path 'images/logo.png' is correct and that the image exists in the specified location. This will prevent broken image links on the webpage.

class="logo__img"
src="images/logo.png"
alt="Moyo logo"
/>
</a>
<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.

Consider adding aria-label or aria-labelledby attributes to the <nav> element to provide a descriptive label for screen readers, enhancing accessibility.

<ul class="nav__list">
<li class="nav__item">
<a
Copy link

Choose a reason for hiding this comment

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

The 'is-active' class is applied to the 'Apple' link. Ensure this class is dynamically managed if this is meant to indicate the current page, to avoid misleading users.

class="nav__link is-active"
href="#"
>
Apple
</a>
</li>
<li class="nav__item">
<a
class="nav__link"
href="#"
>
Samsung
</a>
</li>
<li class="nav__item">
<a
class="nav__link"
href="#"
>
Smartphones
</a>
</li>
<li class="nav__item">
<a
class="nav__link"
href="#"
data-qa="hover"
>
Laptops & Computers
</a>
</li>
<li class="nav__item">
<a
class="nav__link"
href="#"
>
Gadgets
</a>
</li>
<li class="nav__item">
<a
class="nav__link"
href="#"
>
Tablets
</a>
</li>
<li class="nav__item">
<a
class="nav__link"
href="#"
>
Photo
</a>
</li>
<li class="nav__item">
<a
class="nav__link"
href="#"
>
Video
</a>
</li>
</ul>
</nav>
Comment on lines +44 to +112
Copy link

Choose a reason for hiding this comment

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

Make sure that the logo is not part of the <nav> element. The current structure seems correct, but double-check to ensure compliance with the task requirements.

</header>
Comment on lines +33 to +113
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 header stretches the full page width without using horizontal margins. This is a critical requirement according to the task description.

</body>
</html>
68 changes: 68 additions & 0 deletions src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,71 @@
body {
--primary-link-color: #000;
--highlight-link-color: #00acdc;

margin: 0;
}

.header {
width: 100%;
display: flex;
justify-content: space-between;
align-items: center;
}

.logo__link {
margin-left: 50px;
Copy link

Choose a reason for hiding this comment

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

The use of margin-left: 50px; in .logo__link contradicts the requirement for the header to stretch the full page width without using horizontal margins. Consider removing this margin.

}

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

.nav {
margin-right: 50px;
Copy link

Choose a reason for hiding this comment

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

The use of margin-right: 50px; in .nav contradicts the requirement for the header to stretch the full page width without using horizontal margins. Consider removing this margin.

}

.nav__list {
display: flex;
align-items: center;
padding: 0;
margin: 0;
list-style: none;
}

.nav__link {
display: flex;
align-items: center;
position: relative;
height: 60px;
text-transform: uppercase;
font-family: Roboto, serif;
Copy link

Choose a reason for hiding this comment

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

The font stack includes 'Roboto' and 'serif'. Ensure that 'Roboto' is correctly loaded in your HTML file, as it is a sans-serif font, and consider using a fallback sans-serif font instead of 'serif'.

font-size: 12px;
font-weight: 500;
color: var(--primary-link-color);
text-decoration: none;
}

.nav__item:not(:last-child) {
margin-right: 20px;
}

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

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

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