Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add solution #1203

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 109 additions & 37 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,53 +8,125 @@
import { PostDetails } from './components/PostDetails';
import { UserSelector } from './components/UserSelector';
import { Loader } from './components/Loader';
import { User } from './types/User';
import { useEffect, useState } from 'react';
import { Post } from './types/Post';
import { getPosts } from './api/api';

export const App = () => (
<main className="section">
<div className="container">
<div className="tile is-ancestor">
<div className="tile is-parent">
<div className="tile is-child box is-success">
<div className="block">
<UserSelector />
</div>
export const App = () => {
const [selectedUser, setSelectedUser] = useState<User | null>(null);
const [isLoading, setIsLoading] = useState(false);
const [isError, setIsError] = useState(false);
const [posts, setPosts] = useState<Post[]>([]);
const [openedPost, setOpenedPost] = useState<Post | null>(null);

<div className="block" data-cy="MainContent">
<p data-cy="NoSelectedUser">No user selected</p>
const selectUser = (user: User) => {
setOpenedPost(null);
setPosts([]);
setSelectedUser(user);
};

<Loader />
const loadPost = () => {
if (selectedUser && !isError) {
setIsLoading(true);

<div
className="notification is-danger"
data-cy="PostsLoadingError"
>
Something went wrong!
</div>
getPosts(selectedUser.id)
.then(result => {
const results = result.map(post => ({
id: post.id,
userId: post.userId,
title: post.title,
body: post.body,
}));

setPosts(results);
})
.catch(() => {
setIsError(true);

Choose a reason for hiding this comment

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

Consider resetting the isError state after a certain period or when a new user is selected. This will improve user experience by clearing the error message once the issue is resolved or a new action is taken.

setTimeout(() => setIsError(false), 3000);

Choose a reason for hiding this comment

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

The error state is reset after a timeout, which might not be the best approach. Consider resetting the error state when the user takes a new action, such as selecting a different user, to improve user experience.

})
Comment on lines +44 to +47

Choose a reason for hiding this comment

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

The error handling here is incomplete. While you set the error state to true, you should also consider providing more context about the error, such as logging it or displaying a more detailed message to the user. Additionally, ensure that the error state is reset appropriately.

.finally(() => {
setIsLoading(false);
});
}
};

<div className="notification is-warning" data-cy="NoPostsYet">
No posts yet
useEffect(() => {
loadPost();
}, [selectedUser, isError]);

Check warning on line 56 in src/App.tsx

View workflow job for this annotation

GitHub Actions / run_linter (20.x)

React Hook useEffect has a missing dependency: 'loadPost'. Either include it or remove the dependency array

Choose a reason for hiding this comment

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

Including isError in the dependency array of useEffect might cause unnecessary re-fetching of posts when the error state changes. Consider removing isError from the dependencies to prevent this.


const chosePost = (post: Post | null) => {
setOpenedPost(post);
};

return (
<main className="section">
<div className="container">
<div className="tile is-ancestor">
<div className="tile is-parent">
<div className="tile is-child box is-success">
<div className="block">
<UserSelector onSelectUser={selectUser} />
</div>

<PostsList />
<div className="block" data-cy="MainContent">
{!selectedUser && (
<p data-cy="NoSelectedUser">No user selected</p>
)}

{isLoading ? (
<Loader />
) : (
<>
{isError && (
<div
className="notification is-danger"
data-cy="PostsLoadingError"
>
Something went wrong!
</div>
)}

{posts.length === 0 && selectedUser && !isError && (
<div
className="notification is-warning"
data-cy="NoPostsYet"
>
No posts yet
</div>
)}

{posts.length > 0 && (
<PostsList
posts={posts}
onOpenSideBar={chosePost}
openedPost={openedPost}
/>
)}
</>
)}
</div>
</div>
</div>
</div>

<div
data-cy="Sidebar"
className={classNames(
'tile',
'is-parent',
'is-8-desktop',
'Sidebar',
'Sidebar--open',
)}
>
<div className="tile is-child box is-success ">
<PostDetails />
<div
data-cy="Sidebar"
className={classNames(
'tile',
'is-parent',
'is-8-desktop',
'Sidebar',
{
'Sidebar--open': openedPost,
},
)}
>
<div className="tile is-child box is-success ">
{openedPost && <PostDetails post={openedPost} />}
</div>
</div>
</div>
</div>
</div>
</main>
);
</main>
);
};
24 changes: 24 additions & 0 deletions src/api/api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Comment, CommentData } from '../types/Comment';
import { Post } from '../types/Post';
import { User } from '../types/User';
import { client } from '../utils/fetchClient';

export const getUsers = () => {
return client.get<User[]>('/users');
};

export const getPosts = (userId: number) => {
return client.get<Post[]>(`/posts?userId=${userId}`);
};

export const getComments = (postId: number) => {
return client.get<Comment[]>(`/comments?postId=${postId}`);
};

export const deleteComment = (commentId: number) => {
return client.delete(`/comments/${commentId}`);
};
Comment on lines +18 to +20

Choose a reason for hiding this comment

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

The deleteComment function lacks error handling. Consider adding a .catch() block to handle potential errors that may occur during the API call. This will help in providing feedback to the user if the deletion fails.


export const createComment = (data: CommentData) => {
return client.post<Comment>('/comments', data);
};
Comment on lines +22 to +24

Choose a reason for hiding this comment

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

The createComment function also lacks error handling. Similar to deleteComment, consider adding a .catch() block to manage errors that may arise during the comment creation process.

Loading
Loading