solution#1447
Conversation
Nika-Andriy
commented
May 22, 2026
- DEMO LINK
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This implementation has critical issues that violate explicit task requirements. First, comment deletion waits for the server response instead of updating the UI immediately - in PostDetails.tsx, the deleteComment function removes the comment only in the .then() callback. According to requirement #7, delete must happen immediately to improve UX - move the filter operation before the API call. Second, field errors are not cleared on input change - the change handlers in NewCommentForm.tsx and PostDetails.tsx only update field values but don't reset hasNameError, hasEmailError, or hasBodyError states. The task explicitly requires 'remove an error on the field change'. Additionally, the condition for 'No posts yet' message shows during loading - it should only display after loading completes (add !isPostsLoading check). These issues must be fixed before approval.
✨ 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
| event.preventDefault(); | ||
|
|
||
| const cleanName = name.trim(); | ||
| const cleanEmail = email.trim(); | ||
| const cleanBody = body.trim(); | ||
| const isNameValid = !!cleanName; | ||
| const isEmailValid = !!cleanEmail; |
There was a problem hiding this comment.
The delete handler removes the comment in the .then() callback, meaning it waits for server response. Per requirement #7, delete should happen immediately to improve UX - move the filter to happen before the API call.
|
|
||
| const handleNameChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| setName(event.target.value); | ||
| }; | ||
|
|
||
| const handleEmailChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| setEmail(event.target.value); | ||
| }; | ||
|
|
||
| const handleBodyChange = (event: React.ChangeEvent<HTMLTextAreaElement>) => { | ||
| setBody(event.target.value); |
There was a problem hiding this comment.
Task requirement states: 'remove an error on the field change'. These change handlers don't clear the error states when the user types.
|
|
||
| const handleEmailChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| setEmail(event.target.value); |
There was a problem hiding this comment.
The change handlers for email and body also need to clear their respective error states when the user starts typing.
|
|
||
| const handleBodyChange = (event: React.ChangeEvent<HTMLTextAreaElement>) => { | ||
| setBody(event.target.value); |
There was a problem hiding this comment.
Body change handler should also clear hasBodyError when user starts typing.
| posts.length > 0 && ( | ||
| <div data-cy="PostsList"> | ||
| <p className="title">Posts:</p> | ||
|
|
||
| <td data-cy="PostTitle"> | ||
| fugit voluptas sed molestias voluptatem provident | ||
| </td> | ||
| <table className="table is-fullwidth is-striped is-hoverable is-narrow"> | ||
| <thead> | ||
| <tr className="has-background-link-light"> | ||
| <th>#</th> | ||
| <th>Title</th> | ||
| {/* eslint-disable-next-line jsx-a11y/control-has-associated-label */} | ||
| <th> </th> |
There was a problem hiding this comment.
The task requires 'remove an error on the field change'. The change handlers only update the field value but don't clear the corresponding error state. Add setHasNameError(false) to handleNameChange, setHasEmailError(false) to handleEmailChange, and setHasBodyError(false) to handleBodyChange.
| useEffect(() => { | ||
| setIsAddFormOpen(false); | ||
|
|
There was a problem hiding this comment.
Same issue: error should be cleared when email changes. Add setHasEmailError(false) here.
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Same issue: error should be cleared when body changes. Add setHasBodyError(false) here.
| return ( | ||
| <div className="content" data-cy="PostDetails"> |
There was a problem hiding this comment.
Duplicate data-cy="PostDetails" attribute and duplicate <div className="content"> element. Remove one.
|
|
||
| // {<article className="message is-small" data-cy="Comment"> | ||
| // <div className="message-header"> | ||
| // <a href="mailto:misha@mate.academy" data-cy="CommentAuthor"> | ||
| // Misha Hrynko | ||
| // </a> | ||
|
|
||
| // <button | ||
| // data-cy="CommentDelete" | ||
| // type="button" | ||
| // className="delete is-small" | ||
| // aria-label="delete" | ||
| // > | ||
| // delete button | ||
| // </button> | ||
| // </div> | ||
|
|
||
| // <div className="message-body" data-cy="CommentBody"> | ||
| // {'Multi\nline\ncomment'} | ||
| // </div> | ||
| // </article>} |
There was a problem hiding this comment.
Unused commented code should be removed from the codebase.
| selectedUser: User | null; | ||
| setOpenPostId: (id: number | null) => void; | ||
| setIsLoading: (load: boolean) => void; | ||
| setSelectedUser: (user: User) => void; |
There was a problem hiding this comment.
The prop type is (user: User) => void but the App component passes setSelectedUser from useState<User | null>(...) which is (user: User | null) => void. These types don't match. Update the prop type to (user: User | null) => void.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The implementation has successfully fixed two of the three previous issues (immediate comment deletion and field error clearing on input change). However, a critical requirement violation remains: the "No posts yet" message displays during loading in multiple files. According to requirement #3, this message should only appear after loading completes. The issue exists in App.tsx (line 55), PostDetails.tsx, NewCommentForm.tsx, and UserSelector.tsx. The current condition selectedUser && posts.length === 0 evaluates to true immediately when a user is selected because posts starts as []. Add !isPostsLoading && to each condition to ensure the message only displays after the API response confirms there are no posts for the selected user. This is a straightforward fix that will make the application behave correctly during the loading state.
✨ 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
|
|
||
| <PostsList /> | ||
| {selectedUser && posts.length === 0 && ( | ||
| <div className="notification is-warning" data-cy="NoPostsYet"> | ||
| No posts yet | ||
| </div> |
There was a problem hiding this comment.
This violates the requirement that 'No posts yet' should only display after loading completes. Add !isPostsLoading check to prevent showing this message while posts are being fetched.
| setErrorMessage('Cant delete a comment'), | ||
| ); | ||
| }; | ||
|
|
||
| export const PostDetails: React.FC = () => { | ||
| return ( |
There was a problem hiding this comment.
This violates the requirement that 'No posts yet' should only display after loading completes. Add !isPostsLoading to the condition to prevent showing this message while waiting for the API response.
| event.preventDefault(); | ||
|
|
||
| const cleanName = name.trim(); | ||
| const cleanEmail = email.trim(); | ||
| const cleanBody = body.trim(); |
There was a problem hiding this comment.
This condition shows 'No posts yet' while posts are loading. When a user is selected, posts starts as [], so posts.length === 0 is true during loading. Add !isPostsLoading && to the condition to only show this message after loading completes and confirms there are no posts for this user.
| data-cy="UserSelector" | ||
| className={classNames('dropdown', { | ||
| 'is-active': isSelectorOpen, | ||
| })} | ||
| > |
There was a problem hiding this comment.
This condition shows 'No posts yet' while posts are being loaded. When a user is selected, posts starts as [] and becomes true for posts.length === 0 immediately, causing the message to display during loading. Add !isPostsLoading && to only show this message after loading completes.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. The critical loading state issue has been successfully fixed—the "No posts yet" message now correctly displays only after the API response confirms no posts exist. All core functionality works well: users load on page load, posts display with proper loading states, the sidebar opens with post details, comments load correctly, and the NewCommentForm implements proper validation with error clearing on input change. One improvement to consider: The form error handling could be enhanced—when postComments().catch() sets an error message, consider re-enabling the form state so users can retry their submission [CHECKLIST ITEM #12]. This ensures a smooth user experience even when the API call fails. Keep up the excellent work! Your implementation demonstrates solid understanding of React patterns, TypeScript interfaces, and proper state management. The optimistic comment deletion and conditional styling with classnames are particularly well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