Develop #1535

OkMoroz · 2024-12-17T10:22:52Z

DEMO LINK

minimal005 · 2024-12-17T10:31:10Z

src/App.tsx

-    return <UserWarning />;
-  }
+  const [todos, setTodos] = useState<Todo[]>([]);
+  const [errorMessage, setErrorMessage] = useState<string>('');


в тебе є enum по помилкам, типізуй useState типом Error
щоб не робити тут додаткової перевірки на пустий рядок, то можеж в enum прописати, наприклад Nothing: '', aбо Empty: ''

Бо в даному випадку в цей state можна покласти довільний рядок

Дякую, візьму до уваги

minimal005 · 2024-12-17T10:37:46Z

src/App.tsx

+  const [errorMessage, setErrorMessage] = useState<string>('');
+  const [tempTodo, setTempTodo] = useState<Todo | null>(null);
+  const [filter, setFilter] = useState<Filters>(Filters.All);
+  const [loadingId, setLoadingId] = useState<Loading>({});


краще зберігати саму id, без об'єкта і судячи з назви там має бути id

minimal005 · 2024-12-17T10:39:10Z

src/App.tsx

+          onDelete={handleDelete}
+        />
+
+        {todos.length > 0 && (


краще вказати перевірку !!todos.length

чому саме той варіант кращий?

@OkMoroz !! - converts the value to boolean type and more readable i think

VolodymyrKirichenko

Good job 🔥
But let's make some changes

VolodymyrKirichenko · 2024-12-19T15:29:12Z

src/App.tsx

-    return <UserWarning />;
-  }
+  const [todos, setTodos] = useState<Todo[]>([]);
+  const [errorMessage, setErrorMessage] = useState<ErrorMessage | string>('');


it will be safer that way

Suggested change

const [errorMessage, setErrorMessage] = useState<ErrorMessage | string>('');

const [errorMessage, setErrorMessage] = useState<ErrorMessage>(ErrorMessage.Default);

VolodymyrKirichenko · 2024-12-19T15:31:33Z

src/App.tsx

+    return () => clearTimeout(timeoutId);
+  }, []);
+
+  const handleAdd = (newTodo: Todo): Promise<Todo | void> => {


why u need Todo in this place?
you don't use this feature to get an item(onSubmit - ur logic)

Suggested change

const handleAdd = (newTodo: Todo): Promise<Todo | void> => {

const handleAdd = (newTodo: Todo): Promise<void> => {

VolodymyrKirichenko · 2024-12-19T15:33:06Z

src/App.tsx

+          onDelete={handleDelete}
+        />
+
+        {todos.length > 0 && (


@OkMoroz !! - converts the value to boolean type and more readable i think

VolodymyrKirichenko · 2024-12-19T15:33:51Z

src/api/todos.ts

+export const USER_ID = 2132;
+export const TODOS_ENDPOINT = '/todos';


move it to a separate file - constants

VolodymyrKirichenko · 2024-12-19T15:34:46Z

src/components/Footer.tsx

+  const filtersValue = Object.values(Filters);
+  const activeTodosCount = todos.filter(todo => !todo.completed).length;
+  const isCompleted = todos.some(todo => todo.completed);
+  let isDeleteCompleted = false;


loading state?

VolodymyrKirichenko · 2024-12-19T15:34:57Z

src/components/Footer.tsx

+  const filtersValue = Object.values(Filters);
+  const activeTodosCount = todos.filter(todo => !todo.completed).length;
+  const isCompleted = todos.some(todo => todo.completed);


VolodymyrKirichenko · 2024-12-19T15:36:37Z

src/components/Footer.tsx

+        {filtersValue.map(filter => (
+          <a
+            key={filter}
+            href={`#/${filter !== Filters.All ? filter.toLowerCase() : ''}`}


will it be better?

Suggested change

href={`#/${filter !== Filters.All ? filter.toLowerCase() : ''}`}

href={`#/${filter !== Filters.All && filter.toLowerCase()}`}

VolodymyrKirichenko · 2024-12-19T15:37:29Z

src/components/TodoItem.tsx

+import { Todo } from '../types/Todo';
+import { Loading } from '../types/Loading';
+
+enum TodoKey {


move all the enums to the separate directory

VolodymyrKirichenko · 2024-12-19T15:38:39Z

src/components/TodoItem.tsx

+        data-cy="TodoLoader"
+        className={cn('modal overlay', {
+          'is-active':
+            Object.hasOwn(loadingId, todo.id) || isLoading || todo.id === 0,


move it to a variable, I don't fully understand what it is

VolodymyrKirichenko · 2024-12-19T15:40:20Z

src/components/TodoList.tsx

+        />
+      ))}
+
+      {tempTodo !== null && (


the same

Suggested change

{tempTodo !== null && (

{!tempTodo && (

StasSokolov1

Looks much more better, I'm approving it🔥

OkMoroz added 2 commits December 16, 2024 19:19

added start codes

81db3ef

added solutions

8bca719

minimal005 approved these changes Dec 17, 2024

View reviewed changes

OkMoroz added 2 commits December 19, 2024 13:08

fixed todos.ts

15538b5

fixed useState error

455fede

VolodymyrKirichenko suggested changes Dec 19, 2024

View reviewed changes

fixed after comments

018ba67

OkMoroz requested a review from VolodymyrKirichenko December 19, 2024 17:20

StasSokolov1 approved these changes Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop #1535

Develop #1535

OkMoroz commented Dec 17, 2024

minimal005 Dec 17, 2024

OkMoroz Dec 17, 2024

minimal005 Dec 17, 2024

minimal005 Dec 17, 2024

OkMoroz Dec 17, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko left a comment

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

VolodymyrKirichenko Dec 19, 2024

StasSokolov1 left a comment

	const [errorMessage, setErrorMessage] = useState<ErrorMessage \| string>('');
	const [errorMessage, setErrorMessage] = useState<ErrorMessage>(ErrorMessage.Default);

	const handleAdd = (newTodo: Todo): Promise<Todo \| void> => {
	const handleAdd = (newTodo: Todo): Promise<void> => {

		export const USER_ID = 2132;
		export const TODOS_ENDPOINT = '/todos';

	href={`#/${filter !== Filters.All ? filter.toLowerCase() : ''}`}
	href={`#/${filter !== Filters.All && filter.toLowerCase()}`}

Develop #1535

Are you sure you want to change the base?

Develop #1535

Conversation

OkMoroz commented Dec 17, 2024

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

VolodymyrKirichenko 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

StasSokolov1 left a comment

Choose a reason for hiding this comment