Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This is a great start, and the core logic for creating and appending elements is on the right track. However, a couple of adjustments are needed to meet all the requirements.
- Selector Specificity: The requirement is to get
inputelements specifically from theformtag. Your current selector,document.querySelectorAll('.field-text'), is too general. A more specific selector likedocument.querySelectorAll('form .field-text')would be more robust and correctly follow the instructions. - Text Formatting: The logic for generating label and placeholder text doesn't handle
nameattributes with multiple words, such asconfirm-password. This results in incorrect text likeConfirm-passwordandCONFIRM-PASSWORDinstead of the expectedConfirm passwordandCONFIRM PASSWORD. Consider replacing hyphens with spaces before applying capitalization.
Please address these points. You're very close!
✨ 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
| element.placeholder = | ||
| element.name.charAt(0).toUpperCase() + element.name.slice(1); |
There was a problem hiding this comment.
This logic correctly capitalizes the first letter, but it doesn't handle name attributes with multiple words, like confirm-password. This would result in Confirm-password instead of the expected Confirm password. Consider replacing hyphens or underscores with a space before capitalizing.
|
|
||
| elementLabel.className = 'field-label'; | ||
| elementLabel.htmlFor = element.id; | ||
| elementLabel.textContent = element.name.toUpperCase(); |
There was a problem hiding this comment.
Similar to the placeholder, this doesn't handle multi-word name attributes correctly. A name like confirm-password would become CONFIRM-PASSWORD, but the expected result is CONFIRM PASSWORD. You'll need to replace special characters with spaces.
src/scripts/main.js
Outdated
| element.parentElement.prepend(elementLabel); | ||
| } | ||
|
|
||
| const allInput = [...document.querySelectorAll('.field-text')]; |
There was a problem hiding this comment.
The requirement is to get input elements from the form tag. Using just a class selector (.field-text) could select elements that are not in the form. A more specific selector like document.querySelectorAll('form .field-text') would be more robust and align better with the task description.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job fixing the selector issue from the previous review! I am requesting one more change before this can be approved. The script still doesn't correctly handle input name attributes that contain a hyphen, like confirm-password.
Currently, this results in incorrect text like Confirm-password. The expected output should have the hyphen replaced with a space, like Confirm password. You'll need to add logic to replace the hyphen with a space before applying the capitalization. You're very close to finishing this!
✨ 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
| element.placeholder = | ||
| element.name.charAt(0).toUpperCase() + element.name.slice(1); |
There was a problem hiding this comment.
This logic correctly capitalizes the first letter, but it doesn't handle name attributes with hyphens, like confirm-password. You'll need to replace the hyphen with a space to get the expected output Confirm password.
|
|
||
| elementLabel.className = 'field-label'; | ||
| elementLabel.htmlFor = element.id; | ||
| elementLabel.textContent = element.name.toUpperCase(); |
There was a problem hiding this comment.
The text for the label should also have hyphens replaced with spaces. Additionally, based on the expected result image, it seems only the first letter should be capitalized, similar to the placeholder text (e.g., Confirm password instead of CONFIRM-PASSWORD).
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving this submission as an educational override to help you progress, but there are a couple of key items from the last review that still need your attention for future tasks.
You've done a great job selecting the elements and creating the labels. The main issue remaining is handling input names that contain a hyphen, like confirm-password. Your code currently produces Confirm-password. The correct approach is to first replace the hyphen with a space and then capitalize the string to get Confirm password. The same logic applies to the uppercase label text.
Additionally, the conditional block for setting the label's for attribute is being overwritten by the unconditional assignment on line 17, which already correctly handles the requirement. You can safely remove that conditional block.
Great progress on this! Keep these details in mind, and you'll nail the next one.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| element.placeholder = | ||
| element.name.charAt(0).toUpperCase() + element.name.slice(1); |
There was a problem hiding this comment.
This logic does not correctly handle input names containing a hyphen, which was the main point of the previous review. For a name like confirm-password, this produces Confirm-password. You need to first replace the hyphen with a space and then capitalize the string to get the required Confirm password.
| if (!element.id && element.name) { | ||
| elementLabel.htmlFor = element.name; | ||
| } |
There was a problem hiding this comment.
This conditional block is made ineffective by the unconditional assignment on line 17, which overwrites anything set here. The requirement is to set the for attribute to the input's id, which line 17 already handles.
| } | ||
|
|
||
| elementLabel.htmlFor = element.id; | ||
| elementLabel.textContent = element.name.toUpperCase(); |
There was a problem hiding this comment.
Similar to the placeholder, this doesn't handle hyphens correctly. For confirm-password, this code will produce CONFIRM-PASSWORD instead of the expected CONFIRM PASSWORD. The hyphen should be replaced with a space before converting the text to uppercase.
2 participants
No description provided.