Develop

[MykolaFatkullin]

• DEMO LINK

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because a few core requirements of the task are not yet met and will likely cause tests to fail. First, in src/scripts/main.js, getEmployees currently passes the salary string directly into the Employee constructor and returns Employee instances; the task requires returning plain objects with the schema { name, position, salary, age }, and salary (and likely age) must be Numbers, so you should use convertSalary or Number() before creating the returned objects. Second, name is derived with element.textContent.trim(), which risks including position, salary, and age text from the same <li>; instead, query a specific child (e.g. element.querySelector('.name').textContent) or use a dedicated data attribute so only the employee name is captured. Third, sortList(list) ignores its list parameter and appends items using a global ul; update it to append to the passed list (or its parent, depending on the expected signature) so the function matches the requirement and doesn’t rely on globals. These points are functional, not stylistic: they directly affect the returned data shape, types, and how the DOM is manipulated, so they are critical for passing the automated checks. Once you adjust these aspects, your overall approach (DOM selection, sorting by dataset salary, and having a salary conversion helper) will fit the task requirements well.

---

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

You pass the salary string directly into the Employee constructor. The task requires converting salary from the data attribute to a number. Use the existing convertSalary helper (or Number) so salary becomes a Number before creating the object. This violates the technical constraint about salary conversion.

[mateacademy-ai-mentor]

Using element.textContent.trim() for the name may include position, salary, and age text from the same

. Extract the name from the specific child element (e.g. element.querySelector('.name').textContent) or from a dedicated data attribute so the name property contains only the employee name as required by the schema.

[mateacademy-ai-mentor]

sortList appends sorted items with the global ul variable instead of using the passed list parameter. Consider appending to the list's parent (e.g. list[0].parentElement.appendChild(...)) or accepting the actual <ul> element to avoid relying on globals and to match the function signature requirement.

[mateacademy-ai-mentor]

getEmployees returns instances of the Employee class. Tests often expect plain objects with properties name, position, salary, age. Consider returning plain object literals (with salary as a Number) to ensure compatibility with the expected schema.

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because there are still a few critical issues that will cause tests to fail and prevent the task from being fully completed.

Critical issues to fix:

1. In src/scripts/main.js, sortList(list) currently does element.parentElement = ul, which does not move DOM nodes and also uses the global ul instead of the list parameter. You need to append the sorted <li> elements back to the passed-in list, for example:

   sortedItems.forEach(element => list.appendChild(element));

   so the actual DOM order is updated.
2. The call order should be adjusted so that if the employees array is expected to reflect the sorted list, you invoke sortList(list) before getEmployees(list) (right now they are called in the opposite order).
3. getEmployees(list) is returning new Employee(...) instances, but the requirement is to return plain objects of the form { name, position, salary, age } with salary and age as Numbers; change the return value to plain literals like { name, position, salary, age } and ensure age is converted to a number (e.g. Number(element.dataset.age)) and salary uses your convertSalary helper.
4. Your name field is currently derived with element.textContent.trim(), which can include position, salary, and age text from the same <li>; instead, select just the specific child element for the name, such as element.querySelector('.name').textContent.trim().

Minor note: The Employee class is not needed once you switch to plain objects, so you can remove it to simplify the code. Your overall approach is close; with these corrections, your solution should meet the checklist requirements and pass the automated tests.

---

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

sortList currently does .forEach((element) => element.parentElement = ul). parentElement is read-only and assigning to it will not move nodes. Also the function must use its list parameter instead of the global ul. Append sorted items back to the passed list, e.g. list.appendChild(element).

[mateacademy-ai-mentor]

Both functions are called (good). If the expected employees array should reflect the sorted order, call sortList(li) before getEmployees(li). The task requires calling both functions, but adjust order if the tests expect the returned array to be sorted.

[mateacademy-ai-mentor]

getEmployees returns instances of Employee (new Employee(...)). The task requires returning plain objects matching the schema { name, position, salary, age }. Return plain objects (not class instances) and ensure numeric fields are Numbers (see next comment).

[mateacademy-ai-mentor]

Using element.textContent.trim() for name can include other text from the same <li> (position, salary, age). Select the specific child that holds the name, e.g. element.querySelector('.name').textContent.trim(), so name contains only the employee name.

[mateacademy-ai-mentor]

element.dataset.age is used directly; convert age to a Number before returning (e.g. Number(element.dataset.age) or parseInt(...)) so age has the correct type. Likewise ensure salary is a Number (use convertSalary).

[MykolaFatkullin]

trim() function should be present, In html name stores in text content, not in dataset

[mateacademy-ai-mentor]

Review Summary

I am approving your solution as an educational override so you can keep progressing while you practice these concepts.

You’ve done a solid job implementing the salary parsing helper and using dataset for sorting, and both sortList and getEmployees are present and being called in the right order. The main issues to address next are: (1) pass the ul element itself into sortList(ul) and getEmployees(ul) instead of ul.children, and make sortList work only with the passed-in list and list.appendChild(...) [CHECKLIST ITEM #13, #16, #19]; (2) have getEmployees return plain objects like { name, position, salary, age } rather than new Employee(...), and remove the now-unnecessary Employee class [CHECKLIST ITEM #4, #5, #6, #21, #24]; and (3) read name from element.querySelector('.name').textContent.trim() and ensure salary uses your convertNumber helper while age is converted with Number(element.dataset.age) [CHECKLIST ITEM #22, #23]. As a smaller polish task, remember to fix your README demo link by replacing <your_account> with your actual GitHub username and removing stray backticks.

Overall, you’re very close: the structure is there and only a few contract/details issues are blocking full compliance, which makes this a great opportunity to strengthen your understanding of DOM manipulation and data shaping.

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Footnotes

1. Rate AI review example ↩

[mateacademy-ai-mentor]

const li = ul.children; gives an HTMLCollection. The functions should receive the list element (the ul) per the task (Checklist #10, #13). Prefer passing ul into your functions and iterate list.children inside them.

[mateacademy-ai-mentor]

You're calling the functions with li (HTMLCollection). Pass the ul element instead: sortList(ul); const employees = getEmployees(ul);. Keep the call order as sortList before getEmployees so the returned array reflects the sorted list (Checklist #16, #20).

[mateacademy-ai-mentor]

sortList currently spreads list and then appends nodes to the global ul. The function must operate on the passed-in list parameter and append sorted items back to it. Do this pattern inside the function: const items = [...list.children]; items.sort(...); items.forEach(item => list.appendChild(item));. Replace ul.appendChild(element) with list.appendChild(element) (Checklist #13, #16, #19).

[mateacademy-ai-mentor]

getEmployees maps DOM elements to new Employee(...). The task requires returning plain object literals of the shape { name, position, salary, age } (Checklist #4, #5, #6, #21). Also, name should be taken from the specific child element (e.g. element.querySelector('.name').textContent.trim()), not from element.textContent.trim() which includes other data (Checklist #23). Ensure salary uses your convertNumber helper and age is converted to a Number (e.g. Number(element.dataset.age)) (Checklist #22).

[mateacademy-ai-mentor]

The Employee class is unnecessary once you return plain objects — remove it to simplify the code (Checklist #21, #24).