Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is very close and achieves the correct visual result! However, this requires changes because the implementation doesn't quite match the task requirements for creating and adding elements.
The main issue is that the task requires you to create a <label> element programmatically using document.createElement('label') and then insert it into the document (for example, with element.parentElement.insertBefore()). Your current solution uses outerHTML to replace the input with an HTML string, which is a different approach.
Please update your script to create and insert the label elements as DOM nodes. Also, you can remove the unused label variable declared at the top of your script for a cleaner solution. You're on the right track!
✨ 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
|
|
||
| // placeholder="" | ||
| // write code here | ||
| const label = document.createElement('label'); |
There was a problem hiding this comment.
The label variable created here is never used to modify the form. The actual labels are generated as strings on line 11. This line, along with lines 13 and 14 that reference this variable, can be removed as they are not part of the solution.
src/scripts/main.js
Outdated
| // write code here | ||
| const label = document.createElement('label'); | ||
|
|
||
| const input = document.querySelectorAll('.field-text'); |
There was a problem hiding this comment.
The task asks you to get all inputs from the form tag. While selecting by class .field-text works for this specific page, a more precise selector like document.querySelectorAll('form .field-text') would be better. This ensures you only modify inputs that are actually inside a form, making your code more robust.
src/scripts/main.js
Outdated
|
|
||
| input.forEach((el) => { | ||
| el.placeholder = (el.name).replace(/\b\w/g, (l) => l.toUpperCase()); | ||
| el.outerHTML = `<label class="field-label" for="${el.id}">${(el.name).toUpperCase()}</label>${el.outerHTML}`; |
There was a problem hiding this comment.
While using outerHTML works to add the label, the task instructions ask you to 'create element label'. A more standard approach would be to use document.createElement('label') inside your loop. After creating the label element, you can set its properties and then insert it into the DOM before the input. Consider using a method like element.parentElement.insertBefore() for this.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job getting the placeholders to work correctly! The visual output looks perfect.
However, changes are needed because the core requirement of this task is to create and insert DOM elements programmatically, but the current solution still uses outerHTML. As mentioned in the previous feedback, using outerHTML is not permitted for this exercise.
Please update your code to follow this approach:
- Inside your
forEachloop, create a new label for each input usingconst newLabel = document.createElement('label');. - Set the
htmlForandtextContentfornewLabel. - Insert it into the document before the input element using
el.parentElement.insertBefore(newLabel, el).
You're very close to the solution! Just focus on using these specific DOM manipulation methods.
✨ 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
|
|
||
| // placeholder="" | ||
| // write code here | ||
| const label = document.createElement('label'); |
There was a problem hiding this comment.
This label is created only once, but a new one is needed for each input field. The logic to create a label should be inside the forEach loop. As it stands, this label (and the related code on line 13) is not used and has no effect on the final output.
src/scripts/main.js
Outdated
|
|
||
| input.forEach((el) => { | ||
| el.placeholder = el.name.replace(/\b\w/g, (l) => l.toUpperCase()); | ||
| el.outerHTML = `<label class="field-label" for="${el.id}">${el.name.toUpperCase()}</label>${el.outerHTML}`; |
There was a problem hiding this comment.
As mentioned in the previous feedback, the task requires you to avoid using outerHTML. The goal is to practice DOM manipulation by creating elements programmatically.
Please try this approach instead:
- Create a
labelelement usingdocument.createElement(). - Set its properties (
className,htmlFor,textContent). - Insert it into the DOM before the input using
el.parentElement.insertBefore(newLabel, el).
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 progress, but please review the feedback below carefully for your next task.
Great job replacing outerHTML with insertBefore and getting the placeholders to work perfectly! The main issue remaining is that the task requires creating a new <label> element for each input. Currently, you create one label outside the loop and then clone it.
To fix this, please move the label creation (document.createElement('label')) and class assignment (label.classList.add(...)) inside your forEach loop. This will ensure a unique element is created for each input, which is a key goal of this exercise, and will also simplify your code. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
src/scripts/main.js
Outdated
| const label = document.createElement('label'); | ||
| label.classList.add('field-label'); |
There was a problem hiding this comment.
These lines create a single label element that is then reused for all inputs. The task requires creating a new, separate label for each input field. Please move the label creation (document.createElement) and class assignment inside the forEach loop.
src/scripts/main.js
Outdated
|
|
||
| el.placeholder = el.name.replace(/\b\w/g, (l) => l.toUpperCase()); | ||
| label.htmlFor = el.id; | ||
| el.parentElement.insertBefore(label.cloneNode(label.textContent = el.name), el); |
There was a problem hiding this comment.
This line is quite complex because it's modifying, cloning, and inserting the label all at once. If you create a new label inside the loop as suggested, you can simplify this part significantly. You would just need to insert the newly created label element.
No description provided.