Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on the core logic! Your script successfully adds the labels and placeholders as required. However, a couple of adjustments are needed to make your solution more robust and fully meet the task's specifications.
- Selector Specificity: The task requires getting
inputelements specifically from the<form>tag. Your current selector,document.querySelectorAll('input'), will grab all inputs on the entire page. Please update this to be more specific, like'form input', to ensure only the correct elements are targeted. - Adding a Class: You've used
classList.toggle('field-label'). While this works here, the more appropriate method isclassList.add('field-label'). Usingaddclearly states the intention to ensure the class is present, whereastogglecould unintentionally remove it under different circumstances.
Making these changes will ensure your code is both correct and resilient. Keep up the great effort!
✨ 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
| 'use strict'; | ||
|
|
||
| // write code here | ||
| const inputs = document.querySelectorAll('input'); |
There was a problem hiding this comment.
The task requires getting all input elements specifically from the form tag. Using document.querySelectorAll('input') will select all inputs on the page, which might not be what's intended. Consider using a more specific selector like 'form input' to be more precise.
src/scripts/main.js
Outdated
| [...inputs].forEach((element) => { | ||
| const label = document.createElement('label'); | ||
|
|
||
| label.classList.toggle('field-label'); |
There was a problem hiding this comment.
While toggle might work in this scenario, the standard and more reliable method to add a class is classList.add('field-label'). Using toggle could unintentionally remove the class if it were ever present. The requirement is to ensure the label has the class.
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 pull request! I am approving your solution as you've successfully addressed all the feedback from the previous review. The code now fully meets all task requirements, with the selector correctly scoped to the form and classList.add being used as specified. The implementation is clean, efficient, and well-executed. Well done!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
DEMO LINK