Solution by cassiaqueiroz · Pull Request #1317 · mate-academy/react_todo-app

cassiaqueiroz · 2026-06-03T06:40:55Z

mateacademy-ai-mentor

Review Summary

Your implementation has one critical functional bug and three explicit checklist violations that must be addressed. The Clear Completed button in Footer.tsx has inverted logic - it's disabled when there ARE completed todos (todos.every(todo => !todo.completed)) instead of when there are NONE. The fix is to use todos.some(todo => todo.completed). Checklist item #7 requires using the classnames library for calculated classes throughout: Header.tsx, Filter.tsx, NewTodo.tsx, and TodoItem.tsx all use template literals instead. Checklist item #10 requires defining a FILTERS constant object with 'all', 'active', 'completed' values and using it everywhere filter strings appear - this is violated in Filter.tsx, TodoList.tsx, TodoItem.tsx, and the FilterType definition. Checklist item #4 requires using an internal ID counter instead of +new Date() in TodoContext.tsx's addTodo function, as rapid todo creation can produce duplicate IDs. Please fix these issues and resubmit.

✨ 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-06-03T07:15:33Z

+      <div className="todoapp">
+        <h1 className="todoapp__title">todos</h1>
+
+        <div className="todoapp__content">


Checklist item #7 requires using the classnames library for calculated classes instead of template literals like this.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+          <Header />
+          <TodoList />
+          <Footer />
+        </div>


Checklist item #10 requires creating constants for filter strings instead of using hardcoded string literals. Define a FILTERS constant object with 'all', 'active', 'completed' values.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+import { Header } from './components/Header';
+import { TodoList } from './components/TodoList';
+import { Footer } from './components/Footer';
+import { TodoProvider } from './context/TodoContext';


The onClick variable shadows the filter type's onClick method from useTodo. Consider renaming to avoid confusion and follow the naming convention that handler functions should start with 'handle'.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+
+        <div className="todoapp__content">
+          <Header />
+          <TodoList />


