-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Develop #7751
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?
Develop #7751
Changes from 4 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 |
|---|---|---|
| @@ -1,22 +1,55 @@ | ||
| <!doctype html> | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <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" | ||
| /> | ||
| <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"/> | ||
| <title>Moyo header</title> | ||
| <link | ||
| rel="stylesheet" | ||
| href="./style.css" | ||
| /> | ||
| <link href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap" rel="stylesheet"> | ||
| <link rel="stylesheet" href="./style.css"/> | ||
| </head> | ||
|
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. |
||
| <body> | ||
| <h1>Moyo header</h1> | ||
| <header class="header"> | ||
| <a href="/"> | ||
| <img src="images/logo.png" alt="MOYO logo" | ||
| class="logo"/> | ||
|
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. HTML attributes formatting must follow the checklist: "If the HTML-element has long attribute values or number of attributes is more than 2 - start each one, including the first, on the new line with 2-space indentation related to tag. Tag’s closing bracket should be on the same level as opening one." The |
||
| </a> | ||
| <nav class="nav"> | ||
| <ul class="nav-list"> | ||
| <li> | ||
| <a data-qa="hover" | ||
| href="/" class="nav-link is-active">Apple</a> | ||
|
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. Anchor elements with more than 2 attributes must follow the same formatting rule: "If the HTML-element has long attribute values or number of attributes is more than 2 - start each one, including the first, on the new line with 2-space indentation related to tag. Tag’s closing bracket should be on the same level as opening one." The first nav |
||
| </li> | ||
| <li> | ||
| <a data-qa="hover" href="/" | ||
| class="nav-link">Samsung</a> | ||
|
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. Another nav link showing the same attribute-formatting issue. Reformat anchors with 3+ attributes so each attribute starts on its own line (2-space indent) as required: "If the HTML-element has long attribute values or number of attributes is more than 2 - start each one..." |
||
| </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> | ||
|
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. The 4th nav link (Laptops & computers) must have the test attribute |
||
| </li> | ||
| <li> | ||
|
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 uses the flex |
||
| <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> | ||
| </html> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,66 @@ | ||
| :root { | ||
| --color-blue: #00ACDC; | ||
| } | ||
|
|
||
| body { | ||
| margin: 0; | ||
| font-family: Roboto, sans-serif; | ||
| font-weight: 500; | ||
| background-color: lightblue; | ||
|
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. The page must match the design Pixel Perfect: "all the sizes, colors and distances MUST be the same as on the design." Setting |
||
| } | ||
|
|
||
| .header { | ||
| width: 100%; | ||
| background-color: #fff; | ||
|
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 #10: "Use the Roboto font, selecting only roman style, medium (500) weight and normal width for embedding." You set |
||
| display: flex; | ||
| align-items: center; | ||
| padding: 0 20px; | ||
| 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 violates checklist item #20: "Implement the logo as an image wrapped with a link, using an image from 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 #32: "The |
||
|
|
||
| .logo { | ||
| width: 40px; | ||
|
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 #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. 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 #21: "The blue link with a line below must have a class |
||
| height: 40px; | ||
| } | ||
|
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 #23: "Add |
||
|
|
||
| .nav { | ||
| flex: 1; | ||
| } | ||
|
|
||
| .nav-list { | ||
| list-style: none; | ||
| margin: 0; | ||
| padding: 0; | ||
| display: flex; | ||
| gap: 20px; | ||
|
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 uses the flex |
||
| } | ||
|
|
||
| .nav-link { | ||
|
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 #7: "Link color must change on the |
||
| position: relative; | ||
| text-decoration: none; | ||
| color: black; | ||
| text-transform: uppercase; | ||
| display: flex; | ||
|
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. 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 |
||
| 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; | ||
| } | ||
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.
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 the body will likely fail visual tests. Remove or change this to the design color (or omit body background so the default/design background is used).