Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Develop #815

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 224 additions & 16 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,232 @@
/* eslint-disable max-len */
/* eslint-disable jsx-a11y/control-has-associated-label */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to rewrite the code so that it is not necessary to disable the linter

import React from 'react';
import { UserWarning } from './UserWarning';
import React, {
useEffect,
useMemo,
useRef,
useState,
} from 'react';

const USER_ID = 0;
import { TodoAppHeader } from './components/TodoAppHeader';
import { TodoAppFooter } from './components/TodoAppFooter';
import { ErrorNotification } from './components/ErrorNotification';

import { Todo } from './types/Todo';
import * as todoService from './api/todos';
import { FilterLink, preparedTodos } from './utils/TodoFilter';
import { ErrorMessage } from './utils/errorMessages';
import { TodoAppRow } from './components/TodoAppRow/TodoAppRow';
import { TempTodo } from './components/TodoAppRow';

export const App: React.FC = () => {
if (!USER_ID) {
return <UserWarning />;
}
const [todos, setTodos] = useState<Todo[]>([]);
const [selectedFilter, setSelectedFilter] = useState(FilterLink.All);
const [errorMessage, setErrorMessage] = useState(ErrorMessage.Default);
const [isRequesting, setIsRequesting] = useState(false);
const [tempTodo, setTempTodo] = useState<Todo | null>(null);
const [processingTodoIds, setProcessingTodoIds] = useState<number[]>([]);

useEffect(() => {
todoService.getTodos(todoService.USER_ID)
.then(setTodos)
.catch(() => {
setErrorMessage(ErrorMessage.Load);
});
}, []);

const timerId = useRef<number>(0);

useEffect(() => {
if (timerId.current) {
window.clearTimeout(timerId.current);
}

timerId.current = window.setTimeout(() => {
setErrorMessage(ErrorMessage.Default);
}, 3000);
}, [errorMessage]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this logic to ErrorNotification component


const handleAddTodo = (todoTitle: string): Promise<void> => {
setTempTodo({
id: 0,
userId: todoService.USER_ID,
title: todoTitle,
completed: false,
});

setIsRequesting(true);

return todoService
.addTodo(todoTitle)
.then((newTodo) => {
setTodos((prevTodos) => [...prevTodos, newTodo]);
})
.catch((error) => {
setErrorMessage(ErrorMessage.Add);
throw error;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to return something from the handler and throw an error as you handle it on this step

})
.finally(() => {
setIsRequesting(false);
setTempTodo(null);
});
};

const handleDeleteTodo = (todoId: number) => {
setProcessingTodoIds((prevTodoIds) => [...prevTodoIds, todoId]);

todoService
.deleteTodo(todoId)
.then((() => {
setTodos((prevTodos) => prevTodos.filter(todo => todo.id !== todoId));
}))
.catch(() => {
setErrorMessage(ErrorMessage.Delete);
})
.finally(() => {
setProcessingTodoIds(
(prevTodoIds) => prevTodoIds.filter(id => id !== todoId),
);
});
};

const handleUpdateTodo = async (todo: Todo, newTodoTitle: string) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't use await there no sense to do your function async

setProcessingTodoIds((prevTodoIds) => [...prevTodoIds, todo.id]);

return todoService
.updateTodo({
id: todo.id,
title: newTodoTitle,
userId: todo.userId,
completed: todo.completed,
})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.updateTodo({
id: todo.id,
title: newTodoTitle,
userId: todo.userId,
completed: todo.completed,
})
.updateTodo({
...todo,
title: newTodoTitle,
})

.then(updatedTodo => {
setTodos(prevState => prevState.map(currentTodo => (
currentTodo.id !== updatedTodo.id
? currentTodo
: updatedTodo
)));
})
.catch(() => {
setErrorMessage(ErrorMessage.Update);
throw new Error();
})
.finally(() => {
setProcessingTodoIds(
(prevTodoIds) => prevTodoIds.filter(id => id !== todo.id),
);
});
};

const handleCompletedChange = (todo:Todo) => {
setProcessingTodoIds((prevTodoIds) => [...prevTodoIds, todo.id]);

todoService
.updateTodo({
id: todo.id,
title: todo.title,
userId: todo.userId,
completed: !todo.completed,
})
.then(updatedTodo => {
setTodos(prevState => prevState.map(currentTodo => (
currentTodo.id !== updatedTodo.id
? currentTodo
: updatedTodo
)));
})
.catch((error) => {
setErrorMessage(ErrorMessage.Update);
throw error;
})
.finally(() => {
setProcessingTodoIds(
(prevTodoIds) => prevTodoIds.filter(id => id !== todo.id),
);
});
};

const handleDeleteAllCompletedTodos = () => {
todos
.filter((todo) => todo.completed)
.forEach((todo) => handleDeleteTodo(todo.id));
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you need to send a batch of requests it's better to create an array of requests and handle them with Promise.all or Promise.allsettled


const visibleTodos = preparedTodos(todos, selectedFilter);

const todosCounter = useMemo(() => {
return todos.filter(todo => !todo.completed).length;
}, [todos]);

const isAllCompleted = useMemo(() => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const isAllCompleted = useMemo(() => {
const isAllTodosCompleted = useMemo(() => {

return todos.every(todo => todo.completed);
}, [todos]);

const hasCompletedTodo = todos.some(({ completed }) => completed);

const handleToggleButton = () => {
if (!isAllCompleted) {
todos
.filter((todo) => !todo.completed)
.forEach((todo) => handleCompletedChange(todo));
}

if (isAllCompleted) {
todos
.filter((todo) => todo.completed)
.forEach((todo) => handleCompletedChange(todo));
}

This comment was marked as outdated.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!isAllCompleted) {
todos
.filter((todo) => !todo.completed)
.forEach((todo) => handleCompletedChange(todo));
}
if (isAllCompleted) {
todos
.filter((todo) => todo.completed)
.forEach((todo) => handleCompletedChange(todo));
}
todos
.filter((todo) => todo.completed === isAllCompleted)
.forEach((todo) => handleCompletedChange(todo));

You can simplify in this case

};

return (
<section className="section container">
<p className="title is-4">
Copy all you need from the prev task:
<br />
<a href="https://github.com/mate-academy/react_todo-app-add-and-delete#react-todo-app-add-and-delete">React Todo App - Add and Delete</a>
</p>

<p className="subtitle">Styles are already copied</p>
</section>
<div className="todoapp">
<h1 className="todoapp__title">todos</h1>

<div className="todoapp__content">
<TodoAppHeader
onToggleClick={handleToggleButton}
todos={visibleTodos}
onTodoAdd={handleAddTodo}
setErrorMessage={setErrorMessage}
isRequesting={isRequesting}
isAllCompleted={isAllCompleted}
/>

{!!todos.length && (
<>
<section className="todoapp__main" data-cy="TodoList">
{visibleTodos.map((todo: Todo) => (
<TodoAppRow
todo={todo}
key={todo.id}
isLoading={processingTodoIds.includes(todo.id)}
onTodoDelete={handleDeleteTodo}
onChangeBox={() => (
handleCompletedChange(todo))}
onTodoUpdate={(todoTitle) => (
handleUpdateTodo(todo, todoTitle)
)}
/>
))}

{tempTodo && (
<TempTodo todo={tempTodo} />
)}
</section>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create TodoList component


<TodoAppFooter
selectedFilter={selectedFilter}
setSelectedFilter={setSelectedFilter}
todosCounter={todosCounter}
hasCompletedTodo={hasCompletedTodo}
deleteAllComleted={handleDeleteAllCompletedTodos}
/>
</>
)}
</div>

<ErrorNotification
setErrorMessage={setErrorMessage}
errorMessage={errorMessage}
/>
</div>
);
};
22 changes: 0 additions & 22 deletions src/UserWarning.tsx

This file was deleted.

33 changes: 33 additions & 0 deletions src/api/todos.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { Todo } from '../types/Todo';
import { client } from '../utils/fetchClient';

export const USER_ID = 11482;

export const getTodos = (userId: number) => {
return client.get<Todo[]>(`/todos?userId=${userId}`);
};

export const addTodo = (title: string) => {
return client.post<Todo>('/todos', {
title,
userId: USER_ID,
completed: false,
});
};

export const deleteTodo = (todoId: number) => {
return client.delete(`/todos/${todoId}`);
};

export const updateTodo = ({
id,
title,
userId,
completed,
}: Todo): Promise<Todo> => {
return client.patch(`/todos/${id}`, {
title,
userId,
completed,
});
};
36 changes: 36 additions & 0 deletions src/components/ErrorNotification/ErrorNotification.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/* eslint-disable jsx-a11y/control-has-associated-label */
import React from 'react';
import cn from 'classnames';
import { ErrorMessage } from '../../utils/errorMessages';

type Props = {
setErrorMessage: (value: ErrorMessage) => void;
errorMessage: ErrorMessage;
};

export const ErrorNotification: React.FC<Props> = ({
setErrorMessage,
errorMessage,
}) => {
return (
<div
data-cy="ErrorNotification"
className={cn(
'notification',
'is-danger',
'is-light',
'has-text-weight-normal',
{ hidden: errorMessage === ErrorMessage.Default },
)}
>
<button
data-cy="HideErrorButton"
type="button"
className="delete"
onClick={() => setErrorMessage(ErrorMessage.Default)}
/>

{errorMessage}
</div>
);
};
1 change: 1 addition & 0 deletions src/components/ErrorNotification/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './ErrorNotification';
Loading