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.

Sign up for GitHub

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

Develop #1175

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+598 −263
Open

Develop #1175

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2315cc2
solution
siefimov Oct 9, 2024
61dee3e
add task solution
siefimov Oct 9, 2024
a83bb35
fix reset selectedPost state
siefimov Oct 9, 2024
bf4f807
fix after AI review
siefimov Oct 10, 2024
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
9 changes: 9 additions & 0 deletions src/App.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,12 @@
.message-body {
white-space: pre-line;
}

.tile.is-ancestor {
display: flex;
}

.tile.is-parent {
flex: 1;
padding: 0.75rem;
}
85 changes: 46 additions & 39 deletions src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,54 +7,61 @@ import './App.scss';
import { PostsList } from './components/PostsList';
import { PostDetails } from './components/PostDetails';
import { UserSelector } from './components/UserSelector';
import { Loader } from './components/Loader';
import { useState } from 'react';
import { User } from './types/User';
import { Post } from './types/Post';

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>

<div className="block" data-cy="MainContent">
<p data-cy="NoSelectedUser">No user selected</p>
export const App = () => {
const [selectedUser, setSelectedUser] = useState<User | null>(null);
const [selectedPost, setSelectedPost] = useState<Post | null>(null);

<Loader />
const handleUserSelect = (user: User) => {
setSelectedUser(user);
setSelectedPost(null);
};

<div
className="notification is-danger"
data-cy="PostsLoadingError"
>
Something went wrong!
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
selectedUser={selectedUser}
onSelectUser={handleUserSelect}
/>
</div>

<div className="notification is-warning" data-cy="NoPostsYet">
No posts yet
<div className="block" data-cy="MainContent">
{selectedUser ? (
<PostsList
selectedUser={selectedUser}
onPostSelect={setSelectedPost}
/>
) : (
<p data-cy="NoSelectedUser">No user selected</p>
)}
</div>

<PostsList />
</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': selectedPost },
)}
>
<div className="tile is-child box is-success ">
{selectedPost && <PostDetails post={selectedPost} />}
</div>
</div>
</div>
</div>
</div>
</main>
);
</main>
);
};
18 changes: 18 additions & 0 deletions src/api/comments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { ApiPath } from '../enums/api-path.enum';
import { QueryParams } from '../enums/query-params.enum';
import { Comment } from '../types/Comment';
import { client } from '../utils/fetchClient';

export const getCommentsByPost = (postId: number) => {
return client.get<Comment[]>(
`${ApiPath.COMMENTS}?${QueryParams.POST_ID}=${postId}`,
);
};

export const addComment = (comment: Omit<Comment, 'id'>) => {
return client.post<Comment>(ApiPath.COMMENTS, comment);
};

export const deleteComment = (commentId: number) => {
return client.delete(`${ApiPath.COMMENTS}/${commentId}`);
};
9 changes: 9 additions & 0 deletions src/api/posts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { client } from '../utils/fetchClient';
import { Post } from '../types/Post';
import { ApiPath, QueryParams } from '../enums/enums';

export const getPostsByUserId = (userId: number) => {
return client.get<Post[]>(
`${ApiPath.POSTS}?${QueryParams.USER_ID}=${userId}`,
);
};
5 changes: 5 additions & 0 deletions src/api/users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { User } from '../types/User';
import { client } from '../utils/fetchClient';
import { ApiPath } from '../enums/api-path.enum';

export const getUsers = () => client.get<User[]>(ApiPath.USERS);
160 changes: 131 additions & 29 deletions src/components/NewCommentForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,87 @@
import React from 'react';
import cn from 'classnames';
import React, { useState } from 'react';
import { CommentData } from '../types/Comment';

type Props = {
postId: number;
Copy link

Choose a reason for hiding this comment

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

Good job on defining the Props type for your component. This helps to ensure type safety for the props that your component receives.

onSubmit: (data: CommentData & { postId: number }) => void;
};

export const NewCommentForm: React.FC<Props> = ({ postId, onSubmit }) => {
const [formData, setFormData] = useState<CommentData>({
name: '',
email: '',
body: '',
});

const [errors, setErrors] = useState({
name: false,
email: false,
body: false,
});

const [isLoading, setIsLoading] = useState(false);

Copy link

Choose a reason for hiding this comment

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

Good job on using a separate state variable to track the loading state. This is a good practice as it allows you to provide feedback to the user about the loading state.

const handleChange = (
e: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>,
) => {
const { name, value } = e.target;

setFormData(prevData => ({
...prevData,
Copy link

Choose a reason for hiding this comment

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

The setFormData and setErrors methods are being called multiple times within the same function. This is not recommended as it can lead to unnecessary re-renders and make the code harder to understand. It would be better to combine these into a single setState call. You can achieve this by creating a new object with the updated values and then calling setState once with this new object.

[name]: value,
}));
setErrors(prevErrors => ({
...prevErrors,
[name]: false,
}));
};

Copy link

Choose a reason for hiding this comment

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

Good use of functional updates when setting state. This ensures that you always have the most current state when updating.

const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();
Copy link

Choose a reason for hiding this comment

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

Great job on using async/await syntax for handling promises. This makes your code easier to read and understand.


const newErrors = {
name: !formData.name,
email: !formData.email,
Copy link

Choose a reason for hiding this comment

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

This code does not check if the input fields are empty but filled with spaces only. This can lead to adding a movie with empty data. You should trim the input values before checking if they are empty.

body: !formData.body,
};
Copy link

Choose a reason for hiding this comment

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

The email validation here is not sufficient. Currently, it only checks if the email field is not empty, but it does not verify if the input is a valid email format. You can use a simple regex pattern to check for a valid email format.


setErrors(newErrors);

if (Object.values(newErrors).some(error => error)) {
return;
}

setIsLoading(true);

try {
await onSubmit({ ...formData, postId });
Copy link

Choose a reason for hiding this comment

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

Good job on using a try/finally block. This ensures that setIsLoading(false) will be called whether or not the onSubmit function throws an error.


Copy link

Choose a reason for hiding this comment

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

There is a potential issue with setting state based on previous state like this. If the onSubmit function is asynchronous, this could potentially lead to state being out of sync. You should consider using the function form of setState that accepts a function where the first argument is the previous state.

setFormData({
...formData,
body: '',
});
} finally {
setIsLoading(false);
}
};

const handleClear = () => {
setFormData({
Copy link

Choose a reason for hiding this comment

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

Good job on handling form reset. This improves user experience as they can easily clear the form.

name: '',
email: '',
body: '',
});

setErrors({
name: false,
email: false,
body: false,
});
};

export const NewCommentForm: React.FC = () => {
return (
<form data-cy="NewCommentForm">
<form data-cy="NewCommentForm" onSubmit={handleSubmit}>
<div className="field" data-cy="NameField">
Copy link

Choose a reason for hiding this comment

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

Nice job on using semantic HTML elements. This is good for accessibility and SEO.

<label className="label" htmlFor="comment-author-name">
Author Name
Expand All @@ -14,24 +93,30 @@ export const NewCommentForm: React.FC = () => {
name="name"
id="comment-author-name"
placeholder="Name Surname"
className="input is-danger"
className={cn('input', { 'is-danger': errors.name })}
value={formData.name}
onChange={handleChange}
/>

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

<span
className="icon is-small is-right has-text-danger"
data-cy="ErrorIcon"
>
<i className="fas fa-exclamation-triangle" />
</span>
{errors.name && (
<span
className="icon is-small is-right has-text-danger"
data-cy="ErrorIcon"
>
<i className="fas fa-exclamation-triangle" />
</span>
)}
</div>

<p className="help is-danger" data-cy="ErrorMessage">
Name is required
</p>
{errors.name && (
<p className="help is-danger" data-cy="ErrorMessage">
Name is required
</p>
)}
</div>

<div className="field" data-cy="EmailField">
Expand All @@ -45,24 +130,30 @@ export const NewCommentForm: React.FC = () => {
name="email"
id="comment-author-email"
placeholder="[email protected]"
className="input is-danger"
className={cn('input', { 'is-danger': errors.email })}
value={formData.email}
onChange={handleChange}
/>

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

<span
className="icon is-small is-right has-text-danger"
data-cy="ErrorIcon"
>
<i className="fas fa-exclamation-triangle" />
</span>
{errors.email && (
<span
className="icon is-small is-right has-text-danger"
data-cy="ErrorIcon"
>
<i className="fas fa-exclamation-triangle" />
</span>
)}
</div>

<p className="help is-danger" data-cy="ErrorMessage">
Email is required
</p>
{errors.email && (
<p className="help is-danger" data-cy="ErrorMessage">
Email is required
</p>
)}
</div>

<div className="field" data-cy="BodyField">
Expand All @@ -75,25 +166,36 @@ export const NewCommentForm: React.FC = () => {
id="comment-body"
name="body"
placeholder="Type comment here"
className="textarea is-danger"
className={cn('textarea', { 'is-danger': errors.body })}
value={formData.body}
onChange={handleChange}
/>
</div>

<p className="help is-danger" data-cy="ErrorMessage">
Enter some text
</p>
{errors.body && (
<p className="help is-danger" data-cy="ErrorMessage">
Enter some text
</p>
)}
</div>

<div className="field is-grouped">
<div className="control">
<button type="submit" className="button is-link is-loading">
<button
type="submit"
className={cn('button is-link', { 'is-loading': isLoading })}
>
Add
</button>
</div>

<div className="control">
{/* eslint-disable-next-line react/button-has-type */}
<button type="reset" className="button is-link is-light">
<button
type="reset"
className="button is-link is-light"
onClick={handleClear}
>
Copy link

Choose a reason for hiding this comment

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

The handleClear function is being used as a click handler for the reset button. This is not necessary, as the reset type button will automatically reset all form values to their initial state. You can remove this click handler.

Clear
</button>
</div>
Expand Down
Loading
Loading