Solution #2600
Solution #2600
Conversation
src/App.tsx
Outdated
| useEffect(() => { | ||
| getTodos() | ||
| .then(setTodos) | ||
| .finally(() => { | ||
| setVisibleTodos(todos); | ||
| setLoading(false); | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
It's better to move the logic inside the useEffect hook to a separate function, as it improves code readability, reusability, and makes testing easier. It also helps keep useEffect focused on side effects.
src/App.tsx
Outdated
| useEffect(() => { | ||
| setVisibleTodos(() => { | ||
| return filterTodos(todos, todoCategory, query); | ||
| }); | ||
| }, [todos, query, todoCategory]); |
There was a problem hiding this comment.
Create a handle function for this logic instead of using useEffect
| useEffect(() => { | ||
| getUser(todo.userId) | ||
| .then(setUser) | ||
| .finally(() => { | ||
| setLoading(false); | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
Create a handle function for this logic.
React Hook useEffect has a missing dependency
| {todo.completed ? ( | ||
| <strong className="has-text-success">Done</strong> | ||
| ) : ( | ||
| <strong className="has-text-danger">Planned</strong> | ||
| )} |
There was a problem hiding this comment.
You can render a single tag and conditionally add a className based on a certain condition.
src/App.tsx
Outdated
|
|
||
| export const App: React.FC = () => { | ||
| const [todos, setTodos] = useState<Todo[]>([]); | ||
| const [visibleTodos, setVisibleTodos] = useState<Todo[]>([]); | ||
| const [electTodoId, setElectTodoId] = useState<number>(0); |
There was a problem hiding this comment.
What is elect?) Probably it should be selectedTodoId
src/App.tsx
Outdated
| const [todos, setTodos] = useState<Todo[]>([]); | ||
| const [visibleTodos, setVisibleTodos] = useState<Todo[]>([]); | ||
| const [electTodoId, setElectTodoId] = useState<number>(0); | ||
| const [loading, setLoading] = useState<boolean>(true); |
There was a problem hiding this comment.
- Boolean variables should start from
isorhas - You should show loader only when you actually loading something
src/components/TodoLine/TodoLine.tsx
Outdated
| import classNames from 'classnames'; | ||
|
|
||
| type Props = { | ||
| TodoElement: Todo; |
There was a problem hiding this comment.
| TodoElement: Todo; | |
| todo: Todo; |
Names should be in camelCase, in PascalCase only component names and classes
|
|
||
| export const TodoModal: React.FC<Props> = ({ todo, onElectTodoId }) => { | ||
| const [user, setUser] = useState<User>(); | ||
| const [loading, setLoading] = useState(true); |
There was a problem hiding this comment.
The same regarding loading state
| {todo.completed ? ( | ||
| <strong className="has-text-success">Done</strong> | ||
| ) : ( | ||
| <strong | ||
| className={ | ||
| todo.completed ? 'has-text-success' : 'has-text-danger' | ||
| } | ||
| > | ||
| {todo.completed ? 'Done' : 'Planned'} | ||
| </strong> | ||
| )} |
There was a problem hiding this comment.
| {todo.completed ? ( | |
| <strong className="has-text-success">Done</strong> | |
| ) : ( | |
| <strong | |
| className={ | |
| todo.completed ? 'has-text-success' : 'has-text-danger' | |
| } | |
| > | |
| {todo.completed ? 'Done' : 'Planned'} | |
| </strong> | |
| )} | |
| <strong | |
| className={ | |
| todo.completed ? 'has-text-success' : 'has-text-danger' | |
| } | |
| > | |
| {todo.completed ? 'Done' : 'Planned'} | |
| </strong> |
|
|
||
| {' by '} | ||
|
|
||
| <a href="mailto:[email protected]">Leanne Graham</a> | ||
| <a href={`mailto:${user?.email}`}> | ||
| {user !== undefined ? user.name : ''} |
There was a problem hiding this comment.
| {user !== undefined ? user.name : ''} | |
| {user?.name} |
It would be the same result
src/utils/filterTodosByQuery.ts
Outdated
|
|
||
| function filterTodos( | ||
| todos: Todo[], | ||
| todoCategory: string, |
There was a problem hiding this comment.
Category is not a string, you have a corresponding type TodoCompletedCategory
src/utils/filterTodosByQuery.ts
Outdated
| if (todoCategory === 'active') { | ||
| return todos.filter( | ||
| todo => !todo.completed && todo.title.toLowerCase().includes(query), | ||
| ); | ||
| } | ||
|
|
||
| if (todoCategory === 'completed') { |
src/utils/filterTodosByQuery.ts
Outdated
| export function filterTodosByQuery( | ||
| todos: Todo[], | ||
| setTodos: React.Dispatch<React.SetStateAction<Todo[]>>, | ||
| todoCategory: TodoCompletedCategory, | ||
| query: string, | ||
| ) { | ||
| setTodos(() => { | ||
| return filterTodos(todos, todoCategory, query); | ||
| }); |
There was a problem hiding this comment.
You can do it directly in the component
src/utils/setTodosFromApi.ts
Outdated
| import { getTodos } from '../api'; | ||
| import { Todo } from '../types/Todo'; | ||
|
|
||
| export function setTodosFromApi( |
DEMO LINK