-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Develop #7740
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 #7740
Changes from all commits
1a447f2
bdf7845
a009de0
8009c92
3b62a00
0f93356
00817be
c3b1c0f
ce2528f
8c097b1
1933c3b
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 |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| name: Test | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [ master ] | ||
|
|
||
| jobs: | ||
| build: | ||
|
|
||
| runs-on: ubuntu-latest | ||
|
|
||
| strategy: | ||
| matrix: | ||
| node-version: [20.x] | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
| - run: npm install | ||
| - run: npm test | ||
| - name: Upload HTML report(backstop data) | ||
| if: ${{ always() }} | ||
| uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: report | ||
| path: backstop_data |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,8 @@ | |
| <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, user-scalable=no, | ||
| initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0" | ||
| /> | ||
| <meta | ||
| http-equiv="X-UA-Compatible" | ||
|
|
@@ -15,8 +16,108 @@ | |
| rel="stylesheet" | ||
| href="./style.css" | ||
| /> | ||
| <link | ||
| rel="preconnect" | ||
| href="https://fonts.googleapis.com" | ||
| /> | ||
| <link | ||
| rel="preconnect" | ||
| href="https://fonts.gstatic.com" | ||
| crossorigin="anonymous" | ||
| /> | ||
| <link | ||
| rel="stylesheet" | ||
| href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap" | ||
|
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 #44: "In The Google Fonts |
||
| /> | ||
| </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 #46: "Update the Pull Request description to include the provided Checklist block, with each item marked Please update the PR description to include the DEMO and TEST REPORT links and copy the full Checklist block into the PR body, marking completed items with |
||
| <a | ||
| href="#image" | ||
| class="image" | ||
| > | ||
| <img | ||
| src="./images/logo.png" | ||
| class="logo" | ||
| alt="MOYO-logo" | ||
| /> | ||
| </a> | ||
| <nav class="nav"> | ||
| <ul class="nav_list"> | ||
| <li class="list_item"> | ||
| <a | ||
| class="nav__link is-active" | ||
| href="#apple" | ||
| > | ||
| Apple | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="list_item"> | ||
| <a | ||
| class="nav__link" | ||
| href="#samsung" | ||
| > | ||
| Samsung | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="list_item"> | ||
| <a | ||
| class="nav__link" | ||
| href="#smartphones" | ||
|
Comment on lines
+65
to
+68
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 rule uses a tag selector: |
||
| > | ||
| Smartphones | ||
| </a> | ||
| </li> | ||
|
Comment on lines
+71
to
+72
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. Checklist item #9 requires: "Link color should be changed on |
||
|
|
||
| <li class="list_item"> | ||
| <a | ||
| class="nav__link" | ||
| href="#laptops&computers" | ||
| data-qa="hover" | ||
| > | ||
| Laptops & Computers | ||
| </a> | ||
|
Comment on lines
+67
to
+81
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. Checklist item #26 requires a CSS variable for the blue color: "A CSS variable is used for the blue color". Right now the blue hex |
||
| </li> | ||
|
|
||
|
Comment on lines
+75
to
+83
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. |
||
| <li class="list_item"> | ||
| <a | ||
| class="nav__link" | ||
| href="#gadgets" | ||
| > | ||
| Gadgets | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="list_item"> | ||
| <a | ||
| class="nav__link" | ||
| href="#tablets" | ||
| > | ||
| Tablets | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="list_item"> | ||
| <a | ||
| class="nav__link" | ||
| href="#photo" | ||
| > | ||
| Photo | ||
| </a> | ||
| </li> | ||
|
|
||
| <li class="list_item"> | ||
| <a | ||
| class="nav__link" | ||
| href="#video" | ||
| > | ||
| Video | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| </nav> | ||
| </header> | ||
| </body> | ||
| </html> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,88 @@ | ||
| body { | ||
| margin: 0; | ||
| box-sizing: border-box; | ||
| font-family: Roboto, sans-serif; | ||
| font-size: 12px; | ||
| text-transform: uppercase; | ||
| font-weight: 500; | ||
|
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 "Lines of code have 80 chars max" and the checklist section "Keep your attributes correctly formatted" — the meta viewport tag value is split across lines and creates long lines. Wrap long attribute values and keep each attribute on its own line (see the Good Example in the checklist). |
||
| } | ||
|
|
||
|
Comment on lines
+5
to
+9
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 tag spans multiple lines but some resulting lines are still too long. Per checklist item #44 break this tag so each attribute starts on its own line and ensure no line exceeds 80 characters (wrap long attribute values accordingly). |
||
| :root { | ||
| --blue: #00acdc; | ||
| } | ||
|
|
||
| .header { | ||
| display: flex; | ||
| flex-direction: row; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| margin: 0; | ||
| padding: 0; | ||
| width: 100%; | ||
| } | ||
|
|
||
| .image { | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| padding-left: 50px; | ||
| } | ||
|
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 "Lines of code have 80 chars max" and the checklist section "Keep your attributes correctly formatted" — the long Google Fonts href should be wrapped across lines (start each attribute on a new line as in the Good Example). Break the href/rel attributes so no line exceeds 80 characters. |
||
|
|
||
| .logo { | ||
|
Comment on lines
+28
to
+31
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. |
||
| width: 40px; | ||
| height: 40px; | ||
| } | ||
|
|
||
| .nav { | ||
| margin-right: 50px; | ||
| } | ||
|
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 #14: "Logo is an image wrapped with a link" — the anchor wrapping the logo has no href attribute. Make the logo anchor a real link (for example: |
||
|
|
||
| .nav_list { | ||
| display: flex; | ||
| justify-content: space-between; | ||
| list-style: none; | ||
| margin: 0; | ||
| padding: 0; | ||
| } | ||
|
|
||
| .list_item { | ||
| margin-left: 20px; | ||
| } | ||
|
|
||
| .list_item:first-child { | ||
| margin-left: 0; | ||
| } | ||
|
|
||
| .nav__link { | ||
| display: flex; | ||
| text-decoration: none; | ||
| color: black; | ||
| line-height: 60px; | ||
| align-items: center; | ||
| position: relative; | ||
| } | ||
|
|
||
| .nav__link.is-active { | ||
| display: flex; | ||
| color: var(--blue); | ||
| position: relative; | ||
| } | ||
|
|
||
| .nav__link:hover { | ||
| color: var(--blue); | ||
| } | ||
|
|
||
| [data-qa='hover']:hover { | ||
| color: var(--blue); | ||
| } | ||
|
Comment on lines
+75
to
+77
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 #9: "Link color should be changed on :hover." Currently only the 4th link (via |
||
|
|
||
| .nav__link.is-active::after { | ||
| display: flex; | ||
| content: ''; | ||
| position: absolute; | ||
| width: 100%; | ||
| height: 4px; | ||
| background-color: var(--blue); | ||
| bottom: 0; | ||
| border-radius: 8px; | ||
| } | ||
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 #44: "In
src/index.html, break the<meta name="viewport" ...>tag and the Google Fonts<link>tag so that: each attribute starts on its own line, and each resulting line is under 80 characters (e.g., placehref,rel, andname/contenton separate wrapped lines)."The viewport meta's attributes/content currently produce a line that still exceeds the required 80-character limit / are not wrapped as requested. Please ensure each attribute starts on its own line and wrap long attribute values so every source line is <= 80 chars.