Skip to content

add task solution#1818

Open
nols212 wants to merge 2 commits intomate-academy:masterfrom
nols212:develop
Open

add task solution#1818
nols212 wants to merge 2 commits intomate-academy:masterfrom
nols212:develop

Conversation

@nols212
Copy link

@nols212 nols212 commented Feb 19, 2026

Replace <your_account> with your Github username in the link
- DEMO LINK

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 requesting changes because there are still a couple of critical checklist requirements not fully met, which can break the expected behavior and automated tests. First, in src/scripts/main.js you currently select all inputs via something like document.querySelectorAll('input'); per the task you must only process inputs inside the form element (e.g., get the form with document.querySelector('form') and then form.querySelectorAll('input')) so non-form controls are not labeled. Second, you append labels after each input (input.parentElement.append(label)), but the expected structure is that each label appears as a sibling before its corresponding input; switch to input.before(label) or parent.insertBefore(label, input) so the DOM order matches the required layout. These two issues are blockers because they violate explicit checklist items about scoping to the form and correct label placement order.

On the positive side, you correctly create labels with the field-label class, generate or use ids and for attributes properly, and derive label text and placeholders from input.name. As a minor (non-blocking) improvement, you could skip control-type inputs like submit, button, hidden, and reset, and optionally capitalize the label text to visually match the placeholders. Once you restrict selection to form inputs and ensure labels are inserted before their inputs, your solution should align well with the task’s requirements. Keep going—you’re very close, and the core logic is solid.


✨ 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

'use strict';

// write code here
const inputs = document.querySelectorAll('input');

Choose a reason for hiding this comment

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

This selects all inputs on the page. Checklist item #10 requires getting inputs from the form tag. Use document.querySelectorAll('form input') or get the form first (const form = document.querySelector('form')) and then query its inputs so only form inputs are processed.

}

label.setAttribute('for', input.id);
input.parentElement.append(label);

Choose a reason for hiding this comment

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

Appending the label with input.parentElement.append(label) places the label after the input. To match the expected result (label as a sibling before the input) insert the label before the input using input.before(label) or parent.insertBefore(label, input). This keeps labels and inputs as separate siblings (checklist items #16 and #17).

const label = document.createElement('label');

label.classList.add('field-label');
label.textContent = input.name;

Choose a reason for hiding this comment

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

You set label.textContent = input.name which satisfies checklist item #14 (relying on the input's name). If you want a more consistent UI you could capitalize the label text as well (optional enhancement), but this is not required by the checklist.

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 because it fully meets the specified functional requirements for this task. Your main.js correctly finds all input elements within the form, ensures each has an id, creates a corresponding <label> with the field-label class, and sets label.htmlFor to match the input’s id, so the association is accessible and correct. You also derive both the label text and the placeholder from the input’s name and apply capitalization to the placeholder as required, while inserting the label as a sibling before the input instead of wrapping it. All of this is done within a single loop and without modifying HTML or styles, which aligns well with the constraints.

As optional polish (not required for this task), you might consider skipping labels for inputs like type="hidden", submit, or reset, and making the label text more human-friendly by turning user_name into User name. You could also make the generated IDs more robust in case the script is ever run multiple times. Overall, you implemented the core behavior cleanly and in line with the instructions—nice work.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

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