done Add TODO form task by maximtsyrulnyk · Pull Request #3561 · mate-academy/react_add-todo-form

maximtsyrulnyk · 2026-02-23T18:15:51Z

done Add TODO form task

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are a few issues that can cause runtime errors and violate explicit checklist requirements. The most critical one is in TodoInfo.tsx, where UserInfo is rendered with todo?.user; since todo.user is optional, this can be undefined, which breaks [CHECKLIST ITEM #4] and may crash when UserInfo accesses user fields—only render UserInfo when todo.user exists or make the user prop optional and guard inside UserInfo. Closely related, UserInfo currently declares user: User but callers may pass undefined, so you should either change the prop to user?: User and early-return when it’s missing, or guarantee that all callers pass a non-null User.

There are also a couple of robustness issues that can break behavior in edge cases: TodoList calls todos.map without guarding against todos being null/undefined (again [CHECKLIST ITEM #4]); add a simple if (!todos) return null; or similar before mapping. In App.tsx, generateNewTodoId uses Math.max(...todos.map(...)) + 1 without handling the empty-array case; when todos is empty this yields -Infinity, so add a fallback like returning 1 when todos.length === 0. Finally, your form labels use htmlFor="titleInput" and htmlFor="userSelect" but the corresponding input and select elements lack matching id attributes, which can cause accessibility/test failures; add id="titleInput" and id="userSelect" respectively. Once these are fixed, your overall implementation of the todo flow, validation, and rendering will be in good shape—your structure and use of types and derived prepareTodos logic are on the right track.

✨ 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-02-23T18:21:34Z

src/components/TodoList/TodoList.tsx

+export const TodoList = ({ todos }: Prop) => {
+  return (
+    <section className="TodoList">
+      {todos.map(todo => (


This violates CHECKLIST ITEM #4: "Don't render the component if the property that you pass to the component has null or undefined value." If todos is null/undefined, todos.map will throw. Consider adding a guard such as if (!todos) return null; or render a fallback message before calling todos.map.

mateacademy-ai-mentor · 2026-02-23T18:21:34Z

src/App.tsx

+  function generateNewTodoId() {
+    return Math.max(...todos.map(todo => todo.id)) + 1;


generateNewTodoId uses Math.max(...todos.map(todo => todo.id)) + 1. If todos is an empty array this becomes Math.max() on an empty list which yields -Infinity, producing an invalid id. Consider guarding for the empty array, e.g. return todos.length ? Math.max(...todos.map(t => t.id)) + 1 : 1; to ensure a valid id is produced.

mateacademy-ai-mentor · 2026-02-23T18:21:34Z

src/App.tsx

+          <label htmlFor="titleInput">Title:</label>
+          <input
+            type="text"
+            data-cy="titleInput"
+            placeholder="Enter a title"
+            value={title}
+            onChange={event => {
+              setTitle(() => event.target.value);
+              setIsTitleEmpty(false);
+            }}
+          />


The label uses htmlFor="titleInput" but the corresponding input element does not have an id attribute. To properly associate the label and input (and satisfy the 'Add labels' requirement), add id="titleInput" to the input or change the label to match actual input id.

mateacademy-ai-mentor · 2026-02-23T18:21:34Z

src/App.tsx

+          <label htmlFor="userSelect">User:</label>
+          <select
+            data-cy="userSelect"
+            onChange={event => {
+              setUserId(event.target.value);
+              setUserIsNotSelected(false);
+            }}
+            value={String(userId)}


Same issue for the user select: the label has htmlFor="userSelect" but the <select> element lacks id="userSelect". Add the id to the select so the label is correctly associated.

mateacademy-ai-mentor · 2026-02-23T18:21:34Z

src/App.tsx

+    const newTodo: Todo = {
+      id: generateNewTodoId(),
+      title,
+      completed: false,
+      userId: +userId,
+    };


When creating newTodo you don't include the user object on the todo itself (you only attach users later in prepareTodos). The displayed list will have users, but the todo stored in state lacks user. If tests or later code expect each todo item in state to include a user object (as stated in the requirements: "each TODO item must have ... a user object"), consider adding user: getUserById(+userId) to newTodo so the state contains fully-prepared todos.

mateacademy-ai-mentor · 2026-02-23T18:21:34Z

src/components/UserInfo/UserInfo.tsx

+import { User } from '../../types/User';
+
+type Prop = {
+  user: User;


The prop is declared as user: User (non-optional) but callers might pass undefined. This can cause runtime errors when accessing properties. Consider making the prop optional (user?: User) and guarding rendering, or ensure callers always pass a valid User. This relates to CHECKLIST ITEM #4: "Don't render the component if the property that you pass to the component has null or undefined value".

mateacademy-ai-mentor · 2026-02-23T18:21:34Z

src/components/UserInfo/UserInfo.tsx

+
+export const UserInfo = ({ user }: Prop) => {
+  return (
+    <a className="UserInfo" href={`mailto:${user.email}`}>


This line accesses user.email directly. If user is undefined, this will throw. If you make the prop optional, guard before rendering (e.g. if (!user) return null;), or provide a fallback value.

mateacademy-ai-mentor · 2026-02-23T18:21:34Z

src/components/TodoInfo/TodoInfo.tsx

+    >
+      <h2 className="TodoInfo__title">{todo.title}</h2>
+
+      <UserInfo user={todo?.user} />


This passes todo?.user into UserInfo, which may be undefined. This violates CHECKLIST ITEM #4: "Don't render the component if the property that you pass to the component has null or undefined value". Consider rendering UserInfo only when todo.user is present (e.g. {todo.user ? <UserInfo user={todo.user} /> : null}).

mateacademy-ai-mentor · 2026-02-23T18:21:35Z

src/components/TodoInfo/TodoInfo.tsx

-export const TodoInfo = () => {};
+import cn from 'classnames';
+import { Todo } from '../../types/Todo';
+import { UserInfo } from '../UserInfo';


UserInfo is imported and used with a possibly undefined prop. Either ensure todo.user is always defined before passing it, or change UserInfo to accept an optional user prop (user?: User) and handle the missing-user case inside the component to avoid a type/runtime mismatch.

maximtsyrulnyk · 2026-02-23T18:42:54Z

edited code DEMO LINK

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are still a few checklist violations that can break requirements or tests. In src/App.tsx, the title label and input must both use htmlFor="titleInput" / id="titleInput", and the user label and select must both use htmlFor="userSelect" / id="userSelect"; right now they use title and userId, which conflicts with the previously specified identifiers and can cause automated tests to fail. Also in src/App.tsx, both onChange handlers currently use a one-letter parameter name like e, which violates [CHECKLIST ITEM #1]; rename these parameters to something descriptive such as event or changeEvent and update the usages accordingly. In src/components/TodoList/TodoList.tsx, you call todos.map(...) without guarding against todos being null or undefined, which goes against [CHECKLIST ITEM #4] and risks a runtime error; add a simple guard like if (!todos) return null; (or an equivalent check) before mapping. Once these id/label mismatches, event parameter names, and the todos null/undefined guard are fixed, your implementation will fully align with the requirements. Overall the structure and logic of your app look solid, so these are targeted cleanups to meet the project standards.

✨ 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-02-23T18:48:35Z

src/App.tsx

+          <label htmlFor="title">Title:</label>
+          <input
+            id="title"


This violates the previous review correction: "Form labels use htmlFor="titleInput" and htmlFor="userSelect" but the corresponding input and select elements lack matching id attributes; add id="titleInput" and id="userSelect" respectively." The label uses htmlFor="title" and the input id="title" — change both to use titleInput to match the expected identifiers used by tests and the previous review guidance.

mateacademy-ai-mentor · 2026-02-23T18:48:35Z

src/App.tsx

+          <label htmlFor="userId">User:</label>
+          <select
+            id="userId"
+            data-cy="userSelect"


This also violates the previous review correction quoted above. The user label uses htmlFor="userId" and the select id="userId" — change both to userSelect to match the required id names (userSelect) from the spec/previous review.

mateacademy-ai-mentor · 2026-02-23T18:48:35Z

src/App.tsx

+            onChange={e => {
+              setTitle(e.target.value);
+              setTitleError(false);
+            }}


This violates [CHECKLIST ITEM #1] "Don't use one-letter variable naming." Use a descriptive parameter name (for example event or changeEvent) instead of e in the title input onChange handler to improve readability and follow the style rule.

mateacademy-ai-mentor · 2026-02-23T18:48:35Z

src/App.tsx

+            onChange={e => {
+              setSelectedUserId(e.target.value);
+              setUserError(false);
+            }}


This violates [CHECKLIST ITEM #1] "Don't use one-letter variable naming." Replace e with a descriptive name (for example event or changeEvent) in the select onChange handler as well.

mateacademy-ai-mentor · 2026-02-23T18:48:35Z

src/components/TodoList/TodoList.tsx

+export const TodoList = ({ todos }: Props) => {
+  return (
+    <section className="TodoList">
+      {todos.map(todo => (


This violates checklist item #4: "Don't render the component if the property that you pass to the component has null or undefined value." TodoList calls todos.map without guarding against todos being null/undefined, which can throw at runtime. Add a guard such as if (!todos) return null; before mapping (or make todos optional and handle it).

maximtsyrulnyk · 2026-02-24T12:05:01Z

edited code according reccomendations

DEMO LINK

mateacademy-ai-mentor

Review Summary

Excellent work on this revision! I am approving your solution. You've successfully addressed all the feedback from the previous review, resulting in a clean, robust, and high-quality submission that fully meets all requirements. The form logic, validation, and component structure are all implemented perfectly. Keep up the fantastic work!

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

Rate AI review example ↩

done Add TODO form task

474f4bc

mateacademy-ai-mentor suggested changes Feb 23, 2026

View reviewed changes

edited code

c0cb189

maximtsyrulnyk requested a review from mateacademy-ai-mentor February 23, 2026 18:43

mateacademy-ai-mentor suggested changes Feb 23, 2026

View reviewed changes

edited code according reccomendations

e5c536c

maximtsyrulnyk requested a review from mateacademy-ai-mentor February 24, 2026 12:05

mateacademy-ai-mentor approved these changes Feb 24, 2026

View reviewed changes

		function generateNewTodoId() {
		return Math.max(...todos.map(todo => todo.id)) + 1;

Conversation

maximtsyrulnyk commented Feb 23, 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!

maximtsyrulnyk commented Feb 23, 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!

maximtsyrulnyk commented Feb 24, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants