finish#1979
Conversation
IronHedgehog
commented
Apr 28, 2026
- DEMO LINK
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The implementation has critical blockers that prevent task completion. The getEmployees function must return an array of employee objects with {name, position, salary, age} properties extracted from list item data attributes (li.dataset.name, li.dataset.position, etc.) — currently it only manipulates DOM and returns undefined. The sortList function must append sorted items to the list as specified in the requirements, but it currently only returns a sorted array without performing any DOM updates. These are core requirements that are not being met. Please fix both functions so getEmployees extracts and returns employee data, and sortList appends the sorted results to the list DOM.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| const sortList = (list) => { | ||
| return list.sort( | ||
| (a, b) => | ||
| normalizeSalary(b.dataset.salary) - normalizeSalary(a.dataset.salary), | ||
| ); |
There was a problem hiding this comment.
The sortList function should append sorted items to the list, but currently it only returns a sorted array. The appending logic is missing here.
| const getEmployees = (list) => { | ||
| employeeList.innerHTML = ''; | ||
| list.forEach((li) => employeeList.append(li)); |
There was a problem hiding this comment.
The getEmployees function should return an array of employee objects with properties {name, position, salary, age}. Currently it only manipulates DOM and returns undefined. You need to extract data from each list item using li.dataset.name, li.dataset.position, etc.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation successfully fulfills all core requirements for the sortList and getEmployees functions. The sorting logic correctly handles salary as numbers (using the normalizeSalary helper), sorts in descending order, appends results to the DOM, and getEmployees returns a properly structured array of employee objects matching the required schema {name, position, salary, age}.
The name property using textContent while other properties use dataset attributes is noted as a minor inconsistency in the review. This is not a critical blocker—your current implementation will work correctly if the HTML has name as text content rather than a data attribute. If you want to ensure full consistency with data attribute parsing, you could change item.textContent to item.dataset.name.trim() for the name property, but this is optional improvement rather than a required fix.
All functional requirements are met and the code is ready for use.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| const getEmployees = (list) => { | ||
| return list.map((employee) => { | ||
| return { |
There was a problem hiding this comment.
The name property uses textContent while the task requires parsing from data attributes. All other employee properties use dataset.*, so name should also use employee.dataset.name.trim() for consistency with the data attribute parsing requirement.