Solution by HiBlurryface · Pull Request #1277 · mate-academy/react_todo-app

HiBlurryface · 2026-02-20T16:28:09Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because several critical requirements and tests are currently not satisfied, so the task is not yet complete. The NewTodo input is not auto-focused when there are no todos, which directly causes the end‑to‑end test failure for the “Page with no todos” case, and the NewTodo handler allows creating empty/whitespace todos without trimming, violating the “no empty todos” and “trim spaces” requirements [CHECKLIST ITEMS #8, #9]. The toggle‑all behavior is driven by a local flag instead of being derived from the actual todos array, so it can get out of sync and break the rules about when toggle‑all should be active or inactive [CHECKLIST ITEMS #5–#7]. Inline editing does not trim the edited title or delete whitespace‑only titles, and the Clear completed button is never disabled even when there are no completed todos, which both conflict with the stated behavior requirements. In multiple places you rely on raw filter strings ('all', 'active', 'completed') instead of a shared FILTERS constant and you build conditional class names with ternaries instead of the required classnames utility [CHECKLIST ITEMS #7, #10].

The most important fixes to make are:

In Header.tsx, focus the NewTodo input when todos.length === 0, trim the input before dispatch, and skip dispatch if the trimmed string is empty.
In Header.tsx, derive the toggle‑all state from todos (e.g. todos.length > 0 && todos.every(t => t.completed)) instead of a local boolean, and compute its className via classnames.
In TodoItem.tsx, trim the edited title before saving, delete the todo if the trimmed title is empty, use classnames for the todo’s class, and fix the cancel‑editing handler name.
In Footer.tsx, compute how many todos are completed, disable Clear completed when there are none, and switch all filter comparisons/onClick handlers to use a shared FILTERS constant plus classnames for active link styling.
In SortReducer, SortContext, TodoList, App, and related files, introduce and reuse a FILTERS constant for all filter values and tighten the typing/contexts accordingly, and fix the default dispatch typings in the contexts.

Once you address these points, your overall structure (contexts, reducers, basic UI) is solid, and the app should pass the tests and align well with the checklist; you’re close, and these refinements will significantly improve both correctness and code quality.

✨ 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-20T16:46:06Z

src/components/footer/Footer.tsx

@@ -0,0 +1,66 @@
+import { useContext, useEffect, useState } from 'react';


Missing import/use of the classnames library. The project requires: "Use classnames library for calculated classes. [CHECKLIST ITEM #7]" Add an import (e.g. import classNames from 'classnames') and use it to compute the link classes instead of manual ternaries.

mateacademy-ai-mentor · 2026-02-20T16:46:06Z

src/components/footer/Footer.tsx

+      <nav className="filter" data-cy="Filter">
+        <a
+          href="#/"
+          className={


This builds className with a ternary and checks sortBy === 'all' directly. This violates the requirement to use constants for filter values: "Do not rely on an unknown string; make constants for this (e.g., FILTERS = { all: 'all', completed: 'completed', active: 'active' }). [CHECKLIST ITEM #10]" Also, use classnames instead of manual ternary here.

mateacademy-ai-mentor · 2026-02-20T16:46:06Z

src/components/footer/Footer.tsx

+
+        <a
+          href="#/active"
+          className={


This comparison uses the literal 'active' for the filter and builds the className via ternary. Please use a FILTERS constant instead of raw strings: "Do not rely on an unknown string; make constants for this (e.g., FILTERS = { all: 'all', completed: 'completed', active: 'active' }). [CHECKLIST ITEM #10]" and compute classes with classnames.

mateacademy-ai-mentor · 2026-02-20T16:46:06Z

src/components/footer/Footer.tsx

+
+        <a
+          href="#/completed"
+          className={


This comparison uses the literal 'completed' and a ternary for className. Replace the string with a FILTERS constant and use classnames for calculated classes: "Use classnames library for calculated classes. [CHECKLIST ITEM #7]" and "Do not rely on an unknown string... [CHECKLIST ITEM #10]".

mateacademy-ai-mentor · 2026-02-20T16:46:06Z

src/components/footer/Footer.tsx

+            sortBy === 'all' ? 'filter__link selected' : 'filter__link'
+          }
+          data-cy="FilterLinkAll"
+          onClick={() => sortDispatch('all')}


The onClick here calls sortDispatch('all') with a raw string literal. Instead, pass a constant from FILTERS (e.g. sortDispatch(FILTERS.all)) to satisfy: "Do not rely on an unknown string; make constants for this (...)" [CHECKLIST ITEM #10]. This makes filter values explicit and centralised.

mateacademy-ai-mentor · 2026-02-20T16:46:06Z

src/components/footer/Footer.tsx

+            sortBy === 'active' ? 'filter__link selected' : 'filter__link'
+          }
+          data-cy="FilterLinkActive"
+          onClick={() => sortDispatch('active')}


The onClick uses sortDispatch('active') with a raw string literal. Use a FILTERS constant instead (e.g. sortDispatch(FILTERS.active)) as required: "Do not rely on an unknown string..." [CHECKLIST ITEM #10].

mateacademy-ai-mentor · 2026-02-20T16:46:07Z

src/components/footer/Footer.tsx

+            sortBy === 'completed' ? 'filter__link selected' : 'filter__link'
+          }
+          data-cy="FilterLinkCompleted"
+          onClick={() => sortDispatch('completed')}


The onClick uses sortDispatch('completed') with a raw string literal. Use a FILTERS constant instead (e.g. sortDispatch(FILTERS.completed)) to comply with: "Do not rely on an unknown string..." [CHECKLIST ITEM #10].

mateacademy-ai-mentor · 2026-02-20T16:46:07Z

src/components/footer/Footer.tsx

+        </a>
+      </nav>
+
+      <button


The Clear completed button is always enabled. The requirements state: "Implement the clearCompleted button (disabled if there are no completed todos)." You should compute whether there are any completed todos (e.g. const completedCount = todos.filter(t => t.completed).length) and add disabled={completedCount === 0} to this button, so it's disabled when there are no completed todos.

mateacademy-ai-mentor · 2026-02-20T16:46:07Z

src/components/TodoItem.tsx/TodoItem.tsx

+  const [isEditing, setIsEditing] = useState(false);
+  const inputRef = useRef<HTMLInputElement>(null);
+
+  const canselEditings = (event: React.KeyboardEvent<HTMLInputElement>) => {


This function name is misspelled and does not follow the naming conventions required by the checklist. The checklist states: "A function name should describe the result and start from a verb. [CHECKLIST ITEM #6]" and also: "Follow the linked naming conventions for event handler functions and props in React. [CHECKLIST ITEM #6]" Consider renaming to a clearer verb-based name like cancelEditing or handleCancelEditing and fix the spelling.

mateacademy-ai-mentor · 2026-02-20T16:46:07Z

src/components/TodoItem.tsx/TodoItem.tsx

+  const editTodo = () => {
+    if (title === '') {
+      dispatch(deleteTodoAction(todo.id));
+    } else {
+      dispatch(renameTodoAction(todo.id, title));
+    }
+
+    setIsEditing(false);


The editTodo handler uses the raw title value when checking for emptiness and when renaming. This violates the inline-editing requirements: "Trim the saved text." and "Delete the todo if the title is empty." You should trim the title (e.g. const trimmed = title.trim()) before checking trimmed === '' and before dispatching renameTodoAction with the trimmed value.

mateacademy-ai-mentor · 2026-02-20T16:46:11Z

src/components/TodoItem.tsx/TodoItem.tsx

+  return (
+    <div
+      data-cy="Todo"
+      className={todo.completed ? 'todo completed' : 'todo'}


Conditional class assignment uses a ternary instead of the required classnames utility. The checklist requires: "Use classnames library for calculated classes. [CHECKLIST ITEM #7]" Replace className={todo.completed ? 'todo completed' : 'todo'} with something like classNames('todo', { completed: completed }) after destructuring.

mateacademy-ai-mentor · 2026-02-20T16:46:11Z

src/components/TodoItem.tsx/TodoItem.tsx

+
+export const TodoItem = ({ todo }: Props) => {
+  const { dispatch } = useContext(TodoContext);
+  const [title, setTitle] = useState(todo.title);


You access todo properties repeatedly instead of destructuring them. The checklist requires: "Use destructuring wherever possible. It makes code more readable. [CHECKLIST ITEM #2]" Consider destructuring at the top of the component: const { id, title: initialTitle, completed } = todo; and update usages accordingly (also adjust state init to use initialTitle).

mateacademy-ai-mentor · 2026-02-20T16:46:11Z

src/App.tsx

+      completed: false,
+    },
+  ]);
+  const [sortBy, sortDispatch] = useReducer(SortReducer, 'all');


This file uses a raw string literal for the initial filter state: useReducer(SortReducer, 'all'). This violates checklist item #10: "Do not rely on an unknown string; make constants for this (e.g., FILTERS = { all: 'all', completed: 'completed', active: 'active' })." Use a shared FILTERS constant (e.g. FILTERS.all) here so the code is explicit and consistent with the rest of the app.

mateacademy-ai-mentor · 2026-02-20T16:46:12Z

src/App.tsx

+      title: 'Completed Todo',
+      completed: false,


The third initial todo has the title Completed Todo but completed is set to false. This is a logical mismatch in sample data and may confuse testing or UI behavior. Either change the title or set completed: true so the sample data reflects its label correctly.

mateacademy-ai-mentor · 2026-02-20T16:46:12Z

src/components/TodoList/TodoList.tsx

+import { useContext } from 'react';
+import { SortContext } from '../../store/SortContext';
+import { TodoContext } from '../../store/TodoContext';
+import { TodoItem } from '../TodoItem.tsx/TodoItem';


Invalid import path will cause module resolution/build errors. The import reads ../TodoItem.tsx/TodoItem which includes the .tsx extension inside the path and likely doesn't match the real folder/file structure. Update the import to the correct module path (for example ../TodoItem/TodoItem) or the actual relative path used in your project so the component can be resolved.

mateacademy-ai-mentor · 2026-02-20T16:46:12Z

src/components/TodoList/TodoList.tsx

+    if (sortBy === 'active') {
+      return !todo.completed;
+    }
+
+    if (sortBy === 'completed') {
+      return todo.completed;


This uses raw string literals ('active' / 'completed') for filtering. This violates checklist item #10: "Do not rely on an unknown string; make constants for this (e.g., FILTERS = { all: 'all', completed: 'completed', active: 'active' })." Replace these literals by importing and using a FILTERS constant (for example if (sortBy === FILTERS.active)) so the code relies on a single source of truth for filter names.

mateacademy-ai-mentor · 2026-02-20T16:46:12Z

src/store/SortReducer.tsx

+type State = string;
+
+type Action = 'all' | 'completed' | 'active';


This file relies on string literals for filter values instead of a constant. This violates the checklist item: "Do not rely on an unknown string; make constants for this (e.g., FILTERS = { all: 'all', completed: 'completed', active: 'active' }). [CHECKLIST ITEM #10]" Define a FILTERS constant and use it to type State/Action (or derive types from it) to satisfy the requirement and improve type-safety.

mateacademy-ai-mentor · 2026-02-20T16:46:12Z

src/store/TodoContext.tsx

+
+export const TodoContext = createContext<Props>({
+  todos: [],
+  dispatch: () => {},


The default dispatch value () => {} does not match the required React.Dispatch<Action> function signature (Dispatch expects a single Action parameter). This will cause a TypeScript type error. Provide a properly-typed no-op (for example: dispatch: (() => null) as React.Dispatch<Action>), or use createContext<Props>({} as Props) to avoid the mismatch.

mateacademy-ai-mentor · 2026-02-20T16:46:12Z

src/header/Header.tsx

+
+  const addTodo = (event: React.SyntheticEvent<HTMLFormElement>) => {
+    event.preventDefault();
+    dispatch(addTodoAction(newTodo));


This dispatchs the raw newTodo value without trimming or validation. This violates checklist item #8: "NewTodo form shouldn’t create empty todos." and checklist item #9: "NewTodo form should trim redundant spaces." Trim the input (e.g. const text = newTodo.trim()) and only dispatch when text !== ''.

mateacademy-ai-mentor · 2026-02-20T16:46:12Z

src/header/Header.tsx

+export const Header = () => {
+  const { todos, dispatch } = useContext(TodoContext);
+  const [newTodo, setNewTodo] = useState('');
+  const [todosIsDone, setTodosIsDone] = useState(false);


Using a local todosIsDone state to represent whether all todos are completed will get out of sync with the actual todos array. This breaks the requirements: "“Toggle all” should be active only in case when all todos are completed. [CHECKLIST ITEM #5]", "If you manually toggle all todos to completed state, “Toggle all” should stay active. [CHECKLIST ITEM #6]", and ""Toggle all" should stay inactive if at least one todo is not completed. [CHECKLIST ITEM #7]". Instead, derive the boolean from todos (e.g. const allCompleted = todos.length > 0 && todos.every(t => t.completed)) and drive UI from that.

mateacademy-ai-mentor · 2026-02-20T16:46:17Z

src/header/Header.tsx

+  const completeAll = () => {
+    const newValue = !todosIsDone;
+
+    setTodosIsDone(newValue);
+    dispatch(completeAllAction(newValue));


The completeAll handler toggles the local todosIsDone and dispatches that value; because the local state isn't derived from todos, the dispatched value can be incorrect. Dispatch based on the derived allCompleted value (for example dispatch(completeAllAction(!allCompleted))) and remove/update local todosIsDone accordingly.

mateacademy-ai-mentor · 2026-02-20T16:46:17Z

src/header/Header.tsx

+          className={
+            todosIsDone ? 'todoapp__toggle-all active' : 'todoapp__toggle-all'
+          }


Conditional class assignment uses a ternary instead of the classnames library, which violates: "Use classnames library for calculated classes. [CHECKLIST ITEM #7]" Replace todosIsDone ? 'todoapp__toggle-all active' : 'todoapp__toggle-all' with something like classNames('todoapp__toggle-all', { active: allCompleted }) after deriving allCompleted from todos.

mateacademy-ai-mentor · 2026-02-20T16:46:17Z

src/store/SortContext.tsx

+type Action = 'all' | 'completed' | 'active';
+
+export interface Props {
+  sortBy: string;


Props.sortBy is declared as a string but the file defines an Action union that should be used instead. The checklist requires: "Do not rely on an unknown string; make constants for this (e.g., FILTERS = { all: 'all', completed: 'completed', active: 'active' }). [CHECKLIST ITEM #10]— changesortBy: stringto use theAction` type or a FILTERS constant so the sort value is constrained.

mateacademy-ai-mentor · 2026-02-20T16:46:17Z

src/store/SortContext.tsx

+}
+
+export const SortContext = createContext<Props>({
+  sortBy: '',


You provide a default sortBy: '' in the createContext default value. That empty string is not one of the Action values and contradicts the requirement to use concrete filter values / constants. Per checklist item #10 (quoted above), define and use a FILTERS constant (e.g. FILTERS.all) or at least default to a valid action like 'all'.

mateacademy-ai-mentor · 2026-02-20T16:46:17Z

src/store/SortContext.tsx

+
+export const SortContext = createContext<Props>({
+  sortBy: '',
+  sortDispatch: () => {},


sortDispatch default is () => {} but the declared type is React.Dispatch<Action> (i.e. a function that should accept an Action argument). Provide a noop with the correct signature, e.g. (value: Action) => {}, or cast appropriately. The current zero-argument function may be incompatible with strict function typing and lead to TypeScript errors.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are still several critical requirement and checklist violations that will break expected behavior and tests. The app starts with seeded todos in src/App.tsx, which conflicts with the “page with no todos” tests and the requirement that the list be initially empty or loaded from localStorage; this needs to be changed so the reducer initializes from an empty array or from localStorage. Multiple places still use raw filter strings instead of a shared FILTERS constant (App.tsx, TodoList.tsx, Footer.tsx, SortContext.tsx, SortReducer.tsx), which violates [CHECKLIST ITEM #10]; introduce a single exported FILTERS object and use its values and types everywhere instead of 'all' | 'active' | 'completed' literals. In Header.tsx, the new todo value is dispatched without trimming and the input is focused on every todos change rather than only when todos.length === 0, so you should trim before dispatching (and skip when empty) and guard the focus effect with a todos.length === 0 check. In TodoItem.tsx, saving an edited title still uses the untrimmed title instead of title.trim(), doesn’t delete when the trimmed title is empty, the cancel-edit handler name is unclear, and title state is not synchronized when todo.title changes; please apply the suggested pattern of trimming, possibly deleting on empty, and adding a useEffect that updates local title when todo.title changes. Finally, both TodoContext.tsx and SortContext.tsx use incorrectly typed default dispatch functions and, in SortContext, an invalid default sortBy value; update these to use a correctly typed noop or error-throwing function and a valid default filter (e.g. FILTERS.all) so the contexts are type-safe. Once these functional and checklist issues are addressed, the rest of your structure (reducers, basic UI, and styling) looks solid and should align well with the project requirements.

✨ 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-20T17:31:13Z

src/components/TodoItem.tsx/TodoItem.tsx

+    } else {
+      dispatch(renameTodoAction(todo.id, title));


This dispatch uses the raw title instead of the trimmed value. The task requires trimming saved text: "Trim the saved text." Also the previous review asked: "In TodoItem.tsx, trim the edited title before saving, and delete the todo if the trimmed title is empty (to match “Trim the saved text” and “Delete the todo if the title is empty” requirements)." Change to dispatch the trimmed title and update local state accordingly (for example: const trimmed = title.trim(); if (!trimmed) dispatch(deleteTodoAction(todo.id)); else { dispatch(renameTodoAction(todo.id, trimmed)); setTitle(trimmed); }).

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/components/TodoItem.tsx/TodoItem.tsx

+  const [isEditing, setIsEditing] = useState(false);
+  const inputRef = useRef<HTMLInputElement>(null);
+
+  const cancelEditings = (event: React.KeyboardEvent<HTMLInputElement>) => {


Handler name cancelEditings is unclear/grammatically odd. This violates the previous request: "In TodoItem.tsx, fix the cancel‑editing handler name (to correctly reflect its purpose)." Rename it to something explicit (e.g. handleCancelEditing or cancelEditing) to make intent obvious.

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/components/TodoItem.tsx/TodoItem.tsx

+
+export const TodoItem = ({ todo }: Props) => {
+  const { dispatch } = useContext(TodoContext);
+  const [title, setTitle] = useState(todo.title);


title state is initialized from todo.title but never synchronized when todo.title changes after a rename from the context. That can leave the input showing a stale value when re-entering edit mode. Add a useEffect to update title when todo.title changes, e.g. useEffect(() => setTitle(todo.title), [todo.title]), so the local input always reflects the current todo value.

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/App.tsx

+      completed: false,
+    },
+  ]);
+  const [sortBy, sortDispatch] = useReducer(SortReducer, 'all');


This uses a raw string 'all' as the initial sort value. The task requires using a shared FILTERS constant instead of unknown string literals. Please use the FILTERS constant (for example FILTERS.all) so all files reference the same values.

Checklist violation (quote):
"Do not rely on the unknown string, make constants for this: [CHECKLIST ITEM #10]

const FILTERS = { all: 'all', completed: 'completed', active: 'active', }; ```","lineStart":30,

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/App.tsx

+      title: 'Completed Todo',
+      completed: false,


The todo with title Completed Todo is declared with completed: false. This is inconsistent and likely to cause incorrect completed/active counts in the UI/tests. Either set completed: true for this item or rename the title to reflect that it's not completed.

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/header/Header.tsx

+    if (newTodo.trim().length > 0) {
+      dispatch(addTodoAction(newTodo));


This dispatch sends the raw newTodo instead of a trimmed string. The task requires trimming the NewTodo input before creating a todo. This violates: "NewTodo form should trim redundant spaces. [CHECKLIST ITEM #9]" and the previous requested fix: "In Header.tsx, trim the NewTodo input value before dispatching, and skip dispatch if the trimmed string is empty (to satisfy “no empty todos” and “trim spaces” requirements)." Change to trim before dispatch (e.g. const trimmed = newTodo.trim(); if (trimmed.length > 0) dispatch(addTodoAction(trimmed));).

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/header/Header.tsx

+  useEffect(() => {
+    if (inputRef.current) {
+      inputRef.current.focus();
+    }
+  }, [todos]);


This effect focuses the input on every todos change. The previous requested fix required focusing the NewTodo input only when there are no todos: "In Header.tsx, focus the NewTodo input when todos.length === 0." Currently the effect runs unconditionally and may steal focus unexpectedly. Restrict the focus to the empty-list case (for example: if (todos.length === 0 && inputRef.current) inputRef.current.focus();) and consider using todos.length in the dependency array.

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/store/SortReducer.tsx

@@ -0,0 +1,16 @@
+type State = string;
+
+type Action = 'all' | 'completed' | 'active';


This file uses raw filter string literals instead of a shared FILTERS constant. This violates the checklist item: "Do not rely on the unknown string, make constants for this: [CHECKLIST ITEM #10] ts const FILTERS = { all: 'all', completed: 'completed', active: 'active', }; " Introduce and export FILTERS and use it in the reducer and types so filter values are centralized and typings can be tightened (e.g. type Action = typeof FILTERS[keyof typeof FILTERS]).

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/store/SortReducer.tsx

+export const SortReducer = (state: State, action: Action): State => {
+  switch (action) {
+    case 'all':
+      return 'all';
+    case 'completed':
+      return 'completed';
+    case 'active':
+      return 'active';
+    default:
+      return state;
+  }


The switch cases and returned values are literal strings ('all', 'completed', 'active'). Replace these with the corresponding FILTERS values (for example case FILTERS.all: return FILTERS.all;) so the reducer and other modules share the same constants and cannot drift apart.

mateacademy-ai-mentor · 2026-02-20T17:31:13Z

src/store/TodoContext.tsx

+export const TodoContext = createContext<Props>({
+  todos: [],
+  dispatch: () => {},


This uses an untyped noop function as the default dispatch. That violates the previous request: "MEDIUM: In React context files, fix the default dispatch typings in the contexts (so they are correctly typed)." The dispatch in the default value should match React.Dispatch<Action> (for example dispatch: (() => null) as unknown as React.Dispatch<Action> or dispatch: (() => { throw new Error('dispatch not implemented') }) as React.Dispatch<Action>), so the context's default value is correctly typed and safer to use.

mateacademy-ai-mentor

Review Summary

Excellent progress on this PR! You've successfully addressed most of the critical feedback from the previous review, and the application's core functionality is nearly complete. The overall structure using context and reducers is solid.

I am approving this submission to help you keep moving forward, but please take note of the remaining issues for your next iteration. The most critical one is the inconsistent use of shared constants for filtering. Although you created src/store/filters.tsx, several files (App.tsx, TodoList.tsx, SortReducer.tsx) still use raw strings like 'all' and 'active', which violates a key requirement [CHECKLIST ITEM #10].

Additionally, please address the duplicated action type constants between TodoContext.tsx and TodoReducer.tsx and ensure the data-cy attributes in Footer.tsx are unique for each filter link. 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-02-21T15:01:59Z

src/components/footer/Footer.tsx

+          return (
+            <a
+              key={item}
+              href={`#/${item === 'all' ? '' : item}`}


This comparison uses the raw string 'all'. To fully adhere to checklist item #10 ("Do not rely on the unknown string, make constants for this"), it would be better to compare against a value from your Filters constant, for example item === Filters[0].

mateacademy-ai-mentor · 2026-02-21T15:01:59Z

src/components/footer/Footer.tsx

+              className={classNames('filter__link', {
+                selected: sortBy === item,
+              })}
+              data-cy="FilterLinkAll"


The data-cy attribute is the same for all three filter links created by this loop. For testing purposes, these attributes should be unique. Consider making them dynamic, for example, by incorporating the item value.

mateacademy-ai-mentor · 2026-02-21T15:01:59Z

src/components/TodoList/TodoList.tsx

+    if (sortBy === 'active') {
+      return !todo.completed;
+    }
+
+    if (sortBy === 'completed') {
+      return todo.completed;
+    }


This violates checklist item #10: 'Do not rely on the unknown string, make constants for this.' Instead of hardcoding the strings 'active' and 'completed', you should import the Filters constant from ../../store/filters.tsx and use its values for comparison. This will make your code more robust against typos and align with the requirement to use constants.

mateacademy-ai-mentor · 2026-02-21T15:01:59Z

src/components/TodoItem.tsx/TodoItem.tsx

+    setTitle(todo.title);
+  }, [todo.title]);
+
+  const cancelEditings = (event: React.KeyboardEvent<HTMLInputElement>) => {


This function name is a bit unconventional. Per checklist item #6, function names should be clear and follow conventions. A name like handleKeyUp or cancelEdit would be more standard and descriptive, as it handles the key-up event to cancel a single edit.

mateacademy-ai-mentor · 2026-02-21T15:01:59Z

src/components/TodoItem.tsx/TodoItem.tsx

+    <div
+      data-cy="Todo"
+      className={classNames('todo', { completed: todo.completed })}
+      key={todo.id}


This key prop has no effect here. React requires keys on elements within an array, which are provided in the parent component (TodoList.tsx in this case) where you map over the list. Placing it on the root element inside the child component is redundant and demonstrates a misunderstanding of how keys work. This violates checklist item #8: 'Use key attribute correctly'.

mateacademy-ai-mentor · 2026-02-21T15:01:59Z

src/App.tsx

+    localStorage.setItem('todos', JSON.stringify(todos));
+  }, [todos]);
+
+  const [sortBy, sortDispatch] = useReducer(SortReducer, 'all');


This violates checklist item #10: "Do not rely on the unknown string, make constants for this."

The initial state for the filter reducer is hardcoded as 'all'. You should import the Filters constant from src/store/filters.tsx and use a value from it (e.g., Filters[0]) to ensure consistency and avoid magic strings.

mateacademy-ai-mentor · 2026-02-21T15:01:59Z

src/store/SortReducer.tsx

+    case 'all':
+      return 'all';
+    case 'completed':
+      return 'completed';
+    case 'active':
+      return 'active';


This violates checklist item #10: "Do not rely on the unknown string, make constants for this."

Instead of hardcoding strings like 'all', 'completed', and 'active' in your case statements, you should import the Filters constant and use its values. This ensures that your reducer logic is consistent with the filter definitions used elsewhere in the app.

mateacademy-ai-mentor · 2026-02-21T15:02:00Z

src/store/TodoContext.tsx

+const ADD_TODO = 'ADD_TODO';
+const COMPLETE_TODO = 'COMPLETE_TODO';
+const COMPLETE_ALL = 'COMPLETE_ALL';
+const DELETE_TODO = 'DELETE_TODO';
+const DELETE_COMPLETED = 'DELETE_COMPLETED';
+const RENAME_TODO = 'RENAME_TODO';


These action type constants are also defined in TodoReducer.tsx. Duplicating constants like this is risky because if one file is updated and the other isn't, it can lead to bugs that are hard to track down. It would be better to define these constants once, export them from this file, and then import them into TodoReducer.tsx.

mateacademy-ai-mentor · 2026-02-21T15:02:00Z

src/store/TodoContext.tsx

+
+export const TodoContext = createContext<Props>({
+  todos: [],
+  dispatch: (() => { }) as React.Dispatch<Action>,


This no-op function works, but it can silently hide bugs if a component that uses this context is rendered outside of its provider. For consistency with SortContext.tsx and for better error reporting, consider implementing a default dispatch that throws an error, for example:

() => { throw new Error('dispatch must be used within a TodoContext.Provider'); }

mateacademy-ai-mentor · 2026-02-21T15:02:00Z

src/store/TodoReducer.tsx

+const ADD_TODO = 'ADD_TODO';
+const COMPLETE_TODO = 'COMPLETE_TODO';
+const COMPLETE_ALL = 'COMPLETE_ALL';
+const DELETE_TODO = 'DELETE_TODO';
+const DELETE_COMPLETED = 'DELETE_COMPLETED';
+const RENAME_TODO = 'RENAME_TODO';


These action type constants are also defined in TodoContext.tsx. Duplicating constants across files is risky because if a change is made in one file but not the other, it can lead to bugs. To avoid this, you should define these constants in a single file (like TodoContext.tsx), export them, and then import them here.

Solution

c7b9176

mateacademy-ai-mentor suggested changes Feb 20, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Feb 20, 2026

View reviewed changes

Solution

cab59ae

HiBlurryface requested a review from mateacademy-ai-mentor February 20, 2026 17:23

mateacademy-ai-mentor suggested changes Feb 20, 2026

View reviewed changes

Solution

571aaf8

HiBlurryface requested a review from mateacademy-ai-mentor February 21, 2026 14:59

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

View reviewed changes

		@@ -0,0 +1,66 @@
		import { useContext, useEffect, useState } from 'react';

+                      </a>
+                    </nav>
+                    <button

		type State = string;

		type Action = 'all' \| 'completed' \| 'active';

		if (newTodo.trim().length > 0) {
		dispatch(addTodoAction(newTodo));

		@@ -0,0 +1,16 @@
		type State = string;

		type Action = 'all' \| 'completed' \| 'active';

Comments

Conversation

HiBlurryface commented Feb 20, 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!

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!

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!

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!