-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Moyo header #7739
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.
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
base: master
Are you sure you want to change the base?
Moyo header #7739
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,12 +11,104 @@ | |
| content="ie=edge" | ||
| /> | ||
| <title>Moyo header</title> | ||
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,500;1,500&display=swap" | ||
| rel="stylesheet" | ||
| /> | ||
| <link | ||
| rel="stylesheet" | ||
| href="./style.css" | ||
| /> | ||
| </head> | ||
| <body> | ||
| <h1>Moyo header</h1> | ||
| <header class="header"> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates: "[CHECKLIST ITEM #41] Don't use tag names for styling (except |
||
| <a | ||
| href="#" | ||
| class="logo" | ||
| > | ||
| <img | ||
| src="images/logo.png" | ||
| alt="Moyo logo" | ||
| class="logo__image" | ||
| /> | ||
| </a> | ||
|
|
||
| <nav class="nav"> | ||
| <ul class="nav__list"> | ||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link is-active" | ||
| > | ||
| apple | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| samsung | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="nav__item"> | ||
|
Comment on lines
+45
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates requirement: "Nav Links should not have any padding but have the text centered" — the CSS for |
||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| smartphones | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| data-qa="hover" | ||
| > | ||
| laptops & computers | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| gadgets | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| tablets | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| photo | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="nav__item"> | ||
| <a | ||
| href="#" | ||
| class="nav__link" | ||
| > | ||
| video | ||
| </a> | ||
| </li> | ||
| </ul> | ||
|
Comment on lines
+37
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item: "Add empty lines between multiline sibling blocks of HTML" — the |
||
| </nav> | ||
| </header> | ||
| </body> | ||
| </html> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,72 @@ | ||
| body { | ||
| margin: 0; | ||
| font-family: Roboto, sans-serif; | ||
| font-weight: 500; | ||
| font-size: 12px; | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line exceeds the allowed maximum line length. It violates the checklist item: "Lines of code have 80 chars max". Split long attribute values (like the viewport |
||
| :root { | ||
| --blue-color: #00acdc; | ||
| } | ||
|
|
||
| .header { | ||
| display: flex; | ||
| align-items: center; | ||
| box-sizing: border-box; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line includes a Google Fonts request that doesn't follow the task requirements. It violates the checklist item: "The Google font Roboto must be used, embedding only roman style, medium (500) weight, and normal width." The current href requests italic (1,500) as well. Also this attribute value is a long line exceeding the 80-char limit which violates: "Lines of code have 80 chars max". Please change the Google Fonts URL to request only the roman 500 weight (for example use a family query that requests only 0,500 / wght@500) and reformat the tag so no single line is longer than 80 characters. |
||
| width: 100%; | ||
| padding: 0 50px; | ||
| } | ||
|
|
||
| .logo { | ||
| margin-left: 0; | ||
| } | ||
|
|
||
| .logo__image { | ||
| display: block; | ||
| height: 40px; | ||
| } | ||
|
|
||
| .nav { | ||
| margin-left: auto; | ||
| } | ||
|
|
||
| .nav__list { | ||
| display: flex; | ||
| align-items: center; | ||
| list-style: none; | ||
| padding: 0; | ||
| margin: 0; | ||
| } | ||
|
|
||
| .nav__item:not(:last-child) { | ||
| margin-right: 20px; | ||
| } | ||
|
|
||
| .nav__link { | ||
| display: block; | ||
|
Comment on lines
+38
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates the checklist item: "Add empty lines between multiline sibling blocks of HTML". Many of your |
||
| height: 60px; | ||
| line-height: 60px; | ||
| text-decoration: none; | ||
| text-align: center; | ||
| color: #000; | ||
| text-transform: uppercase; | ||
| position: relative; | ||
| } | ||
|
|
||
| .nav__link:hover { | ||
| color: var(--blue-color); | ||
| } | ||
|
|
||
| .nav__link.is-active { | ||
| color: var(--blue-color); | ||
| } | ||
|
|
||
| .nav__link.is-active::after { | ||
| content: ''; | ||
| position: absolute; | ||
| bottom: 0; | ||
| left: 0; | ||
| width: 100%; | ||
| height: 4px; | ||
| background-color: var(--blue-color); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates checklist item #17: "use Roboto font: select ONLY roman style, medium (500) weight and normal width for embedding." The current Google Fonts href requests italic as well (it contains
ital,wght@0,500;1,500). Also this line appears to exceed the 80-character limit — see checklist item #46 below. Please load only the roman 500 variant (for example:?family=Roboto:wght@500&display=swap) and ensure the long href is wrapped/split so no HTML line exceeds 80 characters.