Skip to content
Open

Develop #7742

Show file tree
Hide file tree
Changes from 2 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
29 changes: 29 additions & 0 deletions .github/workflows/test.yml-template
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
170 changes: 5 additions & 165 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@
"keywords": [],
"author": "Mate Academy",
"license": "GPL-3.0",
"dependencies": {},
"devDependencies": {
"@linthtml/linthtml": "^0.9.6",
"@mate-academy/backstop-config": "latest",
"@mate-academy/bemlint": "latest",
"@mate-academy/linthtml-config": "latest",
"@mate-academy/scripts": "^1.8.6",
"@mate-academy/scripts": "^2.1.3",
"@mate-academy/stylelint-config": "latest",
"backstopjs": "6.3.23",
"jest": "^29.7.0",
Expand Down
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://sebpie0203.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://sebpie0203.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)
97 changes: 96 additions & 1 deletion src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,107 @@
content="ie=edge"
/>
<title>Moyo header</title>
<link
rel="preconnect"
href="https://fonts.googleapis.com"
/>
<link
rel="preconnect"
Comment on lines 12 to +19
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: "Code follows all the Code Style Rules ❗️". You are styling many tag names in a group selector (header, nav, ul, li, a, img) to reset margins/paddings (lines below). The Code Style rules require you to avoid using tag selectors (except html and body). For resets, set margins on html, body only. Replace this block with a minimal reset targeting html, body and use classes for other element-specific styles.

href="https://gstatic.com"
crossorigin="anonymous"
Comment on lines +18 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Preconnect target is incorrect: you preconnect to "https://gstatic.com" but the correct domain for Google Fonts static files is "https://fonts.gstatic.com". Update the preconnect href to fonts.gstatic.com (keep crossorigin as needed). This is part of the Google Fonts configuration checklist.

/>

<link
href="https://fonts.googleapis.com/css2?family=Roboto+Mono:ital,wght@0,100..700;1,100..700&family=Roboto:wght@500&display=swap"
rel="stylesheet"
Comment on lines +23 to +25
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: "The Google Fonts Configuration follows requirements." The font includes extra families (e.g. Roboto Mono and multiple styles). The task requires embedding ONLY Roboto (roman style), medium weight (500), normal width. Update the Google Fonts URL to request only Roboto with weight 500.

/>
<link
rel="stylesheet"
href="./style.css"
/>
</head>
Comment on lines +26 to 31
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 also violates checklist item #7. The box-sizing is applied to several tag names here. Per the previous review and the Code Style rules, set box-sizing: border-box on html and use inheritance (e.g. *, *::before, *::after { box-sizing: inherit; } is discouraged here — instead set html { box-sizing: border-box; } and if needed apply box-sizing: inherit to elements via a class). Move box-sizing to html rather than styling header, nav, ul, li, a directly.

<body>
<h1>Moyo header</h1>
<header class="header">
<a
href="./"
class="logo"
>
<img
src="./images/logo.png"
alt="Logo"
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 checklist states: "alt attribute should describe the image content". The image has alt="Logo", which is generic — use a more descriptive text (for example: "MOYO logo" or similar) to satisfy the accessibility/code-style requirement.

class="logo-img"
/>
</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"
Comment on lines +55 to +59
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You have a .logo-link rule here, but there is no element in the HTML using that class. Either remove this unused rule or apply the intended class in HTML. If you intended the logo to share the nav link height, ensure the actual .logo element has the correct height or use the same class name consistently.

>
samsung
</a>
</li>
<li class="nav-item">
<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>
</nav>
</header>
</body>
</html>
Loading
Loading