task by Natalia2408-forever · Pull Request #3574 · mate-academy/react_add-todo-form

Natalia2408-forever · 2026-03-10T10:14:58Z

No description provided.

mateacademy-ai-mentor

Review Summary

Great job on implementing the core functionality for adding todos and setting up the form validation! The component structure is solid. However, a few critical issues need to be addressed before this can be approved.

Here are the main points to work on:

Incomplete Todo Type: The Todo type in src/types/Todo.ts is missing the required user property. This is causing type errors in several components (App.tsx, TodoInfo.tsx, TodoList.tsx). Please update the type to include the user.
Incorrect Data Flow: The TodoForm component currently imports usersFromServer directly. This violates the project's architectural guidelines [CHECKLIST ITEM #5]. Instead, you should pass the users data from the App component down to TodoForm as a prop.
Missing data-id Attribute: The task requires each .TodoInfo element to have a data-id={todo.id} attribute. This is currently missing from the <article> tag in src/components/TodoInfo/TodoInfo.tsx.
Type Mismatch in onSubmit: In TodoForm.tsx, the onSubmit prop is typed to receive userId as a string, but you're correctly converting it to a number before calling the function. Please update the type definition to match the data being passed.

Once these issues are resolved, your solution will be in great shape. Keep up the good work!

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

Important

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

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/App.tsx

+export const initialTodos: Todo[] = todosFromServer.map(todo => ({
+  ...todo,
+  user: getUserById(todo.userId),
+}));


The objects created here for initialTodos (and later for newTodo on line 34) include a user property. However, the imported Todo type does not define this property, which will cause a type error. The task requires a user object in each todo, so you should update the Todo type definition in src/types/Todo.ts to include it.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/App.tsx

+  return (
+    <div className="App">
+      <h1 className="App__title">Static list of todos</h1>
+      <PostForm onSubmit={addTodo} />


This violates checklist item #5: 'Prepare data in one place (App component) and pass it to child components. Don't use import keyword in non-root components'. The PostForm component currently imports usersFromServer directly. Instead, you should pass usersFromServer from this App component to PostForm as a prop.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/components/TodoForm/TodoForm.tsx

@@ -0,0 +1,101 @@
+import React, { useState } from 'react';
+import usersFromServer from '../../api/users';


This violates checklist item #5: 'Don't use import keyword in non-root components'. Data like usersFromServer should be imported in the root App component and passed down to child components as props.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/components/TodoForm/TodoForm.tsx

+};
+
+type Props = {
+  onSubmit: (data: TodoForm) => void;


The type definition for the onSubmit prop is incorrect. It's defined to accept data of type TodoForm, which has userId as a string. However, you call it on line 55 with an object where userId has been converted to a number. This type should match the data shape that is actually passed, which is { title: string; userId: number; }.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/components/TodoForm/TodoForm.tsx

+  return (
+    <form action="/api/todos" method="POST" onSubmit={handleSubmit}>
+      <div className="field">
+        <label htmlFor="titleInout">Title </label>


There's a typo in the htmlFor attribute. It should be "titleInput" to match the id of the <input> element on the next line. This is important for accessibility, as it allows users to focus the input field by clicking on its label.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/components/TodoInfo/TodoInfo.tsx

+import { Todo } from '../../types/Todo';
+
+type Props = {
+  todo: Todo;


The imported Todo type from ../../types/Todo.ts doesn't include the user property, which you are accessing on line 15. You should update the Todo type definition to include the user property to match the data structure being created in App.tsx.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/components/TodoInfo/TodoInfo.tsx

+  const className = `TodoInfo ${todo.completed ? 'TodoInfo--completed' : ''}`;
+
+  return (
+    <article className={className}>


The task description requires adding a data-id={todo.id} attribute to each .TodoInfo element. This attribute is missing from the <article> tag.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/components/UserInfo/UserInfo.tsx

+  if (!user) {
+    return null;
+  }


This violates checklist item #4: "Don't render the component if the property that you pass to the component has null or undefined value." The checklist's example shows that the check should happen in the parent component before rendering, not inside the child component. Since the parent component TodoInfo already checks if todo.user exists (todo.user && <UserInfo ... />), this check is redundant and can be removed.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/types/Todo.ts

+export type Todo = {
+  id: number;
+  title: string;
+  completed: boolean;
+  userId: number;
+};


The task description requires each Todo item to include a user object. This type definition is missing that user property. You need to add it here to match the data structure used in App.tsx and TodoInfo.tsx. You will also need to import the User type.

mateacademy-ai-mentor · 2026-03-10T10:17:17Z

src/components/TodoList/TodoList.tsx

+import { Todo } from '../../types/Todo';
+
+type Props = {
+  todos: Todo[];


The Todo type imported from ../../types/Todo.ts is incomplete. The todos array passed from the App component contains objects that each have a user property, but this type definition doesn't include it. This will cause a TypeScript error. You should update the Todo type in src/types/Todo.ts to reflect the actual data structure.

mateacademy-ai-mentor

Review Summary

Excellent work on this task! I am approving your solution as you've successfully implemented all the core requirements and addressed all the feedback from the previous review. The data flow is clean, the form validation works perfectly, and the components are well-structured.

As a minor point for future improvement, the null check in UserInfo.tsx is redundant. The parent component, TodoInfo, already ensures the user prop is valid before rendering UserInfo, so you can safely remove the if (!user) block to simplify the component. This aligns with the principle of letting parent components handle rendering logic [CHECKLIST ITEM #4].

Overall, this is a very strong submission. Keep up the great work!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2026-03-10T18:40:49Z

src/components/UserInfo/UserInfo.tsx

+  if (!user) {
+    return null;
+  }


This null check is a good example of the 'BAD EXAMPLE' pattern described in checklist item #4. The responsibility for checking if user exists lies with the parent component (TodoInfo), which already does this correctly with {todo.user && <UserInfo user={todo.user} />}.

Since the parent ensures user is never null or undefined when this component is rendered, you can remove this if block. This will simplify the component and align it with the provided guidelines.

task

a389765

mateacademy-ai-mentor suggested changes Mar 10, 2026

View reviewed changes

corrected without a problem with identations

e93c105

Natalia2408-forever requested a review from mateacademy-ai-mentor March 10, 2026 18:38

mateacademy-ai-mentor approved these changes Mar 10, 2026

View reviewed changes

corrected with identations

8a83e5a

Natalia2408-forever requested a review from mateacademy-ai-mentor March 11, 2026 06:32

		@@ -0,0 +1,101 @@
		import React, { useState } from 'react';
		import usersFromServer from '../../api/users';

Conversation

Natalia2408-forever commented Mar 10, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants