Skip to content
Open

Develop #7751

Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 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 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://VovaShmeriga.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://VovaShmeriga.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 All @@ -39,5 +39,5 @@ The page should match the design Pixel Perfect: all the sizes, colors and distan
- [ ] **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)
- [ ] The Google Fonts Configuration follows requirements.
![alt text](./assets/image.png)
- [ ] The Google Fonts Configuration follows requirements.
![alt text](./assets/image.png)
90 changes: 89 additions & 1 deletion src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,100 @@
content="ie=edge"
/>
<title>Moyo header</title>
<link
href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap"
rel="stylesheet"
/>
<link
rel="stylesheet"
href="./style.css"
/>
</head>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist items #4 and #19: "The height must be set for the nav links (not the header)" and "Header height must be set in one place (for the links)". You set .head { height: 60px; } — move the height to the nav link(s) (one source of truth) and remove the explicit header height.

<body>
<h1>Moyo header</h1>
<header class="header">
<a href="/">
<img
src="images/logo.png"
alt="MOYO logo"
class="logo"
/>
</a>
<nav class="nav">
<ul class="nav-list">
<li>
<a
data-qa="hover"
href="/"
class="nav-link is-active"
>
Apple
</a>
</li>
<li>
<a
data-qa="hover"
href="/"
class="nav-link"
>
Samsung
</a>
</li>
<li>
<a
data-qa="hover"
href="/"
class="nav-link"
>
Smartphones
</a>
</li>
<li>
<a
data-qa="hover"
href="/"
class="nav-link"
>
Laptops & computers
</a>
</li>
<li>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This uses the flex gap property which is forbidden by the task. The description explicitly says: "don't use the flex gap property for indents. It's not supported in tests, use margin instead" — replace gap with margins on the list items (and ensure you don't add outer margins before the first or after the last item).

<a
data-qa="hover"
href="/"
class="nav-link"
>
Gadgets
</a>
</li>
<li>
<a
data-qa="hover"
href="/"
class="nav-link"
>
Tablets
</a>
</li>
<li>
<a
data-qa="hover"
href="/"
class="nav-link"
>
Photo
</a>
</li>
<li>
<a
data-qa="hover"
href="/"
class="nav-link"
>
Video
</a>
</li>
</ul>
</nav>
</header>
</body>
</html>
64 changes: 64 additions & 0 deletions src/style.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,67 @@
:root {
--color-blue: #00ACDC;
}

body {
margin: 0;
font-family: Roboto, sans-serif;
font-weight: 500;
background-color: lightblue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The page must match the design Pixel Perfect: "all the sizes, colors and distances MUST be the same as on the design." Setting background-color: lightblue on body will most likely break pixel-perfect color requirements — remove or change it to match the design (use the design background color).

}

.header {
width: 100%;
background-color: #fff;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #10: "Use the Roboto font, selecting only roman style, medium (500) weight and normal width for embedding." You set font-family: Roboto in CSS but you did not include the Google Fonts embed/config in the HTML head with the required selection. Add the correct Google Fonts <link> (roman, weight 500) in the head.

display: flex;
align-items: center;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #20: "Implement the logo as an image wrapped with a link, using an image from src/images, and ensure this logo link is not part of the <nav>." The <img> at this line is not wrapped in an <a> — make the logo a clickable link outside the <nav>.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #32: "The alt attribute for images must describe the image content." The alt value "logo" is too generic; provide a descriptive alt (for example: alt="MOYO logo" or a more specific description).


.logo {
width: 40px;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #8: "CSS is used to show all letters in Uppercase (don't type them in HTML)". Link text is typed in uppercase in HTML (e.g. APPLE). Move casing to CSS using text-transform: uppercase and use normal-case text in HTML.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #21: "The blue link with a line below must have a class is-active in addition to any other classes you add." You currently use class="nav-link active"; change it to include is-active (for example class="nav-link is-active").

height: 40px;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #23: "Add data-qa="hover" attribute to the 4th link (Laptops & computers) for testing." The 4th link (LAPTOPS & COMPUTERS) is missing data-qa="hover" attribute — add it to that <a>.


.nav {
flex: 1;
}

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

.nav-list li:not(:first-child) {
margin-left: 20px;
}

.nav-link {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #7: "Link color must change on the :hover state." There is no :hover rule for .nav-link. Add a hover style (e.g. .nav-link:hover { color: var(--color-blue); }).

position: relative;
text-decoration: none;
color: black;
text-transform: uppercase;
display: flex;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The nav links don't center their text horizontally. The checklist requires: "Nav Links should not have any padding but have the text centered." Add horizontal centering to the .nav-link (for example justify-content: center) while keeping padding at 0.

align-items: center;
height: 60px;
}

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

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

.nav-link.is-active::after {
content: "";
position: absolute;
left: 50%;
transform: translateX(-50%);
width: 40px;
height: 3px;
background-color: var(--color-blue);
border-radius: 2px;
bottom: 0;
}
Loading