task complete#1273
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this task! All the requirements have been met: the buttons for adding and removing rows and columns work as expected, and they are correctly disabled at the minimum and maximum limits. I am approving your solution. As a point of improvement, consider making your row counting more robust. You are currently using table.rows.length for checks, but manipulating rows in table.tBodies[0]. Using table.tBodies[0].rows.length consistently for both checking and manipulation would prevent potential bugs if a <thead> element were added to the table. This is a great habit for writing more resilient code. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const removeColumnButton = document.querySelector('.remove-column'); | ||
|
|
||
| appendRowButton.addEventListener('click', (button) => { | ||
| if (table.rows.length < 10) { |
There was a problem hiding this comment.
When checking the number of rows, you're using table.rows, which includes any header (<thead>) or footer (<tfoot>) rows. However, you are adding rows specifically to table.tBodies[0]. To avoid bugs if the table structure changes, it's safer to count rows from the same place you are modifying them. Consider using table.tBodies[0].rows.length instead.
| table.tBodies[0].appendChild(row); | ||
| } | ||
|
|
||
| if (table.rows.length === 10) { |
There was a problem hiding this comment.
This check for the maximum row count has the same issue as the one on line 11. It should also use table.tBodies[0].rows.length to be consistent and avoid potential bugs with tables that have a header.
| }); | ||
|
|
||
| removeRowButton.addEventListener('click', (button) => { | ||
| if (table.rows.length > 2) { |
There was a problem hiding this comment.
The row count check here should be consistent with the row manipulation logic. Please use table.tBodies[0].rows.length to ensure the logic works correctly even if the table has a <thead>.
| table.tBodies[0].deleteRow(-1); | ||
| } | ||
|
|
||
| if (table.rows.length === 2) { |
There was a problem hiding this comment.
This check for the minimum number of rows should also use table.tBodies[0].rows.length for consistency with the DOM manipulation and the other checks.
2 participants
No description provided.