This handler is called inside the onClick callbacks, creating unnecessary nested arrow functions. Consider passing the filter type directly to setFilter or creating a dedicated handler.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+import { FilterType, useTodo } from '../../context/TodoContext';
+
+export const Filter: React.FC = () => {
+  const { filter, setFilter } = useTodo();


This violates checklist item #10 which requires using constants for filter strings. The values 'all', 'active', and 'completed' are used as raw strings throughout the file. Define a FILTERS constant object instead.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+        data-cy="FilterLinkAll"
+        onClick={event => {
+          event.preventDefault();
+          onClick('all');


This violates checklist item #10 which requires using constants for filter strings. Define a FILTERS constant object and use it here instead of the raw string 'all'.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+
+      <a
+        href="#/active"
+        className={`filter__link ${filter === 'active' ? 'selected' : ''}`}


This violates checklist item #10 which requires using constants for filter strings. Define a FILTERS constant object and use it here instead of the raw string 'active'.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+
+      <a
+        href="#/completed"
+        className={`filter__link ${filter === 'completed' ? 'selected' : ''}`}


This violates checklist item #10 which requires using constants for filter strings. Define a FILTERS constant object and use it here instead of the raw string 'completed'.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+        className={`filter__link ${filter === 'active' ? 'selected' : ''}`}
+        data-cy="FilterLinkActive"
+        onClick={event => {
+          event.preventDefault();


The disabled condition is inverted. Currently todos.every(todo => !todo.completed) returns true when there are NO completed todos (meaning the button SHOULD be disabled). But the button is DISABLED when true, so this logic is backwards. It should be !todos.every(todo => !todo.completed) or todos.some(todo => todo.completed) to disable when there are NO completed todos.

mateacademy-ai-mentor · 2026-06-03T07:15:33Z

+  const handleSubmit = (event: React.FormEvent) => {
+    event.preventDefault();
+
+    if (newTodoTitle.trim()) {


Checklist item #7 violation: Use the classnames library instead of template literals for calculated classes. Import classNames and use it here for the conditional 'active' class.

mateacademy-ai-mentor · 2026-06-03T07:15:38Z

+export const NewTodo: React.FC = () => {
+  const todoContext = useContext(TodoContext);
+
+  if (!todoContext) {


Checklist item #6 violation: Function name 'onClick' is too generic. According to naming conventions, event handlers should be named 'handleToggleAll' to indicate they handle an action.

mateacademy-ai-mentor · 2026-06-03T07:15:38Z

+          type="button"
+          className={`todoapp__toggle-all ${


Checklist item #7 violation: Use the classnames library instead of template literals for calculated classes.

mateacademy-ai-mentor · 2026-06-03T07:15:38Z

+
+export const Header: React.FC = () => {
+  const { todos, toggleAll } = useTodo();
+


Checklist item #6 violation: Function names should start with a verb and describe the result. Consider renaming to handleToggleAll instead of onClick.

mateacademy-ai-mentor · 2026-06-03T07:15:39Z

+import { useTodo } from '../../context/TodoContext';
+import { NewTodo } from '../NewTodo';
+
+export const Header: React.FC = () => {


Checklist item #7 violation: Use the classnames library instead of template literals for calculated classes.

mateacademy-ai-mentor · 2026-06-03T07:15:39Z

+import { useTodo } from '../../context/TodoContext';
+import { NewTodo } from '../NewTodo';
+
+export const Header: React.FC = () => {


Other components use the useTodo custom hook for accessing context. Using raw useContext(TodoContext) here creates inconsistency. Consider using useTodo hook like other components do.

mateacademy-ai-mentor · 2026-06-03T07:15:39Z

+    }
+  }, [isEditing]);
+
+  return (


Checklist item #7 violation: Use the classnames library for calculated classes instead of template literals. Import classNames and use it for the conditional 'completed' class.

mateacademy-ai-mentor · 2026-06-03T07:15:39Z

+
+export const TodoItem: React.FC<Props> = ({ todo }) => {
+  const { toggleTodo, deleteTodo, updateTodoTitle } = useTodo();
+  const [isEditing, setIsEditing] = useState(false);


Checklist item #10 violation: Use constants for filter strings instead of hardcoded values. Define a FILTERS constant object with 'all', 'active', 'completed' values.

mateacademy-ai-mentor · 2026-06-03T07:15:39Z

+  const { toggleTodo, deleteTodo, updateTodoTitle } = useTodo();
+  const [isEditing, setIsEditing] = useState(false);
+  const [newTodoTitle, setNewTodoTitle] = useState(todo.title);
+


Checklist item #10 violation: Use constants for filter strings instead of hardcoded values.

mateacademy-ai-mentor · 2026-06-03T07:15:39Z

+  const [newTodoTitle, setNewTodoTitle] = useState(todo.title);
+
+  useEffect(() => {
+    setNewTodoTitle(todo.title);


Checklist item #10 violation: Use constants for filter strings instead of hardcoded values.

mateacademy-ai-mentor · 2026-06-03T07:15:39Z

+
+  const filteredTodos = todos.filter(todo => {
+    switch (filter) {
+      case 'active':


Checklist item #10 violation: Use constants for filter values instead of hardcoded strings. Define a FILTERS constant object with 'all', 'active', 'completed' values and use FILTERS.active here.

mateacademy-ai-mentor · 2026-06-03T07:15:44Z

+    switch (filter) {
+      case 'active':
+        return !todo.completed;
+      case 'completed':


Checklist item #10 violation: Use constants for filter values instead of hardcoded strings. Use FILTERS.completed here.

mateacademy-ai-mentor · 2026-06-03T07:15:44Z

+        return !todo.completed;
+      case 'completed':
+        return todo.completed;
+      default:


Checklist item #10 violation: Use constants for filter values instead of hardcoded strings. Use FILTERS.all here.

mateacademy-ai-mentor · 2026-06-03T07:15:44Z

+  }, [todos]);
+
+  const addTodo = (title: string) => {
+    const newTodo: Todo = {


Checklist item #4 violation: Use an internal ID counter instead of +new Date() for generating todo IDs. Create a useId ref or counter variable that increments for each new todo. This ensures uniqueness even when creating multiple todos rapidly.

mateacademy-ai-mentor · 2026-06-03T07:15:44Z

    "src"
  ],
  "compilerOptions": {
+    "jsx": "react-jsx",


Checklist item #10 violation: Use constants for filter values instead of hardcoded strings. Define a FILTERS constant object with 'all', 'active', 'completed' values and reference them here.

mateacademy-ai-mentor · 2026-06-03T07:15:44Z

+      "node",
+      "cypress"
+    ]
  }


The onDelete callback parameter in deleteTodo is defined but never used in any component. According to checklist #2, callbacks should take prepared data. This optional callback appears unnecessary - consider removing it to simplify the interface.

mateacademy-ai-mentor

Review Summary

Your implementation successfully meets all requirements. The Clear Completed button correctly uses todos.some() for its disabled logic, the classnames library is properly applied for conditional classes in Header, Filter, and TodoItem components, the FILTERS constant is defined in utils/filters.ts and imported throughout the codebase, and the TodoContext uses an internal ID counter (nextId++) instead of +new Date(). All core functionality works as expected: todo creation with validation and trimming, inline editing with double-click/Enter/Escape/blur handling, toggle individual/all todos, filtering by All/Active/Completed, localStorage persistence with JSON.stringify, and proper disabled states for action buttons. No critical issues or blockers were found. Great work on implementing all checklist items correctly!

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

Rate AI review example ↩

Solution

5019abd

mateacademy-ai-mentor suggested changes Jun 3, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Jun 3, 2026

View reviewed changes

Corrections

d241d2f

cassiaqueiroz requested a review from mateacademy-ai-mentor June 3, 2026 20:21

mateacademy-ai-mentor approved these changes Jun 3, 2026

View reviewed changes


		export const Header: React.FC = () => {
		const { todos, toggleAll } = useTodo();

Conversation

cassiaqueiroz commented Jun 3, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants