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

still a lot to do #849

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

MateuszSlezak
Copy link

Copy link

@choeqq choeqq left a comment

Choose a reason for hiding this comment

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

404 when trying to access your DEMO

src/App.tsx Outdated
import { TodoList } from './components/TodoList/TodoList';
import { Todo } from './types/Todo';

// const USER_ID = 11569;
Copy link

Choose a reason for hiding this comment

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

cleanup

src/App.tsx Outdated
Comment on lines 18 to 28
const [todos, setTodos] = useState<Todo[] >([]);
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [isLoading, setIsLoading] = useState<number[] | []>([]);

const [statusFilter, setStatusFilter] = useState('all');

const [tempTodo, setTempTodo] = useState<Todo | null>(null);

const [error, setError] = useState('');

const [addTodoForm, setAddTodoForm] = useState('');
Copy link

Choose a reason for hiding this comment

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

Suggested change
const [todos, setTodos] = useState<Todo[] >([]);
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [isLoading, setIsLoading] = useState<number[] | []>([]);
const [statusFilter, setStatusFilter] = useState('all');
const [tempTodo, setTempTodo] = useState<Todo | null>(null);
const [error, setError] = useState('');
const [addTodoForm, setAddTodoForm] = useState('');
const [todos, setTodos] = useState<Todo[] >([]);
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [isLoading, setIsLoading] = useState<number[] | []>([]);
const [statusFilter, setStatusFilter] = useState('all');
const [tempTodo, setTempTodo] = useState<Todo | null>(null);
const [error, setError] = useState('');
const [addTodoForm, setAddTodoForm] = useState('');

src/App.tsx Outdated
Comment on lines 83 to 85
// const focusInput = () => {
// inputRef.current.focus();
// };
Copy link

Choose a reason for hiding this comment

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

cleanup

src/App.tsx Outdated
setError('Unable to delete a todo');
} finally {
setTimeout(() => {
setIsLoading(todos.filter(todo => todo.id !== todoId) as number[] | []);
Copy link

Choose a reason for hiding this comment

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

try to type it better, you shouldn't have to use as

src/App.tsx Outdated
}
};

const updateStatusHandler = (todo: Todo, data: {}) => {
Copy link

Choose a reason for hiding this comment

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

you're forcing data to be empty object in your typing, which is not ideal

@@ -0,0 +1,49 @@
import React, { useContext, useState } from 'react';
// import { useTodos } from './useTodos';
Copy link

Choose a reason for hiding this comment

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

cleanup

};

export const TodosProvider = ({ children, userId }: TodosProviderProps) => {
// const { isLoading, ...rest } = useTodos(userId);
Copy link

Choose a reason for hiding this comment

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

cleanup


},
) => {
// const [todosUpdated, setTodosUpdated] = useState(0);
Copy link

Choose a reason for hiding this comment

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

cleanup

import classNames from 'classnames';
import { useRef, useState } from 'react';
import { patchTodo } from '../../api/todos';
// import { deleteTodoHandler } from '../../App'
Copy link

Choose a reason for hiding this comment

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

cleanup

}) => {
const [isEdited, setIsEdited] = useState(false);
const [editTitle, setEditTitle] = useState('');
const [isCompletedLocale] = useState(todo.completed);
Copy link

Choose a reason for hiding this comment

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

cleanup

@MateuszSlezak MateuszSlezak requested a review from choeqq October 4, 2023 02:26
@MateuszSlezak
Copy link
Author

404 when trying to access your DEMO

I'm still working on it, I haven't passed 3 tests and I haven't managed to do everything you marked yet, but I've sorted it out a bit and it's all over now. and the link already works ;D

Copy link

@choeqq choeqq left a comment

Choose a reason for hiding this comment

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

It looks good enough to approve it, despite that, please apply suggested changes 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants