Skip to content
Open

PR #1999

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
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 start & sleep 5 && npm test
- name: Upload tests report(cypress mochaawesome merged HTML report)
if: ${{ always() }}
uses: actions/upload-artifact@v2
with:
name: report
path: reports
9 changes: 5 additions & 4 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"@linthtml/linthtml": "^0.9.6",
"@mate-academy/eslint-config": "latest",
"@mate-academy/linthtml-config": "latest",
"@mate-academy/scripts": "^1.8.5",
"@mate-academy/scripts": "^2.1.3",
"@mate-academy/stylelint-config": "latest",
"@parcel/transformer-sass": "^2.12.0",
"cypress": "^13.13.0",
Expand Down
2 changes: 1 addition & 1 deletion src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,6 @@ <h1>List of employees</h1>
Colleen Hurst
</li>
</ul>
<script src="scripts/main.js"></script>
<script src="scripts/main.js" defer></script>
</body>
</html>
41 changes: 40 additions & 1 deletion src/scripts/main.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,42 @@
'use strict';

// write code here
function getEmployees(pagesTag) {

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 function signature doesn't match requirements. The task specifies getEmployees should take a list property (the DOM list element). Currently the parameter is named pagesTag instead of list.

return Array.from(pagesTag).map((element) => {
return {
name: element.textContent.trim(),
position: element.dataset.position,
salary: element.dataset.salary,
age: element.dataset.age,
};
});
}

function sortList(employees) {

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 function signature doesn't match requirements. The task specifies sortList should take a list property (the DOM list element), not an array of employees. Currently, employee objects are passed instead of the DOM list.

return employees.sort((a, b) => {
const parseSalary = (salary) => {

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 helper function for salary conversion should be a separate standalone function (as mentioned in requirements), not defined inside sortList. Consider moving parseSalary outside the sorting function.

return parseFloat(salary.replace(/[$,]/g, ''));
};

return parseSalary(b.salary) - parseSalary(a.salary);
});
}

const listQuerry = document.querySelectorAll('li');
const list = getEmployees(listQuerry);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to requirements, sortList should sort and append DOM list items directly, not work with an already-converted employee array. The current flow (getEmployees → sortList → rebuild DOM) doesn't match the specified behavior where sortList should manipulate the DOM list directly.

const sortedEmployees = sortList(list);

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 sortList function is called without any arguments here (line 26), but according to requirements, it should be called with the DOM list element as parameter. Similarly, getEmployees on line 25 is called with listQuerry but should be called appropriately per the requirements.


const ul = document.querySelector('ul');

ul.innerHTML = '';

sortedEmployees.forEach((employee) => {
const li = document.createElement('li');

li.textContent = `${employee.name}`;

li.dataset.position = employee.position;
li.dataset.salary = employee.salary;
li.dataset.age = employee.age;

ul.appendChild(li);
});
Loading