Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a few checklist-required behaviors are still not met and they affect core functionality and tests. First, you are selecting inputs with document.getElementsByTagName('input') instead of selecting them from the form element (e.g. const form = document.querySelector('form'); const inputs = form.querySelectorAll('input');), which violates the requirement to scope inputs to the form [CHECKLIST ITEM #11]. Second, the label class is added as label.classList.add('.field-label'), which creates a class literally named .field-label instead of field-label; this breaks expected styling and tests that look for the exact class name [CHECKLIST ITEMS #13 and #21]. Third, you are not setting the for attribute on each label to match the corresponding input id, so labels are not properly linked for accessibility or for the tests [CHECKLIST ITEM #14]; ensure each input has an id (you can derive it from name if needed) and then do label.htmlFor = input.id. Finally, double-check that your label text format matches what the tests expect (you currently use toUpperCase(); the requirement is just that it is based on the name, likely in a capitalized, human-readable form) and remember that only main.js should be changed for this task. Once you address these issues and re-run the tests, your solution should be in good shape.
✨ 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.getElementsByTagName('input'); |
There was a problem hiding this comment.
You're selecting all inputs from the document with getElementsByTagName('input'). The task requires getting inputs from the form tag specifically (checklist item #11). Select the form first (e.g. const form = document.querySelector('form')) and then get its inputs (form.querySelectorAll('input')).
src/scripts/main.js
Outdated
| const inputs = document.getElementsByTagName('input'); | ||
|
|
||
| [...inputs].forEach((input) => { | ||
| const label = document.createElement('label'); |
There was a problem hiding this comment.
After creating the label you must set its for attribute to the corresponding input id (checklist item #14). Add label.htmlFor = input.id or label.setAttribute('for', input.id) before appending the label.
src/scripts/main.js
Outdated
| [...inputs].forEach((input) => { | ||
| const label = document.createElement('label'); | ||
|
|
||
| label.classList.add('.field-label'); |
src/scripts/main.js
Outdated
|
|
||
| const inputName = input.name.replace(/([A-Z])/g, ' $1'); | ||
|
|
||
| label.textContent = inputName.toUpperCase(); |
There was a problem hiding this comment.
You set label.textContent = inputName.toUpperCase(). The checklist only requires the label be based on the input name (item #4) — confirm whether tests expect UPPERCASE text; if not, consider using a capitalized form instead to match the expected label text.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is still a critical accessibility and requirements issue with the label–input linkage. Your current code sets label.htmlFor = input.id, but it never guarantees that input.id actually exists; this violates the requirement that each label’s for attribute must match a real input id and will cause tests to fail [CHECKLIST ITEM #14]. To fix this, before assigning label.htmlFor, ensure each input has a stable id, for example on the line where you process each input: input.id = input.id || input.name; (or a normalized version derived from input.name), then set label.htmlFor = input.id.
There is also a medium-severity robustness issue: the string you build from input.name using replace(/([A-Z])/g, ' $1') can start with a space when the name begins with an uppercase letter, which leads to a leading space in the label and placeholder text. On that line (around line 11), trim the result before capitalizing, e.g. const inputName = input.name.replace(/([A-Z])/g, ' $1').trim();, so the resulting fieldName is consistently human-readable. Once you add the id-assignment step and the trim for the label text, your implementation should fully meet the task requirements, so please make these updates and re-run the tests. Overall the structure of your solution is solid; these adjustments will bring it in line with both accessibility best practices and the automated checks.
✨ 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
| const fieldName = `${inputName[0].toUpperCase()}${inputName.substring(1)}`; | ||
|
|
||
| label.textContent = fieldName; | ||
| label.htmlFor = input.id; |
There was a problem hiding this comment.
Critical: you set label.htmlFor = input.id but there is no guarantee input.id exists. Tests expect the label's for to match the input's id (CHECKLIST ITEM #14). Add logic to ensure each input has an id (for example derive one from input.name when missing) before assigning label.htmlFor.
src/scripts/main.js
Outdated
|
|
||
| label.classList.add('field-label'); | ||
|
|
||
| const inputName = input.name.replace(/([A-Z])/g, ' $1'); |
There was a problem hiding this comment.
Medium: the inputName value (line 11) can get a leading space when input.name starts with an uppercase letter because of the regex; trim the result before capitalizing to avoid a leading space in fieldName. e.g. const inputName = input.name.replace(/([A-Z])/g, ' $1').trim();
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution under an educational override so you can keep progressing while you learn, even though there are a couple of important issues to address. You did a solid job looping through each form’s inputs, creating labels with the field-label class, setting label.textContent and input.placeholder from input.name, and inserting the label before the input as required. The main high-severity gap is that you assign label.htmlFor = input.id without first guaranteeing that input.id exists; to fix this, at the point where you handle each input (e.g., around the line where you set htmlFor), add something like input.id = input.id || input.name.replace(/\s+/g, '-').toLowerCase(); so every label’s for is linked to a real, stable id. A second issue is that you build fieldName using inputName[0].toUpperCase() without checking if input.name might be empty; adding a guard such as const fieldName = inputName ? inputName[0].toUpperCase() + inputName.substring(1) : ''; (or by skipping inputs with no name) will prevent runtime errors. Also remember that the exercise expects only src/scripts/main.js to change, so be sure to revert any edits you may have made to other files like README.md before final submission. Overall, your core approach is correct and close to the expectations; with these small robustness and requirement-alignment fixes, your solution will be both more accessible and fully compliant with the task. Keep going—these are exactly the kinds of edge cases that sharpen your JavaScript and frontend skills.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const fieldName = `${inputName[0].toUpperCase()}${inputName.substring(1)}`; | ||
|
|
||
| label.textContent = fieldName; | ||
| label.htmlFor = input.id; |
There was a problem hiding this comment.
High-severity: you set label.htmlFor = input.id but the code never ensures input.id exists. Before this line assign a stable id to the input (derived from input.name) and then set label.htmlFor. Example: input.id = input.id || input.name; or a normalized variant such as input.id = input.id || input.name.replace(/\s+/g, '-').toLowerCase(); so the for attribute always matches a real input id.
| label.classList.add('field-label'); | ||
|
|
||
| const inputName = input.name.replace(/([A-Z])/g, ' $1').trim(); | ||
| const fieldName = `${inputName[0].toUpperCase()}${inputName.substring(1)}`; |
There was a problem hiding this comment.
Medium: fieldName is built using inputName[0].toUpperCase() which will throw if input.name (hence inputName) is empty. Add a small guard/default when inputName is empty, e.g. const fieldName = inputName ? inputName[0].toUpperCase() + inputName.substring(1) : ''; or skip inputs without a name.
DEMO LINK