https://Pavlo-lab-max-Kopalko.github.io/react_movies-list-add-form/ #2747
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this pull request! 😊 While your implementation is on the right track, there are a few critical issues that need to be addressed to meet the task requirements fully:
-
Form Submission Handling: The
onSubmithandler should be placed on the<form>element, not the<button>. This ensures proper form submission handling and allows the form to be submitted using the Enter key. -
Prevent Default Behavior: The
onSubmitfunction should includeevent.preventDefault()to prevent the default form submission behavior, allowing you to handle the form data as needed. -
Form Clearing: After a movie is added, the form should be cleared, and errors should not be shown. This logic is currently missing.
-
Trimming Input Values: Ensure that spaces are trimmed from input values before enabling the submit button. This logic needs to be implemented.
Please address these issues to ensure the functionality aligns with the task requirements. Keep up the great work, and don't hesitate to reach out if you need any help! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
src/components/NewMovie/NewMovie.tsx
Outdated
| onClick={() => setCount(count + 1)} | ||
| onSubmit={() => { |
There was a problem hiding this comment.
The onSubmit handler should be placed on the <form> element instead of the <button> element. This ensures that the form submission is handled correctly when the form is submitted.
src/components/NewMovie/NewMovie.tsx
Outdated
| onSubmit={() => { | ||
| infoTitle.trim(); | ||
| infoDescription.trim(); | ||
| infoImageURL.trim(); | ||
| infoImdbURL.trim(); | ||
| infoImdbID.trim(); | ||
| }} |
There was a problem hiding this comment.
The onSubmit function currently only trims the input values but does not prevent the default form submission behavior or handle the form data. Consider using event.preventDefault() to prevent the default form submission and then handle the form data as needed.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: 7]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
🎉 Great Work!
Your implementation is well on its way, and you've addressed many of the previous issues effectively. Here are some key points from the review:
- Form Submission Handling: You've correctly placed the
onSubmithandler on the<form>element in theNewMoviecomponent, ensuring proper form submission handling. - Prevent Default Behavior: The inclusion of
event.preventDefault()in theonSubmitfunction is a good practice to prevent default form submission behavior. - Form Clearing: The form is cleared after a movie is added by incrementing the
countstate, which is a clever way to reset the form. - Trimming Input Values: It's great to see that spaces are trimmed from input values before enabling the submit button.
🚀 Suggestions for Improvement:
- Naming Consistency: Consider renaming
handleChangeinfoDescriptiontohandleChangeInfoDescriptionto maintain consistent camelCase naming. - Form Reset: While the form is cleared by changing the form's key, consider resetting the form fields directly after a movie is added to ensure clarity and maintainability.
Your code meets the task requirements, and the decision to approve is to help you move forward with your learning journey. Please review the comments to further enhance your code. Keep up the great work and continue improving! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| const handleSubmit = () => { | ||
| const objMovie = { | ||
| title: infoTitle.trim(), | ||
| description: infoDescription.trim(), | ||
| imgUrl: infoImageURL.trim(), | ||
| imdbUrl: infoImdbURL.trim(), | ||
| imdbId: infoImdbID.trim(), | ||
| }; | ||
|
|
||
| onAdd(objMovie); | ||
| }; |
There was a problem hiding this comment.
Consider resetting the form fields after a movie is added to ensure the form is cleared. This can be done by setting the state variables (infoTitle, infoDescription, etc.) back to their initial values.
| const handleChangeinfoDescription = ( | ||
| event: React.ChangeEvent<HTMLInputElement>, | ||
| ) => { | ||
| setInfoDescription(event.target.value); | ||
| }; |
There was a problem hiding this comment.
There is a minor inconsistency in the naming convention. Consider renaming handleChangeinfoDescription to handleChangeInfoDescription to maintain consistent camelCase naming.
No description provided.