Skip to content

Task solution#2021

Open
MateuszCieplak wants to merge 1 commit into
mate-academy:masterfrom
MateuszCieplak:develop
Open

Task solution#2021
MateuszCieplak wants to merge 1 commit into
mate-academy:masterfrom
MateuszCieplak:develop

Conversation

@MateuszCieplak

Copy link
Copy Markdown

@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.

Great job on implementing the task! 🎉 The code is well-structured and meets most of the requirements. There are a couple of minor improvements you could make, such as avoiding unnecessary React fragments and renaming the getUser function to getUserById for clarity. Keep up the good work and happy coding! 😊

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

Comment thread src/App.tsx
import { User } from './components/Types/Users';
import { PostList } from './components/PostList';

function getUser(userId: number): 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.

The function getUser should be renamed to getUserById to clearly indicate that it retrieves a user by their ID. This aligns with the checklist item: "If you are searching some entity by some filter, specify it in function/method name".

Comment on lines +18 to +22
<>
<hr />

<b data-cy="NoCommentsMessage">No comments yet</b>
</>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid using a React fragment (<>...</>) when there is already a wrapper element present, such as the <div> with the class CommentList. This aligns with the checklist item: "Don't use React fragment, if you already have a wrapper".

Comment on lines +10 to +22
<>
<div className="PostInfo">
<div className="PostInfo__header">
<h3 className="PostInfo__title">{post.title}</h3>
<p>
{' Posted by '}
<UserInfo user={post.user} />
</p>
</div>
<p className="PostInfo__body">{post.body}</p>
<CommentList comments={post.comments} />
</div>
</>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid using a React fragment (<>...</>) when there is already a wrapper element present, such as the <div> with the class PostInfo. This aligns with the checklist item: "Don't use React fragment, if you already have a wrapper".

Comment on lines +7 to +11
<>
{posts.map(post => (
<PostInfo key={post.id} post={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.

Avoid using a React fragment (<>...</>) when there is already a wrapper element present, such as the <div> with the class PostList. This aligns with the checklist item: "Don't use React fragment, if you already have a wrapper" .

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