add task solution#1868
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a couple of core requirements are not yet fully met. In src/scripts/main.js, the line where you select inputs using document.querySelectorAll('input') (near the top of the script) grabs every input in the document instead of only those inside the <form>, which violates the requirement to get all input elements from the form tag; you should instead scope the selector to the form, for example with document.querySelector('form').querySelectorAll('input') or document.querySelectorAll('form input'). Another critical requirement that is still missing is updating the demo link by replacing <your_account> with your GitHub username in the appropriate file (likely the README or description), so that the provided demo link actually points to your project. There is also a minor issue in src/scripts/main.js where 'use strict' appears twice at the top; one of these can be safely removed because strict mode only needs to be enabled once. Once you scope the input selection to the form, update the demo link placeholder, and remove the redundant 'use strict', your solution will align well with all of the specified checklist items. You’ve done a solid job with label creation, class assignment, and using capitalized input.name for both labels and placeholders—fixing these last details will make the implementation complete.
✨ 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
| 'use strict'; | ||
| 'use strict'; | ||
|
|
||
| const inputs = document.querySelectorAll('input'); |
There was a problem hiding this comment.
This line selects every input in the document, but the task requires getting inputs from the form tag specifically. Consider using document.querySelector('form').querySelectorAll('input') or document.querySelectorAll('form input') so only form inputs are processed.
| @@ -1,3 +1,21 @@ | |||
| 'use strict'; | |||
| 'use strict'; | |||
There was a problem hiding this comment.
There's a duplicate 'use strict' directive here — it's redundant and can be removed (the first line already enables strict mode).
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Changes are needed before this task can be considered complete, but your core JavaScript implementation in src/scripts/main.js looks solid and meets the functional requirements (labels created per input, correct for/id linkage, placeholders based on name, and proper DOM insertion as siblings).
The critical issue is in your project description/README: on the DEMO LINK line (around line 5 in README.md), the URL still uses the placeholder https://<your_account>.github.io/js_task_fix_form_DOM/ instead of your actual GitHub username, which violates the task requirement to provide a working demo link and prevents reviewers from accessing your deployed solution.
To fix this, replace <your_account> in that URL with your real GitHub username so it becomes something like https://your-github-username.github.io/js_task_fix_form_DOM/ and ensure the link opens your project page.
As a minor (non-blocking) improvement in main.js, you could also handle inputs that lack an id by generating one (e.g., input.id = 'input-' + counter) before setting label.htmlFor = input.id, so every label is properly associated even if the HTML is incomplete.
Once you update the demo link in the README, your solution will fully satisfy the task requirements; your DOM logic is well structured, so you’re very close—just fix the link and you should be good to go.
✨ 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
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you polish a few remaining details. Your src/scripts/main.js implementation does a strong job of creating labels, assigning the field-label class, and setting capitalized placeholders based on input.name, which aligns well with the core requirements [CHECKLIST ITEMS #1–3, #12–20].
Critical issues to fix next:
- In
README.md, update the demo URL to use your actual GitHub username instead of the<your_account>placeholder so reviewers can access your deployed demo and you satisfy [CHECKLIST ITEM #7]. - In
src/scripts/main.js, around the block where you create each label (roughly lines 15–30), if an input is missing anid,label.setAttribute('for', input.id);results in an empty association, so consider first generating anid(for exampleinput.id = 'input-' + index) and then usinglabel.htmlFor = input.idto fully meet [CHECKLIST ITEM #15].
Minor improvements:
- Make sure you run
npm run test,npm run test:only -- -n, andnpm run test:only -- -land verify the results so you cover [CHECKLIST ITEMS #9–11], and consider the small idiomatic tweak of usinglabel.htmlForinstead ofsetAttribute('for', ...)for clarity.
Overall, your DOM manipulation logic is well-structured and very close to production-ready quality; addressing these few points will make your solution even more robust and complete.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.