Develop #1499

Mariana-VV · 2024-10-24T08:48:27Z

DEMO LINK

anastasiiavorobiova

Great work!

Looks a bit broken on the edit fail. Consider fixing it

anastasiiavorobiova

The app always shows an error on deleting the todo (when a user deletes a title during editing) Consider fixing it

etojeDenys

on toggle all, you should show loaders only on todos that you need to update
show loaders on clear completed all

Screen.Recording.2024-10-24.at.18.15.18.mov

I can change the edited input (in the todo) to just have some spaces

you need to remove the todo in this case

error messages have too much spaces

volodymyr-soltys97

Hi, you need to fix all comments from the previous review and make a deploy again after your changes

anastasiiavorobiova

Great work!
If a lot of todos are checked and a user clicks on the clear all button, every todo gets the loading state

etojeDenys · 2024-10-25T16:33:40Z

src/App.tsx

+        {loadError && 'Unable to load todos'}
+        {titleError && 'Title should not be empty'}
+        {addError && 'Unable to add a todo'}
+        {deleteError && 'Unable to delete a todo'}
+        {updateError && 'Unable to update a todo'}


can we just create only one errorMessage state and save our messages there?

Suggested change

{loadError && 'Unable to load todos'}

{titleError && 'Title should not be empty'}

{addError && 'Unable to add a todo'}

{deleteError && 'Unable to delete a todo'}

{updateError && 'Unable to update a todo'}

{errorMessage}

I think it should work

volodymyr-soltys97

Almost done!

volodymyr-soltys97 · 2024-10-26T15:36:29Z

src/App.tsx

+  // const [loadError, setLoadError] = useState(false);
+  // const [addError, setAddError] = useState(false);
+  // const [deleteError, setDeleteError] = useState(false);
+  // const [updateError, setUpdateError] = useState(false);


Remove all comments

Suggested change

// const [loadError, setLoadError] = useState(false);

// const [addError, setAddError] = useState(false);

// const [deleteError, setDeleteError] = useState(false);

// const [updateError, setUpdateError] = useState(false);

volodymyr-soltys97 · 2024-10-26T15:38:09Z

src/App.tsx

+  async function updateTodo(
+    updatedTodo: Todo,
+    // successUpdateState?: VoidFunction,
+  ): Promise<void> {


It's important to use one way of creating functions, fix it everywhere

Suggested change

async function updateTodo(

updatedTodo: Todo,

// successUpdateState?: VoidFunction,

): Promise<void> {

const updateTodo = async (

updatedTodo: Todo,

// successUpdateState?: VoidFunction,

): Promise<void> => {

volodymyr-soltys97 · 2024-10-26T15:38:20Z

src/App.tsx

+
+  async function updateTodo(
+    updatedTodo: Todo,
+    // successUpdateState?: VoidFunction,


Remove comments

Suggested change

// successUpdateState?: VoidFunction,

volodymyr-soltys97 · 2024-10-26T15:38:36Z

src/api/todos.ts

+import { client } from '../utils/fetchClient';
+
+export const USER_ID = 839;
+//https://mate.academy/students-api/todos?userId=839


Suggested change

//https://mate.academy/students-api/todos?userId=839

volodymyr-soltys97 · 2024-10-26T15:39:07Z

src/components/TodoItem/TodoItem.tsx

+  // const successUpdateState = () => {
+  //   setIsEdited(false);
+  // };


Suggested change

// const successUpdateState = () => {

// setIsEdited(false);

// };

volodymyr-soltys97 · 2024-10-26T15:39:15Z

src/components/TodoItem/TodoItem.tsx

+
+      const newTodo = { ...todo, title: tempTitle.trim() };
+
+      // setIsEdited(true);


Suggested change

// setIsEdited(true);

volodymyr-soltys97 · 2024-10-26T15:39:28Z

src/components/TodoItem/TodoItem.tsx

+        // className={classNames('modal overlay', {
+        //   'is-active': !todo.id || tempArray.includes(todo),
+        // })}


Suggested change

// className={classNames('modal overlay', {

// 'is-active': !todo.id || tempArray.includes(todo),

// })}

volodymyr-soltys97 · 2024-10-26T15:40:27Z

src/App.tsx

+      .getTodos()
+      .then(setTodos)
+      .catch(() => setErrorMessage('Unable to load todos'));
+    wait(3000).then(() => setErrorMessage(''));


Do you need wait here and then() again?
You can use finally() method

TarasHoliuk

GJ! A few more small comments 🙂

TarasHoliuk · 2024-10-27T09:38:54Z

src/App.tsx

+      setErrorMessage('Title should not be empty');
+      wait(3000).finally(() => setErrorMessage('Title should not be empty'));


Suggested change

setErrorMessage('Title should not be empty');

wait(3000).finally(() => setErrorMessage('Title should not be empty'));

setErrorMessage('Title should not be empty');

you already showed the message, no need to do it again after a delay

TarasHoliuk · 2024-10-27T09:43:43Z

src/components/TodoForm/TodoForm.tsx

+    <>
+      {/* this button should have `active` class only if all todos are completed */}
+      {!!todos.length && (
+        //   что значит !!


Is it a question?) just remove it

TarasHoliuk · 2024-10-27T09:50:23Z

src/App.tsx

+      .getTodos()
+      .then(setTodos)
+      .catch(() => setErrorMessage('Unable to load todos'));
+    wait(3000).finally(() => setErrorMessage(''));


Hint: you can add useEffect with errorMessage in dependencies array
if it's not empty add 3000ms timeout and set setErrorMessage('')

It will allow to remove this in every method

volodymyr-soltys97

Great!

Mariana-VV added 10 commits July 11, 2024 16:19

check

e0f64c1

message

1a3cdc8

message

5b3b423

message

6a1456a

message

69aa58f

onlycheck

d950f61

onlycheck

1f5741c

onlycheck

ddc63fc

solution

dd1fa55

solution

ee93fbe

anastasiiavorobiova suggested changes Oct 24, 2024

View reviewed changes

Mariana-VV added 2 commits October 24, 2024 13:23

improvement

9d0dcb3

onlycheck

df5c5a8

Mariana-VV requested a review from anastasiiavorobiova October 24, 2024 12:10

anastasiiavorobiova suggested changes Oct 24, 2024

View reviewed changes

try

3385ffa

Mariana-VV closed this Oct 24, 2024

Mariana-VV reopened this Oct 24, 2024

Mariana-VV requested a review from anastasiiavorobiova October 24, 2024 13:41

etojeDenys suggested changes Oct 24, 2024

View reviewed changes

improvement

a787c03

Mariana-VV requested a review from etojeDenys October 25, 2024 09:34

volodymyr-soltys97 suggested changes Oct 25, 2024

View reviewed changes

improvement

e85df0d

Mariana-VV requested a review from volodymyr-soltys97 October 25, 2024 11:09

anastasiiavorobiova suggested changes Oct 25, 2024

View reviewed changes

message

71ab383

Mariana-VV requested a review from anastasiiavorobiova October 25, 2024 16:24

etojeDenys suggested changes Oct 25, 2024

View reviewed changes

message

5061ace

Mariana-VV requested a review from etojeDenys October 26, 2024 08:32

volodymyr-soltys97 suggested changes Oct 26, 2024

View reviewed changes

message

fa12eb5

Mariana-VV requested a review from volodymyr-soltys97 October 26, 2024 16:04

TarasHoliuk suggested changes Oct 27, 2024

View reviewed changes

message

a61c620

Mariana-VV requested a review from TarasHoliuk October 27, 2024 15:11

volodymyr-soltys97 approved these changes Oct 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop #1499

Develop #1499

Mariana-VV commented Oct 24, 2024 •

edited

Loading

anastasiiavorobiova left a comment

anastasiiavorobiova left a comment

etojeDenys left a comment

volodymyr-soltys97 left a comment

anastasiiavorobiova left a comment

etojeDenys Oct 25, 2024

volodymyr-soltys97 left a comment

volodymyr-soltys97 Oct 26, 2024

volodymyr-soltys97 Oct 26, 2024

volodymyr-soltys97 Oct 26, 2024

volodymyr-soltys97 Oct 26, 2024

volodymyr-soltys97 Oct 26, 2024

volodymyr-soltys97 Oct 26, 2024

volodymyr-soltys97 Oct 26, 2024

volodymyr-soltys97 Oct 26, 2024

TarasHoliuk left a comment

TarasHoliuk Oct 27, 2024

TarasHoliuk Oct 27, 2024

TarasHoliuk Oct 27, 2024

volodymyr-soltys97 left a comment

	// const successUpdateState = () => {
	// setIsEdited(false);
	// };


		const newTodo = { ...todo, title: tempTitle.trim() };

		// setIsEdited(true);

	// className={classNames('modal overlay', {
	// 'is-active': !todo.id \|\| tempArray.includes(todo),
	// })}

		setErrorMessage('Title should not be empty');
		wait(3000).finally(() => setErrorMessage('Title should not be empty'));

Develop #1499

Are you sure you want to change the base?

Develop #1499

Conversation

Mariana-VV commented Oct 24, 2024 • edited Loading

anastasiiavorobiova left a comment

Choose a reason for hiding this comment

anastasiiavorobiova left a comment

Choose a reason for hiding this comment

etojeDenys left a comment

Choose a reason for hiding this comment

volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

anastasiiavorobiova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volodymyr-soltys97 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

TarasHoliuk 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

volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

Mariana-VV commented Oct 24, 2024 •

edited

Loading