add solution#1821
Conversation
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 structural bug that breaks the table layout. The table.append(tr) call is currently placed inside your cell-creation loop (around lines 367, 373, 379, 385, 391, 397), meaning you're appending the incomplete row to the table after each individual cell instead of once after all 6 cells are complete. This causes each <td> to become a separate row in the DOM. You need to move table.append(tr) to line 398, after the final tdCentury is appended to tr. This is why the test can't find the 'Male' content - the table structure is malformed. Fix this by moving the row append call outside your loop, and your table should render correctly.
✨ 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
| tr.append(tdName); | ||
| table.append(tr); | ||
|
|
||
| const tdSex = document.createElement('td'); | ||
|
|
||
| tdSex.textContent = person.sex; | ||
| tr.append(tdSex); | ||
| table.append(tr); | ||
|
|
||
| const tdBorn = document.createElement('td'); | ||
|
|
||
| tdBorn.textContent = person.born; | ||
| tr.append(tdBorn); | ||
| table.append(tr); | ||
|
|
||
| const tdDied = document.createElement('td'); | ||
|
|
||
| tdDied.textContent = person.died; | ||
| tr.append(tdDied); | ||
| table.append(tr); | ||
|
|
||
| const tdAge = document.createElement('td'); | ||
|
|
||
| tdAge.textContent = person.died - person.born; | ||
| tr.append(tdAge); | ||
| table.append(tr); | ||
|
|
||
| const tdCentury = document.createElement('td'); | ||
|
|
||
| tdCentury.textContent = Math.ceil(person.died / 100); | ||
| tr.append(tdCentury); |
There was a problem hiding this comment.
The row is being appended to the table after EACH cell (lines 367, 373, 379, 385, 391, 397). This causes the row to move to the table after the first cell, and all subsequent cells are appended directly to the table instead of the row. Move table.append(tr) to line 398, after all 6 cells are appended to the row.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes to fix a critical structural bug in the table creation logic. The table.append(tr) call is currently placed inside the cell-creation loop (around lines 367, 378, 384, 390, 396, and 402), which means it's being called after each individual cell is appended to the row. This creates a malformed table where each <td> becomes a separate row instead of all 6 cells being in a single row.
The fix is straightforward: Remove all table.append(tr) calls from inside the loop, and add a single table.append(tr) once, after the final tr.append(tdCentury) call completes the row. The row should only be appended to the table after all 6 cells (name, gender, born, died, age, century) have been added to it.
The rest of your implementation—calculating age as died - born, calculating century as Math.ceil(died / 100), and selecting the dashboard table—looks correct. Once you move the table.append(tr) outside the loop, the table structure will be properly formed and tests should pass.
✨ 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 tdName = document.createElement('td'); | ||
|
|
||
| tdName.textContent = person.name; | ||
| tr.append(tdName); |
There was a problem hiding this comment.
Move this table.append(tr) outside the cell-creation loop. It should be deleted here and called only once after the final tdCentury is appended to tr.
| tdSex.textContent = 'Female'; | ||
| } | ||
|
|
||
| tr.append(tdSex); |
There was a problem hiding this comment.
Move this table.append(tr) outside the cell-creation loop. It should be deleted here and called only once after the final tdCentury is appended to tr.
| const tdBorn = document.createElement('td'); | ||
|
|
||
| tdBorn.textContent = person.born; | ||
| tr.append(tdBorn); |
There was a problem hiding this comment.
Move this table.append(tr) outside the cell-creation loop. It should be deleted here and called only once after the final tdCentury is appended to tr.
| const tdDied = document.createElement('td'); | ||
|
|
||
| tdDied.textContent = person.died; | ||
| tr.append(tdDied); |
There was a problem hiding this comment.
Move this table.append(tr) outside the cell-creation loop. It should be deleted here and called only once after the final tdCentury is appended to tr.
| const tdAge = document.createElement('td'); | ||
|
|
||
| tdAge.textContent = person.died - person.born; | ||
| tr.append(tdAge); |
There was a problem hiding this comment.
Move this table.append(tr) outside the cell-creation loop. It should be deleted here and called only once after the final tdCentury is appended to tr.
| const tdCentury = document.createElement('td'); | ||
|
|
||
| tdCentury.textContent = Math.ceil(person.died / 100); | ||
| tr.append(tdCentury); |
There was a problem hiding this comment.
Keep only ONE table.append(tr) here at the end of the loop, after tr.append(tdCentury). Delete all the other table.append(tr) calls (lines 367, 378, 384, 390, 396).
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Educational Override: While this review would typically require changes before approval, I'm approving your work to support your learning journey.
You've done a great job implementing the core logic—calculating age and century correctly, creating all 6 cells per row, and using proper DOM methods. However, there's still one critical issue to fix: line 383 contains a table.append(tr) that needs to be removed. This line currently appends the row after only 3 cells (name, sex, born), creating malformed rows. The single table.append(tr) at line 398 (after all 6 cells) should be the only append in the loop. Once you remove that one line, your table generation will work perfectly. Keep practicing—you're very close to getting this completely right!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| tdBorn.textContent = person.born; | ||
| tr.append(tdBorn); | ||
| table.append(tr); |
There was a problem hiding this comment.
Remove this line. The row is being appended prematurely after only 3 cells (name, sex, born). The row should only be appended to the table after ALL 6 cells are added to it.
| const tdCentury = document.createElement('td'); | ||
|
|
||
| tdCentury.textContent = Math.ceil(person.died / 100); | ||
| tr.append(tdCentury); |
There was a problem hiding this comment.
Keep this line, but ensure it's the ONLY table.append(tr) in the loop. After removing line 383, this will be the correct place to append the complete row (with all 6 cells) to the table.
2 participants
No description provided.