Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because one core requirement is not yet met and it can break functionality. In src/scripts/main.js you currently use document.querySelector('tbody'), which does not specifically target the table with class dashboard as required; this can attach rows to the wrong table and fails the explicit requirement to “find a table with class dashboard in the document.” Please update the selector to something like document.querySelector('.dashboard tbody') or first select the table via document.querySelector('table.dashboard') and then its tbody. Also, you append rows to tbody without checking if the query actually found the element; add a null-check (for example, return early or log an error) so you don’t risk a runtime error when .dashboard (or its tbody) is missing. Apart from this, your row creation, age calculation, and century calculation all look correct and aligned with the task. Once you fix the selector to target .dashboard and add the guard around tbody, your solution should fully satisfy the functional requirements.
✨ 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
src/scripts/main.js
Outdated
| console.log(people); // you can remove it | ||
|
|
||
| // write your code here | ||
| const tbody = document.querySelector('tbody'); |
There was a problem hiding this comment.
This selects a generic tbody instead of the table with class dashboard. Requirement: "Find a table with class dashboard in the document." Replace this with a selector that targets the dashboard table, for example document.querySelector('.dashboard tbody') or first document.querySelector('table.dashboard') then its tbody.
| people.forEach((person) => { | ||
| const tr = document.createElement('tr'); | ||
|
|
||
| tbody.append(tr); |
There was a problem hiding this comment.
Appending directly to tbody assumes the selector succeeded. Add a null-check before using tbody (or select the table and create/locate its tbody) to avoid runtime errors when the element isn't present.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because all core requirements are correctly implemented: you build rows from the people array, create six cells in the correct order, and compute age and century accurately before appending to .dashboard tbody. The remaining concern is about how you handle the null-check for const tbody = document.querySelector('.dashboard tbody'): doing the check inside the forEach callback and using return 'ERROR' there doesn’t actually stop the loop or the outer function and is less clear than checking once up front. As an improvement, move the if (!tbody) check to immediately after the querySelector line, log something like console.error('Table with class "dashboard" not found'), and return; early from the main script/function so the forEach is never called when the table is missing. That change will make your guard more explicit and align better with [CHECKLIST ITEM #17], but it’s not blocking functionality in the happy path where the table exists. Overall, the table is populated correctly, calculations are right, and the selector issue from before is resolved—nice work refining this solution.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| console.log(people); // you can remove it | ||
|
|
||
| // write your code here | ||
| const tbody = document.querySelector('.dashboard tbody'); |
There was a problem hiding this comment.
You correctly select the dashboard tbody using document.querySelector('.dashboard tbody'). Move the existence check here (right after this line) so you validate once before processing rows instead of inside the loop. This will satisfy checklist item #17 more explicitly.
| if (tbody === null) { | ||
| return 'ERROR'; |
There was a problem hiding this comment.
Avoid returning a string from inside a forEach callback. return 'ERROR' on line 366 does not stop the outer function or the loop. Instead, check tbody before the loop and if it's missing do console.error('Table with class "dashboard" not found') and return; to exit early.
No description provided.