Skip to content

add solution#1467

Open
Yevhen-Srdk wants to merge 3 commits into
mate-academy:masterfrom
Yevhen-Srdk:develop
Open

add solution#1467
Yevhen-Srdk wants to merge 3 commits into
mate-academy:masterfrom
Yevhen-Srdk:develop

Conversation

@Yevhen-Srdk

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

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 few critical requirements are not yet met, even though the overall structure and most features are in good shape. The biggest blocker is error handling for adding and deleting comments: client.post/client.delete calls are not wrapped in try {..} catch, failures are not surfaced, and users cannot retry failed Add/Delete operations, which directly violates the task requirement to handle Add/Delete errors and checklist item about using try/catch. Comment deletion is also not optimistic: the UI waits for the server response before removing a comment, but the description explicitly requires deleting immediately in the UI and then handling possible errors (e.g., restoring the comment or allowing a retry). In addition, posts and comments share a single isLoad and error flag, so loaders and error notifications can appear in the wrong context; the requirements expect separate loading/error states for posts vs comments, and isLoad itself violates the “don’t use isLoad, use isLoading/isLoaded” checklist rule.

To move this to completion, you need to: 1) refactor add/delete comment logic to async functions with try/catch, exposing clear error state and retry behavior; 2) make comment deletion optimistic (remove from UI first, then call the API and handle failures); 3) split loading/error state into postsLoading/postsError and commentsLoading/commentsError so posts and comments don’t interfere; and 4) rename isLoad to isLoading (or similar) and remove unused comments/keys generated on render. Once these issues are addressed, the rest of your implementation—user selection, posts table, sidebar, validation, loaders, and general structure—will fully satisfy the task, and you’re very close to that point already.


✨ 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

Comment thread src/App.tsx Outdated
const [formIsOpen, setFormIsOpen] = useState(false);
const [commentBtn, setCommentBtn] = useState(true);
const [error, setError] = useState(false);
const [isLoad, setIsLoad] = useState(false);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checklist item #4 says don't use isLoad, it can be isLoading or isLoaded; using isLoad here violates that requirement, and this single flag is being reused for users, posts, and comments loading, which mixes concerns and causes loaders to show in the wrong place.

Comment thread src/App.tsx Outdated
<p data-cy="NoSelectedUser">No user selected</p>
)}

{isLoad && !error && <Loader />}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The posts loader (<Loader />) is driven by isLoad, which is also toggled for loading users and comments; this means the main content loader can appear while comments are loading or users are fetching, instead of only during posts loading as required.

Comment thread src/App.tsx Outdated
Comment on lines +114 to +123
{error && !isLoad && (
<div
className="notification is-danger"
data-cy="PostsLoadingError"
>
Something went wrong!
</div>
)}

{!error && userPosts.length === 0 && !isLoad && (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using the same error flag for both posts and comments means the posts error notification can be shown due to a comments loading error; requirements distinguish PostsLoadingError and CommentsError, so posts and comments need separate error states.

Comment thread src/App.tsx Outdated
>
<div className="tile is-child box is-success ">
<PostDetails />
{/* activeBtnId === activePost?.id && userPosts.length > 0 */}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This commented line is an unused comment and violates checklist item remove unused comments; either delete it or convert it into meaningful documentation if truly needed.

Comment thread src/App.tsx Outdated
handleCommentDelete={handleCommentDelete}
handleAddComment={handleAddComment}
error={error}
loading={isLoad}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The loading prop passed here is backed by the global isLoad flag, which is also used for posts/users; comments loader should use an error/loading state specific to comments, not a shared flag.

Comment thread src/App.tsx Outdated
Comment on lines +31 to +34
client
.get('/users')
.then(data => setUsers(data as User[]))
.finally(() => setIsLoad(false));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The client.post call is not wrapped in try {..} catch and does not handle errors from the API; checklist item use try {..} catch for error handling and the task requirement Handle Add errors so the user can retry mean you should catch failures, surface an error state, and allow retrying.

Comment thread src/App.tsx Outdated
Comment on lines +49 to +75
.then(data => setPosts(data as Post[]))
.catch(() => setError(true))
.finally(() => setIsLoad(false));
}, [activeUser]);

useEffect(() => {
if (!activePost) {
setComments([]);

return;
}

setIsLoad(true);
setError(false);

client
.get(`/comments?postId=${activePost.id}`)
.then(data => setComments(data as Comment[]))
.catch(() => setError(true))
.finally(() => setIsLoad(false));
}, [activePost]);

useEffect(() => {
setFormIsOpen(false);
setCommentBtn(true);
}, [activeUser, activePost]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The submit handler does not catch errors from addComment; according to the checklist (use try {..} catch) and task (Handle Add errors so the user can retry), you should handle failures from the API and keep the form in a state where the user can retry.

Comment thread src/App.tsx
Comment on lines +32 to +35
.get('/users')
.then(data => setUsers(data as User[]))
.finally(() => setIsLoad(false));
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment deletion waits for the server response before updating UI; the description explicitly requires Delete the comment immediately not waiting for the server response, and checklist item #9 expects try {..} catch-based error handling so users can retry on failure.

Comment thread src/App.tsx
Comment on lines +32 to +35
.get('/users')
.then(data => setUsers(data as User[]))
.finally(() => setIsLoad(false));
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Delete errors from client.delete are not handled with try {..} catch, nor is there any way for the user to retry a failed deletion; this violates the optional requirement Handle Delete errors so the user can retry and checklist item use try {..} catch for error handling.

Comment thread src/App.tsx
.then(data => setUsers(data as User[]))
.finally(() => setIsLoad(false));
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The eslint-disable comment appears unused in this context; checklist item remove unused comments requires cleaning up such comments when they’re no longer necessary.

const newComment: Omit<Comment, 'id'> = {
postId,
name: inputName,
email: inputEmail,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #4: don't use isLoad, it can be isLoading or isLoaded; you should rename this state to isLoading (or similar) and adjust usages accordingly.

setLoading(true);
const newComment: Omit<Comment, 'id'> = {
postId,
name: inputName,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using a single error flag for both posts and comments means comments loading errors will trigger the posts error notification and vice versa; per description you need distinct handling for PostsLoadingError and CommentsError, so consider separate postsError/commentsError and postsLoading/commentsLoading states.

Comment thread src/components/NewCommentForm.tsx Outdated
Comment on lines +18 to +34
const addComment = (
inputName: string,
inputEmail: string,
inputBody: string,
) => {
setLoading(true);
const newComment: Omit<Comment, 'id'> = {
postId,
name: inputName,
email: inputEmail,
body: inputBody,
};

client
.post(`/comments`, newComment)
.then(addedComment => handleAddComment(addedComment as Comment))
.finally(() => setLoading(false));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The addComment function calls the API without try {..} catch and does not expose any way to handle add failures or allow retry; this violates checklist item #9: * use try {..} catch for error handling and the requirement (*) Handle Add errors so the user can retry.

Comment on lines +71 to +75

addComment(trimmedName, trimmedEmail, trimmedBody);
setBody('');
setError(false);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You clear the body but also clear name and email only via handleClear; this is fine, but on submit you don��t keep name and email after a successful submit if the API fails. Once you add error handling for failed adds, ensure you only reset fields according to the spec (keep name and email, clear body).

Comment thread src/components/NewCommentForm.tsx Outdated
Comment on lines +32 to +35
.post(`/comments`, newComment)
.then(addedComment => handleAddComment(addedComment as Comment))
.finally(() => setLoading(false));
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The deleteComment function does not use try {..} catch and only removes the comment after the server response. The description requires: Delete the commnet immediately not waiting for the server response and (*) Handle Delete errors so the user can retry; move the handleCommentDelete(commentId) call before the API call and add error handling to restore or retry on failure.

Comment on lines +99 to 103
value={name}
/>

<span className="icon is-small is-left">
<i className="fas fa-user" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In UserSelector (referenced from here), you are using a dynamically generated string key (key={${user.name}-#${user.id}}) which matches the "generate key on render" pattern; this violates checklist item #6: don't generate key on render � consider using a stable unique field like user.id alone.

Comment thread src/components/PostDetails.tsx Outdated
Comment on lines +32 to +35
const deleteComment = (commentId: number) => {
client
.delete(`/comments/${commentId}`)
.then(() => handleCommentDelete(commentId));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the requirement "Delete the commnet immediately not waiting for the server response to improve the UX" and the generalized requirement "Implement comment deletion" with optimistic UI + retry, because handleCommentDelete is only called after client.delete resolves instead of removing the comment immediately and handling possible errors.

It also violates checklist item #9: "* use try {..} catch for error handling" – deleteComment uses promise chaining with no error path and no try/catch.

Consider deleting from UI first (calling handleCommentDelete before/independent of the server call), wrapping the API request in try/catch, and providing a way to re-add/comment or retry on failure.

handleAddComment,
error,
loading,
formIsOpen,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This state name violates checklist item #4: "don't use isLoad, it can be isLoading or isLoaded". Rename this to isLoading (and update all usages) to match the naming convention requirement.

Comment thread src/components/PostDetails.tsx Outdated
Comment on lines 37 to 51

const hideButton = () => {
setCommentBtn(!commentBtn);
};

export const PostDetails: React.FC = () => {
return (
<div className="content" data-cy="PostDetails">
<div className="content" data-cy="PostDetails">
<div className="block">
<h2 data-cy="PostTitle">
#18: voluptate et itaque vero tempora molestiae
</h2>
<h2 data-cy="PostTitle">{`#${post?.id}: ${post?.title}`}</h2>

<p data-cy="PostBody">
eveniet quo quis laborum totam consequatur non dolor ut et est
repudiandae est voluptatem vel debitis et magnam
</p>
<p data-cy="PostBody">{post?.body}</p>
</div>

<div className="block">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This effect handles posts loading errors via .catch, but the checklist item #9 requires using try {..} catch for error handling. Refactor this to use an async function with try/catch to satisfy that checklist requirement while preserving the same behavior.

Comment thread src/components/PostDetails.tsx Outdated
Comment on lines +54 to +68
{error && !loading && (
<div className="notification is-danger" data-cy="CommentsError">
Something went wrong
</div>
)}

<div className="message-body" data-cy="CommentBody">
Some comment
</div>
</article>
{!error && !loading && comments.length === 0 ? (
<p className="title is-4" data-cy="NoCommentsMessage">
No comments yet
</p>
) : (
<>
<p className="title is-4">Comments:</p>

<article className="message is-small" data-cy="Comment">
<div className="message-header">
<a href="mailto:misha@mate.academy" data-cy="CommentAuthor">
Misha Hrynko
</a>
{comments.map(comment => (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar to posts loading, this effect handles comments loading errors via .catch, but checklist item #9 says "* use try {..} catch for error handling". Refactor to an async function with try/catch to align with the checklist.

Comment thread src/components/PostDetails.tsx Outdated
Comment on lines +18 to +34
};

export const PostDetails = ({
post,
comments,
handleCommentDelete,
handleAddComment,
error,
loading,
formIsOpen,
setFormIsOpen,
commentBtn,
setCommentBtn,
}: Props) => {
const deleteComment = (commentId: number) => {
client
.delete(`/comments/${commentId}`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

addComment uses promise chaining but has no error handling and no try/catch, which violates the optional requirement "() Handle Add errors so the user can retry" and checklist item #9: " use try {..} catch for error handling". You should handle failures (e.g. set an error state so the user can retry) and structure the call with try/catch or at least a .catch that surfaces the error to the UI.

Comment on lines 49 to +75
</div>

<div className="block">
<Loader />
{loading && <Loader />}

<div className="notification is-danger" data-cy="CommentsError">
Something went wrong
</div>

<p className="title is-4" data-cy="NoCommentsMessage">
No comments yet
</p>

<p className="title is-4">Comments:</p>

<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>
{error && !loading && (
<div className="notification is-danger" data-cy="CommentsError">
Something went wrong
</div>
)}

<div className="message-body" data-cy="CommentBody">
Some comment
</div>
</article>
{!error && !loading && comments.length === 0 ? (
<p className="title is-4" data-cy="NoCommentsMessage">
No comments yet
</p>
) : (
<>
<p className="title is-4">Comments:</p>

<article className="message is-small" data-cy="Comment">
<div className="message-header">
<a href="mailto:misha@mate.academy" data-cy="CommentAuthor">
Misha Hrynko
</a>
{comments.map(comment => (
<article
key={comment.id}
className="message is-small"
data-cy="Comment"
>
<div className="message-header">
<a href={`mailto:${comment.email}`} data-cy="CommentAuthor">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In handleSubmit, you set a single error flag and never clear it on individual field change handlers. The description requires "show errors only after the form is submitted" and "remove an error on the field change". With the current design, once error is set to true, error messages remain visible while the user types until another submit or Clear, which doesn't satisfy the "remove an error on the field change" requirement. Consider either separate field-level error state or clearing error in handleNameChange/handleEmailChange/handleBodyChange when the corresponding field becomes valid.

<tbody>
<tr data-cy="Post">
<td data-cy="PostId">17</td>
return (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item don't use isLoad, it can be isLoading or isLoaded; the state name should be updated and all its usages adjusted accordingly.

Comment on lines +31 to +34
<table className="table is-fullwidth is-striped is-hoverable is-narrow">
<thead>
<tr className="has-background-link-light">
<th>#</th>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For handling add/delete errors (task requirement and checklist item use try {..} catch), consider wrapping API calls in try/catch or adding explicit .catch blocks with retry logic instead of only .finally.

Comment on lines +32 to +35
<thead>
<tr className="has-background-link-light">
<th>#</th>
<th>Title</th>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The requirement says comment deletion should be applied immediately to improve UX and errors must be handled so the user can retry; this implementation only deletes the comment after a successful API response and doesn’t handle failures.

Comment on lines +31 to +34
<table className="table is-fullwidth is-striped is-hoverable is-narrow">
<thead>
<tr className="has-background-link-light">
<th>#</th>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checklist item use try {..} catch for error handling is not satisfied here; the POST request should either be inside a try/catch (with async/await) or have an explicit .catch to manage failures and retries.

Comment on lines +33 to +35
<tr className="has-background-link-light">
<th>#</th>
<th>Title</th>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checklist item use try {..} catch for error handling is not satisfied here; the DELETE call should include error handling so the user can retry or you can revert the optimistic UI change.

Comment thread src/components/UserSelector.tsx Outdated
(user, index) =>
user.name && (
<a
key={`${user.name}-#${user.id}`}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #6: don't generate key on render; building a key with a template string from user.name and user.id is unnecessary. Use a stable unique field like user.id alone as the key to comply with the checklist.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

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 few remaining issues are still directly tied to the task description and checklist, so they count as critical blockers for this assignment. Most functionality is correct (users/posts/comments, loading/error states, optimistic add/delete, validation, fetchClient usage), but some required UX and checklist items are not yet satisfied.

Critical issues to fix:

  1. Error removal on field change (required behavior) – In NewCommentForm.tsx and UserSelector.tsx, the error flag is only cleared on submit/clear, not when the user edits name, email, or body. The description explicitly says “remove an error on the field change”, so in each handleNameChange/handleEmailChange/handleBodyChange you should also clear or update error state (e.g. setError(false) or per-field error flags) so validation feedback reacts as soon as the user starts correcting input.
  2. Add/Delete error feedback for retry (required behavior)App tracks commentActionError and passes it to PostDetails, but PostDetails never renders anything based on it. The description’s “(*) Handle Add and Delete errors so the user can retry” implies a visible notification; add a small message (e.g. a Bulma notification is-danger near the comments/form) that appears when commentActionError is true so users see that the operation failed and can try again.
  3. Checklist violations: keys and comments – In UserSelector.tsx, keys like key={`${user.name}-#${user.id}`} violate [CHECKLIST ITEM #6] about not generating keys on render; switch to a stable key={user.id}. Also, broad or unused ESLint-disable comments (e.g. /* eslint-disable @typescript-eslint/indent */ and inline disables in App.tsx/UserSelector.tsx) violate [CHECKLIST ITEM #5]; remove them or narrow them to the specific rule/line only when strictly necessary.

Minor but recommended improvements:

  • In PostDetails.tsx, rename handlers like hideButton to follow the handleX naming convention (e.g. handleWriteCommentButtonClick) as suggested in the checklist, and ensure you don’t leave unused props like commentActionError declared but unused. In App.tsx, consider reducing multiple sequential setState calls in handleClear by grouping related state or at least being intentional about them; this is optional but aligns with the “single setState per call” recommendation.

Once you address the field-change error clearing, visible comment action error feedback, and the checklist items on keys and ESLint comments, your solution will fully match the functional and stylistic requirements and should be ready to pass.


✨ 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

Comment thread src/App.tsx
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/indent */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

setIsLoading, setUsers and the client.get call are fine, but you still have an ESLint disable comment at the top of the file that isn’t clearly needed. This violates checklist item: "remove unused comments" – consider removing or justifying ESLint disables that are no longer required.

Comment thread src/App.tsx Outdated
const [isLoading, setIsLoading] = useState(false);
const [isPostsLoading, setIsPostsLoading] = useState(false);
const [isCommentsLoading, setIsCommentsLoading] = useState(false);
const [commentActionError, setCommentActionError] = useState(false);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your optimistic deletion logic is good and wrapped in try {..} catch, but you don’t surface commentActionError anywhere in the UI (e.g. in PostDetails). The description’s (*) Handle Add and Delete errors so the user can retry implies users should see that an action failed and be able to try again, so consider rendering a small notification when commentActionError is true.

Comment thread src/App.tsx
Comment on lines +38 to +47
.then(data => setUsers(data as User[]))
.finally(() => setIsLoading(false));
}, []);

<Loader />
useEffect(() => {
if (!activeUser) {
setUserPosts([]);

<div
className="notification is-danger"
data-cy="PostsLoadingError"
>
Something went wrong!
</div>
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In this comment form, errors are set on submit and cleared on submit/clear, but they are not removed on field change. The description requires: "remove an error on the field change" – you’ll need to call setError(false) (or similar) in handleNameChange/handleEmailChange/handleBodyChange when the user edits the fields so validation feedback reacts immediately.

Comment thread src/App.tsx
Comment on lines +78 to +83
useEffect(() => {
setFormIsOpen(false);
setCommentBtn(true);
}, [activeUser, activePost]);

const handleCommentDelete = async (commentId: number) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

handleSubmit correctly trims values and blocks submissions with only spaces, which satisfies "make sure you can't add movie with empty data (with spaces only)". However, your handleClear calls setEmail, setName, setBody, and setError sequentially, which goes against the optional checklist item: "* don't use setState several times in one function call (method)". Consider combining state into one object or otherwise reducing multiple setState calls here.

Comment thread src/App.tsx
</div>
</main>
);
</main>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The inline ESLint-disable comment here is not tied to any custom logic and appears to be a leftover workaround. This violates checklist item: "remove unused comments"; if you keep a non-standard button type, use a more targeted solution or remove the disable if the linter no longer complains.

Comment thread src/components/PostDetails.tsx Outdated
commentBtn,
setCommentBtn,
}: Props) => {
const hideButton = () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For checklist item about naming conventions: hideButton is an event handler triggered from onClick, but doesn’t follow the recommended handleX style; consider renaming it (e.g. handleCommentButtonHide or similar) to align with the linked guidelines.

@@ -1,16 +1,46 @@
import React from 'react';
import { useState } from 'react';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The ESLint-disable at the top (/* eslint-disable @typescript-eslint/indent */) appears to be a broad, unused comment now; checklist item #5 says: "remove unused comments". If indentation is already handled by Prettier/ESLint, this global disable should be removed or narrowed to a specific case.

Comment on lines 38 to 39
aria-haspopup="true"
aria-controls="dropdown-menu"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your validation correctly trims values and prevents whitespace-only submissions, but error is only cleared on submit/clear; to satisfy "remove an error on the field change", you should clear or update error inside this change handler when the user edits the name field so that the red state/error message disappears as they type.

Comment on lines 42 to +43
>
<span>Choose a user</span>
<span>{activeUser ? activeUser.name : 'Choose a user'}</span>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same as for handleNameChange: this handler updates email but doesn’t clear error. The description requires removing errors on field change, so consider resetting the error state (or the specific email error) here when the user modifies their email.

Comment thread src/components/UserSelector.tsx Outdated
Comment on lines +53 to +57
{users.map(
(user, index) =>
user.name && (
<a
key={`${user.name}-#${user.id}`}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checklist item #6 says: "don't generate key on render". Here, the key is composed using a template string ${user.name}-#${user.id} inside the map callback, instead of just using a stable user.id. Although this won’t break functionality, it matches the pattern of generating keys at render time; consider switching to key={user.id} to comply with the checklist.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

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 progress while still having a couple of points to improve. Functionally, you’ve implemented the core flow very well: users, posts, and comments load correctly, loaders and error states are in place, the sidebar behavior works, and the comment form has solid validation (including trimming and keeping name/email while clearing only the body). The optimistic add/delete logic with rollback and visible error notification is also nicely handled.

There are two main areas to refine next time: (1) you still have broad ESLint disable comments like /* eslint-disable @typescript-eslint/indent */ and some inline disables for accessibility, which conflicts with the “remove unused comments” guideline [CHECKLIST ITEM #5]; try removing them or scoping them to a single line and fixing the underlying indentation/a11y issues instead. (2) When adding a comment, you currently keep only the optimistic temporary comment and ignore the actual API response; you should replace the temp item with the server’s returned comment so IDs and data stay in sync. Overall, this is strong work with just a few polish points, and you’re clearly on the right track—keep building on this quality of implementation.


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

Footnotes

  1. Rate AI review example

Comment thread src/App.tsx
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/indent */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This broad /* eslint-disable @typescript-eslint/indent */ violates checklist item #5: "remove unused comments". If indentation issues are already handled by Prettier or not critical, consider removing this entirely, or at least narrowing the disable to a specific rule/line instead of the whole file.

@@ -1,8 +1,92 @@
import React from 'react';
import { ChangeEvent, FormEvent, useState } from 'react';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #5: remove unused comments; the broad /* eslint-disable @typescript-eslint/indent */ at the top disables a whole ESLint rule for the file, and there’s no indication that this suppression is strictly necessary. Please remove it or narrow it to a specific line/rule if truly required.

} finally {
setLoading(false);
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This line violates checklist item #5: remove unused comments; the inline eslint-disable comment is used to skip an accessibility rule instead of addressing the label issue on the button. Consider adding a proper accessible label or restructuring the markup instead of disabling the rule.

@@ -1,106 +1,115 @@
import React from 'react';
import { Comment } from '../types/Comment';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #5: "remove unused comments" because it disables @typescript-eslint/indent for the entire file. Consider removing this global disable or narrowing it to the specific line(s) that truly need it, rather than keeping a broad comment.

const handleWriteCommentClick = () => {
setCommentBtn(!commentBtn);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This inline ESLint disable comment falls under checklist item #5: "remove unused comments". Unless this rule suppression is strictly necessary and cannot be fixed by markup changes, you should remove or minimize such disable comments instead of keeping them in the code.

@@ -1,86 +1,68 @@
import React from 'react';
import { Post } from '../types/Post';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #5: remove unused comments; a file-wide /* eslint-disable @typescript-eslint/indent */ is a broad, likely unnecessary ESLint-disable. Consider removing it or narrowing it to a specific rule/line if truly required.

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