Skip to content

add task solution#1840

Open
Ivadriil wants to merge 7 commits intomate-academy:masterfrom
Ivadriil:develop
Open

add task solution#1840
Ivadriil wants to merge 7 commits intomate-academy:masterfrom
Ivadriil:develop

Conversation

@Ivadriil
Copy link

No description provided.

Copy link

@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

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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example


// placeholder=""
// write code here
const label = document.createElement('label');

Choose a reason for hiding this comment

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

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.

// write code here
const label = document.createElement('label');

const input = document.querySelectorAll('.field-text');

Choose a reason for hiding this comment

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

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.


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}`;

Choose a reason for hiding this comment

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

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.

Copy link

@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

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:

  1. Inside your forEach loop, create a new label for each input using const newLabel = document.createElement('label');.
  2. Set the htmlFor and textContent for newLabel.
  3. 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example


// placeholder=""
// write code here
const label = document.createElement('label');

Choose a reason for hiding this comment

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

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.


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}`;

Choose a reason for hiding this comment

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

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:

  1. Create a label element using document.createElement().
  2. Set its properties (className, htmlFor, textContent).
  3. Insert it into the DOM before the input using el.parentElement.insertBefore(newLabel, el).

Copy link

@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 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

  1. Rate AI review example

Comment on lines +6 to +7
const label = document.createElement('label');
label.classList.add('field-label');

Choose a reason for hiding this comment

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

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.


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);

Choose a reason for hiding this comment

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

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.

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