need help #858

HYPER2307 · 2023-10-06T18:55:44Z

sTorba24

Good start! Keep it up :)

Ask for help in your fe_chat and try to find a solution

then request review on github(only when you solve issues, because it's hard to contact on github, mentors can see your comments only after your task appears in queue).

HYPER2307 · 2023-10-09T20:31:28Z

this problems in form edit

lerastarynets

I see you did a hard work) Let's improve it a little bit

lerastarynets · 2023-10-10T07:17:30Z

src/TodosContext/TodosContext.tsx

+  todos: [],
+  setTodos: () => {},
+  errorMessage: '',
+  setErrorMessage: () => {},
+  preparedTodos: [],
+  sortQuery: '',
+  setSortQuery: () => {},
+  tempTodo: {
+    id: 0,
+    userId: 0,
+    title: '',
+    completed: false,
+  },
+  setTempTodo: () => {},
+  todosInProcess: [],
+  setTodosInProcess: () => {},
+  completedTodos: [],
+});


try reducing the amount of props in the component, it's bad practice to have big amount of props

lerastarynets · 2023-10-10T07:19:21Z

src/TodosContext/TodosContext.tsx

+  const preparedTodos = todos?.filter(todo => {
+    switch (sortQuery as SortType) {
+      case SortType.Active:
+        return todo.completed === false;


Suggested change

return todo.completed === false;

return !todo.completed;

lerastarynets · 2023-10-10T07:19:32Z

src/TodosContext/TodosContext.tsx

+        return todo.completed === false;
+
+      case SortType.Completed:
+        return todo.completed === true;


Suggested change

return todo.completed === true;

return todo.completed;

lerastarynets · 2023-10-10T07:19:53Z

src/TodosContext/TodosContext.tsx

+  }) || todos;
+
+  const completedTodos = todos?.filter(todo => (
+    todo.completed !== false


Suggested change

todo.completed !== false

todo.completed

lerastarynets · 2023-10-10T07:21:17Z

src/components/ErrorsNotifications.tsx

+      {/* <br />
+          Unable to delete a todo
+          <br />
+          Unable to update a todo */}


what is the purpose of these commented blocks?

lerastarynets · 2023-10-10T07:21:30Z

src/components/Footer.tsx

+  } = useTodos();
+
+  const notCompletedTodosLength = todos?.filter(todo => (
+    todo.completed === false


Suggested change

todo.completed === false

!todo.completed

lerastarynets · 2023-10-10T07:21:38Z

src/components/Footer.tsx

+  )).length;
+
+  const completedTodos = todos?.filter(todo => (
+    todo.completed !== false


Suggested change

todo.completed !== false

todo.completed

lerastarynets · 2023-10-10T07:22:18Z

src/components/Header.tsx

+  };
+
+  const toggleAllTodos = () => {
+    if (activeTodos?.length === 0) {


Suggested change

if (activeTodos?.length === 0) {

if (!activeTodos?.length) {

lerastarynets · 2023-10-10T07:23:48Z

src/components/TodoItem.tsx

+            onDoubleClick={() => {
+              setNewTitle(newTitle.trim());
+              setIsEditing(true);
+            }}


Pls move the function inside onDoubleClick handler to a separate const and use as a callback here

lerastarynets · 2023-10-10T07:24:12Z

src/components/TodoItem.tsx

+            placeholder="Empty todo will be deleted"
+            value={newTitle}
+            onChange={(event) => setNewTitle(event.target.value)}
+            onBlur={() => handleOnBlur()}


Suggested change

onBlur={() => handleOnBlur()}

onBlur={handleOnBlur}

IvanFesenko

Toggle all button works incorrect - it should make all uncompleted todos completed without additional requests, other way if all todos are completed - they should became active
https://github.com/mate-academy/react_todo-app-with-api/assets/61904995/d6c38594-5e40-4ef8-808e-9e4d458b2f5b

Should be 1 item left

IvanFesenko · 2023-10-10T08:32:35Z

src/App.tsx

+
+          <TodoList />
+
+          {/* Hide the footer if there are no todos */}


Remove redundant comments

IvanFesenko · 2023-10-10T08:32:43Z

src/App.tsx

+        {/* Notification is shown in case of any error */}
+        {/* Add the 'hidden' class to hide the message smoothly */}


IvanFesenko · 2023-10-10T08:33:59Z

src/TodosContext/TodosContext.tsx

+  const preparedTodos = todos?.filter(todo => {
+    switch (sortQuery as SortType) {
+      case SortType.Active:
+        return !todo.completed;
+
+      case SortType.Completed:
+        return todo.completed;
+
+      default:
+        return true;
+    }
+  }) || todos;


No need to filter in case SortType is all

IvanFesenko · 2023-10-10T08:34:58Z

src/components/Footer.tsx

+            type="button"
+            className="todoapp__clear-completed"
+            data-cy="ClearCompletedButton"
+            onClick={() => clearCompleted()}


Will works same but less callbacks

Suggested change

onClick={() => clearCompleted()}

onClick={clearCompleted}

IvanFesenko · 2023-10-10T08:35:19Z

src/components/Header.tsx

+  useEffect(() => {
+    if (inputRef.current) {
+      inputRef.current.focus();
+    }
+  }, [isInputDisabled, !todos?.length, todosInProcess]);


IvanFesenko · 2023-10-10T08:36:11Z

src/components/Header.tsx

+          onClick={() => toggleAllTodos()}
+        />
+      )}
+
+      {/* Add a todo on form submit */}
+      <form
+        onSubmit={(event) => addTodo(event)}
+      >


Suggested change

onClick={() => toggleAllTodos()}

/>

)}

{/* Add a todo on form submit */}

<form

onSubmit={(event) => addTodo(event)}

>

onClick={toggleAllTodos}

/>

)}

{/* Add a todo on form submit */}

<form

onSubmit={addTodo}

>

IvanFesenko · 2023-10-10T08:36:42Z

src/components/TodoItem.tsx

+          <span
+            data-cy="TodoTitle"
+            className="todo__title"
+            onDoubleClick={() => onDoubleClick()}


Suggested change

onDoubleClick={() => onDoubleClick()}

onDoubleClick={onDoubleClick}

VitaliyBondarenko1982

Looks good.

need help

c669ec4

sTorba24 suggested changes Oct 7, 2023

View reviewed changes

commit have a few bugs

9df3a27

HYPER2307 requested a review from sTorba24 October 9, 2023 20:31

lerastarynets suggested changes Oct 10, 2023

View reviewed changes

solution

f2ff9e5

HYPER2307 requested a review from lerastarynets October 10, 2023 08:18

IvanFesenko suggested changes Oct 10, 2023

View reviewed changes

solution

874e255

HYPER2307 requested a review from IvanFesenko October 10, 2023 10:39

VitaliyBondarenko1982 approved these changes Oct 10, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

need help #858

need help #858

HYPER2307 commented Oct 6, 2023 •

edited

Loading

sTorba24 left a comment

HYPER2307 commented Oct 9, 2023

lerastarynets left a comment

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

lerastarynets Oct 10, 2023

IvanFesenko left a comment

IvanFesenko Oct 10, 2023

IvanFesenko Oct 10, 2023

IvanFesenko Oct 10, 2023

IvanFesenko Oct 10, 2023

IvanFesenko Oct 10, 2023

IvanFesenko Oct 10, 2023

IvanFesenko Oct 10, 2023

VitaliyBondarenko1982 left a comment

		{/* Notification is shown in case of any error */}
		{/* Add the 'hidden' class to hide the message smoothly */}

	onDoubleClick={() => onDoubleClick()}
	onDoubleClick={onDoubleClick}

need help #858

Are you sure you want to change the base?

need help #858

Conversation

HYPER2307 commented Oct 6, 2023 • edited Loading

sTorba24 left a comment

Choose a reason for hiding this comment

HYPER2307 commented Oct 9, 2023

lerastarynets 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

IvanFesenko 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

VitaliyBondarenko1982 left a comment

Choose a reason for hiding this comment

HYPER2307 commented Oct 6, 2023 •

edited

Loading