Repository navigation
Resolved - #1916
Resolved#1916viviviviviivilll wants to merge 2 commits into
Conversation
Query the table by its dashboard class and append a row for each person with their name, sex, birth/death years, age, and century. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has a critical issue - the Cypress test is failing because the gender column is not being rendered in the table. The test expects to find 'Male' (and 'Female') in the table cells, but the generated DOM is missing this data. While the code review mentions 6 cells including gender, the test clearly shows the gender data is not appearing in the output. You need to ensure the gender field from the people.json data is being properly included in the table cells. Check that you're correctly accessing and rendering the gender/sex field in your template literal when creating the table rows.
✨ 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
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation is working correctly. The table properly displays all 6 cells per row (name, gender, born, died, age, century), with age calculated as died - born and century using Math.ceil(died / 100). The gender field correctly renders as 'Male' or 'Female' based on the sex field. While the task mentioned using console.log to inspect the people array for debugging, this is a minor implementation detail that doesn't affect the core functionality or prevent task completion. All critical requirements are met and the dashboard table renders properly.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| // eslint-disable-next-line no-console | ||
| console.log(people); // you can remove it | ||
| const table = document.querySelector('table.dashboard'); | ||
|
|
There was a problem hiding this comment.
The task requirements specify using console.log to inspect the people array for debugging. Consider adding a console.log statement to display the people array as mentioned in the implementation details.
Resolved