Skip to content

task solution#1811

Open
freelinex wants to merge 3 commits intomate-academy:masterfrom
freelinex:develop
Open

task solution#1811
freelinex wants to merge 3 commits intomate-academy:masterfrom
freelinex:develop

Conversation

@freelinex
Copy 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 one core requirement about label and placeholder formatting is not yet met, and this will cause the assignment’s visual/tests to fail. You correctly select the form inputs, create labels with class field-label, set the for attribute from input.id, and insert labels/placeholders in main.js, which aligns well with most of the checklist items. However, your current logic uses the raw name (e.g. firstName) for label.textContent and only capitalizes the first character for the placeholder (producing FirstName), while the task explicitly expects human-readable text like First name derived from camelCase names [CHECKLIST ITEM #5, #6].

Critical issues to fix (blockers):

  1. In src/scripts/main.js, where you set label.textContent and compute the placeholder, convert the camelCase name into a spaced, capitalized phrase before using it. For example, you can transform firstName by inserting a space before each uppercase letter and then capitalizing the first character: firstNameFirst name; use this same formatted string for both the label text and the placeholder.
  2. Ensure you consistently rely on input.id for the for attribute (as you do now) and keep the HTML unchanged; all required id attributes are already present.

As a minor improvement (not a blocker), you could defensively handle odd or missing name values by falling back to an empty string or a simple default label. Overall, your structure and DOM manipulation are solid; once you adjust the formatting of labels and placeholders to create human-readable strings, this will meet the assignment requirements fully.


✨ 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

src/index.html Outdated
</div>
<script src="scripts/main.js"></script>
<script
src="scripts/main.js"

Choose a reason for hiding this comment

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

The script src points to scripts/main.js. Make sure this path matches the actual script location so the DOM-fixing code runs (the project's script is expected at src/scripts/main.js relative to the project root, and this relative path from this HTML looks correct).

src/index.html Outdated
<script src="scripts/main.js"></script>
<script
src="scripts/main.js"
defer

Choose a reason for hiding this comment

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

Using defer is appropriate here: it ensures the script runs after parsing the document so your main.js can safely query the inputs. No change required, just keep defer if your script assumes the DOM is ready.


label.className = 'field-label';
label.setAttribute('for', item.id);
label.textContent = item.name;

Choose a reason for hiding this comment

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

label.textContent = item.name; uses the raw name (e.g. firstName). Checklist item #5 expects the label text to rely on the input name but the UI/tests expect a human-readable label (e.g. First name). Consider converting camelCase into spaced words and capitalizing appropriately before assigning textContent (for example insert a space before uppercase letters and capitalize the first character).

label.setAttribute('for', item.id);
label.textContent = item.name;

item.placeholder = item.name[0].toUpperCase() + item.name.slice(1);

Choose a reason for hiding this comment

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

This placeholder computation (item.name[0].toUpperCase() + item.name.slice(1)) will produce FirstName from firstName. Checklist item #6 asks to set placeholders based on the input name and capitalize them — you should convert camelCase to a readable phrase (e.g. First name) rather than only capitalizing the first character of the raw name.

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 a core requirement about label and placeholder formatting is not yet met, which will cause the task’s tests/visual checks to fail. In src/scripts/main.js, you currently set label.textContent = item.name (e.g. firstName), but the instructions require converting camelCase into a spaced, human-readable phrase with only the first word capitalized (e.g. First name) before assigning it to the label [CHECKLIST ITEM #5]. Similarly, the placeholder is built with item.name[0].toUpperCase() + item.name.slice(1), which results in FirstName, but it must reuse the same formatted string you computed for the label so that the placeholder text matches (e.g. item.placeholder = formattedLabelText) [CHECKLIST ITEM #6].

To fix this, create a helper that takes item.name, inserts spaces before uppercase letters, lowercases the rest, then capitalizes just the first character (and handles missing/empty names defensively), and store the result in something like formattedLabelText. Then, use this single formattedLabelText both for label.textContent and for item.placeholder. Once this formatting logic is in place and reused consistently, your DOM creation logic (labels, for attributes, classes, insertion) looks aligned with the rest of the task requirements. Overall structure is good; you just need to correct this formatting behavior to fully satisfy the specification.


✨ 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


label.className = 'field-label';
label.setAttribute('for', item.id);
label.textContent = item.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 to the raw item.name (e.g. firstName). The task requires converting camelCase into a spaced, capitalized phrase (e.g. First name) and using that for the label text (checklist items #5 and #6). Compute a formatted string from item.name and assign it here instead.

label.setAttribute('for', item.id);
label.textContent = item.name;

item.placeholder = item.name[0].toUpperCase() + item.name.slice(1);

Choose a reason for hiding this comment

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

The placeholder uses item.name[0].toUpperCase() + item.name.slice(1), which yields FirstName for firstName. Reuse the same human-readable formatted string you use for the label so the placeholder matches (e.g. item.placeholder = formattedLabelText).

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 keep progressing while you refine your JavaScript and DOM handling skills.

You did a solid job selecting the form inputs, creating label elements with the correct field-label class, wiring up the for attribute, and inserting the labels before the inputs in src/scripts/main.js; this covers the structural part of the task well. The main thing still missing is the required formatting of label text and placeholders: instead of using label.textContent = item.name and a simple capitalization for the placeholder, you need a small helper that converts camelCase names like firstName into First name, and then reuse that same formatted string for both the label and item.placeholder while safely handling empty or missing name values [CHECKLIST ITEM #5, #6]. Also, make sure that each input has an id (e.g., derive it from name when missing) before assigning label.htmlFor, so labels are properly associated with their inputs for accessibility.

As you iterate, focus on writing a reusable formatter function (e.g., insert spaces before capital letters, lowercase everything, then capitalize only the first character) and calling it once per input to keep label and placeholder perfectly in sync. Your current structure is very close; with these formatting and id-handling tweaks, your solution will align well with the task’s expectations and automated tests. Keep going—you’re clearly on the right track, and refining these details is exactly how you grow as a developer.


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

Footnotes

  1. Rate AI review example


label.className = 'field-label';
label.setAttribute('for', item.id);
label.textContent = item.name;

Choose a reason for hiding this comment

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

Do not assign the raw item.name to the label. Per the checklist, convert camelCase (e.g. firstName) into a spaced human‑readable phrase with only the first word capitalized (e.g. First name) and set that as label.textContent. Create a small helper formatter and use it here so label text is correct.

Comment on lines +12 to +14
const placeholder = item.name.toLowerCase();

item.placeholder = placeholder[0].toUpperCase() + placeholder.slice(1);

Choose a reason for hiding this comment

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

Current placeholder logic lowercases the name then capitalizes the first character, producing Firstname. The placeholder must match the formatted label text. Replace this logic by reusing the formatted label string (e.g. item.placeholder = formattedLabelText) and handle empty or missing name values defensively.

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