Skip to content

js_employees_table_DOM_solution#1088

Open
Banderos14 wants to merge 3 commits into
mate-academy:masterfrom
Banderos14:develop
Open

js_employees_table_DOM_solution#1088
Banderos14 wants to merge 3 commits into
mate-academy:masterfrom
Banderos14:develop

Conversation

@Banderos14
Copy link
Copy Markdown

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Changes are needed because two critical requirements are unimplemented: the cell editing feature (double-click to edit a table cell, show an input with class cell-input, save on blur or Enter, and allow only one cell to be edited at a time) and validation for the 'office' field, which is explicitly required to be non-empty per the task. The showNotification function also has a scope issue — it's defined inside the form submit handler but called earlier in validation logic, which will cause a runtime ReferenceError. These are blockers: without cell editing, a core interactive requirement is missing; without office validation, the 'all fields required' rule is violated. Please implement these features and fix the function scope before resubmission.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/scripts/main.js
Comment thread src/scripts/main.js
Comment thread src/scripts/main.js
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Changes are needed before this submission can be approved. The office validation is unreachable because the <select> element lacks an empty default option — currently, 'Tokyo' is always selected, so the if (!office) check at line 205 never triggers. Add <option value="">Select office</option> as the first option to make validation testable and functional. Additionally, the cell editing feature is missing, which is required per the task checklist: double-clicking a cell must replace its text with an editable <input class="cell-input">, support single-cell editing only, save on blur or Enter, and restore the original value if the input is left empty. Both issues are critical blockers — one breaks validation logic, and the other omits a core requirement. Please implement these fixes and resubmit.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/scripts/main.js
Comment thread src/scripts/main.js
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution — excellent work implementing all requirements! You handled table sorting with clean toggle logic, row selection with proper class management, and form validation with thoughtful UX (including the crucial empty default option for the office select). Cell editing via double-click works perfectly: it respects single-cell editing, saves on blur/Enter, and gracefully restores values when empty. Your salary formatting ($X,XXX) and numeric input types for age/salary show strong attention to detail. This is a robust, production-ready implementation that demonstrates solid JavaScript and DOM manipulation skills. Keep up the great work!


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

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants