commit for chat #1183
commit for chat #1183
Conversation
There was a problem hiding this comment.
Great work on implementing the core functionality correctly! 🎉 While there are a few minor improvements suggested, they don't impact the overall functionality. Keep up the good work and consider the feedback for future enhancements!
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/App.tsx
Outdated
| const [posts, setPosts] = useState<Post[]>([]); | ||
| const [errorMessage, setErrorMessage] = useState(''); | ||
| const [notificationMessage, setNotificationMessage] = useState(''); | ||
| const [activePostId, setActivePostId] = useState<number | null>(null) |
There was a problem hiding this comment.
It seems you forgot to add a semicolon at the end of this line. While this is not a critical issue in JavaScript, it's a good practice to include semicolons to avoid potential issues.
| useEffect(() => { | ||
| getUsersFromServer() | ||
| .then(setUsers) | ||
| .catch(() => {}); | ||
| }, []); |
There was a problem hiding this comment.
Consider adding error handling logic in the catch block of this useEffect hook. According to the checklist, you should use try {..} catch for error handling .
| .catch(() => { | ||
| setErrorMessage('Something went wrong!'); |
There was a problem hiding this comment.
The error message for the catch block is incomplete. It seems like the string is not properly closed. Make sure to complete the error message string.
| export const USER_ID = 1549; | ||
|
|
||
| export const getUsersFromServer = () => { | ||
| return client.get<User[]>(`/users`); |
There was a problem hiding this comment.
The endpoint /users is hardcoded in the getUsersFromServer function. Ensure that this endpoint is correct and matches the API specification you are working with.
src/components/OnePost.tsx
Outdated
| setActivePostId: (param: number | null) => void; | ||
| }; | ||
|
|
||
| export const OnePost: React.FC<Props> = ({ post, activePostId, setActivePostId }) => { |
There was a problem hiding this comment.
The component OnePost is correctly implemented with the necessary props and logic to handle post selection. However, ensure that the Post type imported from '../types/Post' includes all the necessary fields used in this component.
| }; | ||
|
|
||
| export const OneUser: React.FC<Props> = ({ user, setSelectedUserId, setIsDropDownOpen }) => { | ||
| const handleUserSelect = () => { |
There was a problem hiding this comment.
The function handleUserSelect is correctly implemented to set the selected user ID and close the dropdown. Ensure that the User type includes an id field of type number.
| className="dropdown-item" | ||
| onClick={handleUserSelect} | ||
| > | ||
| {user.name} |
There was a problem hiding this comment.
The href attribute in the anchor tag is using a template string to dynamically set the URL fragment. Ensure that the user.id is always defined to avoid potential issues.
| </button> | ||
| </td> | ||
| </tr> | ||
| {posts.map(post => ( |
There was a problem hiding this comment.
Consider adding a unique key prop to the OnePost component when mapping over the posts array. This is important for React to efficiently update and render components. You can use post.id as the key.
src/components/UserSelector.tsx
Outdated
| > | ||
| <span>Choose a user</span> | ||
|
|
||
| {activeUser?.name |
There was a problem hiding this comment.
Ensure that the activeUser variable correctly handles cases where selectedUserId does not match any user in the users array. This will prevent potential issues with undefined values.
| {users.map(user => ( | ||
| <OneUser | ||
| user={user} | ||
| key={user.id} |
There was a problem hiding this comment.
Good job using user.id as the key for the OneUser component. This ensures that each component instance is uniquely identified, which is important for React's reconciliation process.
src/api/comments.ts
Outdated
| // export interface Comment { | ||
| // id: number; | ||
| // postId: number; | ||
| // name: string; | ||
| // email: string; | ||
| // body: string; | ||
| // } | ||
|
|
||
| // export const updateTodo = ({ id, title, completed }: Omit<Todo, 'userId'>) => { | ||
| // return client.patch<Todo>(`/todos/${id}`, { title, completed }); | ||
| // }; |
There was a problem hiding this comment.
| // export interface Comment { | |
| // id: number; | |
| // postId: number; | |
| // name: string; | |
| // email: string; | |
| // body: string; | |
| // } | |
| // export const updateTodo = ({ id, title, completed }: Omit<Todo, 'userId'>) => { | |
| // return client.patch<Todo>(`/todos/${id}`, { title, completed }); | |
| // }; |
Consider removing unused lines of code
DEMO LINK