Develop #1192

Mariana-VV · 2024-11-03T15:11:52Z

https://mariana-vv.github.io/react_dynamic-list-of-posts/

mateacademy-ai-mentor

Thank you for your hard work on this project! While your code meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

State Management: The handling of setIsPostLoaded in App.tsx is misleading and could cause incorrect UI behavior. It should accurately reflect the loading status of posts.
Function Defaults: Assigning default empty functions to openForm and deleteComment can lead to unexpected behavior. Ensure these functions are always provided when components are used.
API Defaults: The use of default parameters like userId = 0 and postId = 0 in users.ts might lead to unintended API calls. Verify these defaults are intentional and handled correctly.
Unrelated Code: The deleteTodos function in users.ts seems unrelated to the current context and should be removed if unnecessary.
PropTypes and Error Handling: The absence of propTypes and lack of try-catch error handling are significant omissions that affect code robustness and documentation.

Please address these issues to enhance the functionality and reliability of your code. Keep up the great work, and I'm confident you'll tackle these challenges successfully! 😊

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2024-11-03T15:13:24Z

src/App.tsx

+
+  const getPostsByCurrentUser = (user: User | null | undefined) => {
+    setIsPostLoadError(false);
+    setIsPostLoaded(true);


Setting setIsPostLoaded(true) here might be misleading as it indicates that posts are already loaded. Consider setting this state to true only when the posts have successfully loaded.

mateacademy-ai-mentor · 2024-11-03T15:13:24Z

src/App.tsx

+      .getPostsByUser(user?.id)
+      .then(setPosts)
+      .catch(() => setIsPostLoadError(true))
+      .finally(() => setIsPostLoaded(false));


Setting setIsPostLoaded(false) in the finally block might not accurately reflect the loading status. Consider setting this state to false only after the posts have been successfully loaded or if an error occurs.

mateacademy-ai-mentor · 2024-11-03T15:13:24Z

src/api/users.ts

+export const deleteTodos = (todoId: number) => {
+  return client.delete(`/todos/${todoId}`);
+};


The function deleteTodos appears to be unrelated to the current context of users, posts, and comments. Consider removing it if it's not needed.

mateacademy-ai-mentor · 2024-11-03T15:13:24Z

src/api/users.ts

+export const getPostsByUser = (userId = 0) => {
+  return client.get<Post[]>(`/posts?userId=${userId}`);


The default parameter userId = 0 might lead to unintended API calls if not handled properly. Ensure that this default value is intentional and that the API can handle requests with userId = 0 appropriately.

mateacademy-ai-mentor · 2024-11-03T15:13:24Z

src/api/users.ts

+export const getCommentsByPost = (postId = 0) => {
+  return client.get<Comment[]>(`/comments?postId=${postId}`);


The default parameter postId = 0 might lead to unintended API calls if not handled properly. Ensure that this default value is intentional and that the API can handle requests with postId = 0 appropriately.

mateacademy-ai-mentor · 2024-11-03T15:13:25Z

src/components/PostItem.tsx

+  setCurrentPost,
+  currentPost,
+  getCommentsByCurrentPost,
+  openForm = () => {},


Assigning a default empty function to openForm might lead to unexpected behavior if not properly handled. Ensure that this function is always provided when the component is used.

mateacademy-ai-mentor · 2024-11-03T15:13:25Z

src/components/PostItem.tsx

+
+type Props = {
+  post: Post;
+  key?: number;


The key prop is defined in the Props type but not used in the component. Consider removing it if it's not needed to avoid confusion.

mateacademy-ai-mentor · 2024-11-03T15:13:25Z

src/components/PostsList.tsx

+  setCurrentPost: (post: Post | null) => void;
+  currentPost?: Post | null;
+  getCommentsByCurrentPost: (post: Post) => void;
+  openForm?: (isOpen: boolean) => void;


The openForm prop is optional, but there is no default value provided in the PostItem component. Ensure that this function is always provided when the component is used to avoid unexpected behavior.

mateacademy-ai-mentor · 2024-11-03T15:13:25Z

src/components/UserSelector.tsx

      <div className="dropdown-trigger">
        <button
          type="button"
          className="button"
          aria-haspopup="true"
          aria-controls="dropdown-menu"
+          onClick={handleDropdownClick}
+          onBlur={() => setIsMenuShown(false)}


Using onBlur to close the dropdown might cause it to close unexpectedly if the user interacts with the dropdown items. Consider using a different approach, such as closing the dropdown when clicking outside of it.

mateacademy-ai-mentor · 2024-11-03T15:13:25Z

src/components/UserSelector.tsx

+              onMouseDown={() => {
+                handleUserClick(user);
+              }}


Using onMouseDown to handle user selection might not be the best choice for accessibility. Consider using onClick instead to ensure better accessibility and consistency with user expectations.

anastasiiavorobiova

Great work! Almost done!

anastasiiavorobiova · 2024-11-04T14:24:17Z

src/components/NewCommentForm.tsx

+
+  const handleFormSubmit = (event: FormEvent) => {
+    event.preventDefault();
+    if (!name) {


Suggested change

if (!name) {

if (!name.trim()) {

anastasiiavorobiova · 2024-11-04T14:24:27Z

src/components/NewCommentForm.tsx

+      setNameError(true);
+    }
+
+    if (!email) {


Suggested change

if (!email) {

if (!email.trim()) {

anastasiiavorobiova · 2024-11-04T14:24:34Z

src/components/NewCommentForm.tsx

+      setEmailError(true);
+    }
+
+    if (!message) {


Suggested change

if (!message) {

if (!message.trim()) {

anastasiiavorobiova · 2024-11-04T14:24:50Z

src/components/NewCommentForm.tsx

+      setMessageError(true);
+    }
+
+    if (name && email && message) {


Suggested change

if (name && email && message) {

if (name.trim() && email.trim() && message.trim()) {

anastasiiavorobiova · 2024-11-04T14:25:37Z

src/components/UserSelector.tsx

+  currentUser?: User | null;
+  setCurrentPost: (post: Post | null) => void;
+  setPosts: (posts: Post[]) => void;
+  // setComments: (comments: Comment[]) => void;


Suggested change

// setComments: (comments: Comment[]) => void;

volodymyr-soltys97

Almost done!

volodymyr-soltys97 · 2024-11-05T07:36:48Z

src/App.tsx

+    dataFromServer.getUsers().then(setUsers);
+  }, []);
+
+  function addComment(comment: Comment) {


It is important to stick to one way of creating functions, fix it everywhere

Suggested change

function addComment(comment: Comment) {

const addComment = (comment: Comment) => {

volodymyr-soltys97 · 2024-11-05T07:38:06Z

src/components/CommentItem.tsx

+    <article className="message is-small" data-cy="Comment">
+      <div className="message-header">
+        <a href={`mailto:${comment?.email}`} data-cy="CommentAuthor">
+          {comment?.name}


Use destructuring for comment

etojeDenys

good job, but you need and you can try to remove the comment that user tries to delete immediately from the screen

Mariana-VV added 3 commits November 2, 2024 14:28

onlycheck

a601987

onlycheck

2ca5db4

message

05df961

mateacademy-ai-mentor suggested changes Nov 3, 2024

View reviewed changes

Mariana-VV added 2 commits November 3, 2024 17:01

solution

1fd6530

solution

f087a8c

anastasiiavorobiova suggested changes Nov 4, 2024

View reviewed changes

solution

d85709a

Mariana-VV requested a review from anastasiiavorobiova November 5, 2024 07:32

volodymyr-soltys97 suggested changes Nov 5, 2024

View reviewed changes

improvement

02f5cbc

Mariana-VV requested a review from volodymyr-soltys97 November 5, 2024 09:40

etojeDenys approved these changes Nov 5, 2024

View reviewed changes

comment delete immedeetly

315fd80

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop #1192

Develop #1192

Mariana-VV commented Nov 3, 2024 •

edited

Loading

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

mateacademy-ai-mentor Nov 3, 2024

anastasiiavorobiova left a comment

anastasiiavorobiova Nov 4, 2024

anastasiiavorobiova Nov 4, 2024

anastasiiavorobiova Nov 4, 2024

anastasiiavorobiova Nov 4, 2024

anastasiiavorobiova Nov 4, 2024

volodymyr-soltys97 left a comment

volodymyr-soltys97 Nov 5, 2024

volodymyr-soltys97 Nov 5, 2024

etojeDenys left a comment

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

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

	if (name && email && message) {
	if (name.trim() && email.trim() && message.trim()) {

	function addComment(comment: Comment) {
	const addComment = (comment: Comment) => {

Develop #1192

Are you sure you want to change the base?

Develop #1192

Conversation

Mariana-VV commented Nov 3, 2024 • edited Loading

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anastasiiavorobiova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etojeDenys left a comment

Choose a reason for hiding this comment

Mariana-VV commented Nov 3, 2024 •

edited

Loading