add task solution #1025

AndriiZakharenko · 2024-09-09T14:28:55Z

AndriiZakharenko · 2024-09-09T14:43:12Z

add tests

VolodymyrKirichenko

Good job, but let's improve ur code a bit 🥹

VolodymyrKirichenko · 2024-09-09T15:35:45Z

src/api.ts

@@ -22,4 +22,4 @@ function get<T>(url: string): Promise<T> {

 export const getTodos = () => get<Todo[]>('/todos');

-export const getUser = (userId: number) => get<User>(`/users/${userId}`);
+export const getUser = (userId: number) => get<User>(`/users/${userId}`);


I think it will be more readable)

Suggested change

export const getUser = (userId: number) => get<User>(`/users/${userId}`);

export const getUserById = (userId: number) => get<User>(`/users/${userId}`);

I think it will be more readable)

Not fixed

VolodymyrKirichenko · 2024-09-09T15:39:18Z

src/components/TodoFilter/TodoFilter.tsx

+ const setStatus = (status: string) => {
+ dispatch(filterActions.setStatus(status as Status));
+ };


set? use handle prefix for such functions.
also handle functions receive event by default

Suggested change

const setStatus = (status: string) => {

dispatch(filterActions.setStatus(status as Status));

};

const handleChangeStatus = (event: React.ChangeEvent<HTMLSelectElement>) => {

dispatch(filterActions.setStatus(event.target.value));

};

do it for all the cases!

VolodymyrKirichenko · 2024-09-09T15:39:47Z

src/components/TodoFilter/TodoFilter.tsx

+ <select
+ data-cy="statusSelect"
+ value={currentStatus}
+ onChange={event => setStatus(event.target.value)}


Suggested change

onChange={event => setStatus(event.target.value)}

onChange={handleChangeStatus}

VolodymyrKirichenko · 2024-09-09T15:43:01Z

src/features/currentTodo.ts

+// we use string literal as a type to avoid mistype in future
+type RemoveTodoAction = { type: 'currentTodo/REMOVE' };
+
+// payload is a typical name for an action data
+type SetTodoAction = {
+ type: 'currentTodo/SET';
+ payload: Todo;
+};
+
+// Action creator return type protect us from a mistype
+const removeTodo = (): RemoveTodoAction => ({ type: 'currentTodo/REMOVE' });

-export const currentTodoSlice = createSlice({
- name: 'currentTodo',
- initialState,
- reducers: {},
+const setTodo = (todo: Todo): SetTodoAction => ({
+ type: 'currentTodo/SET',
+ payload: todo,
 });
+
+// These actions will be used in the application


why do I only see comments in this file?
GPT?

VolodymyrKirichenko · 2024-09-09T15:46:35Z

src/components/TodoFilter/TodoFilter.tsx

+ };
+
+ const setQuery = (inputValue: string) => {
+ dispatch(filterActions.setQuery(inputValue));


add trim method to ignore spaces

BudnikOleksii

Good work 👍
Left some comments and suggestions

BudnikOleksii · 2024-09-13T06:36:31Z

src/App.tsx

+ setIsLoading(true);
+ getTodos()
+ .then(result => {
+ dispatch(todosSlice.actions.addTodos(filteredTodo(result)));


You don't need to fetch todos every time when filters changes

BudnikOleksii · 2024-09-13T06:37:13Z

src/App.tsx

- <div className="block">
- <TodoFilter />
- </div>
+ const filteredTodo = useCallback(


Suggested change

const filteredTodo = useCallback(

const filteredTodos = useMemo(

Just filter the todos that you have in the state, store in a variable and then show only them

BudnikOleksii · 2024-09-13T06:38:37Z

src/App.tsx

- <div className="block">
- <Loader />
- <TodoList />
+ <div className="block">{isLoading ? <Loader /> : <TodoList />}</div>


Suggested change

<div className="block">{isLoading ? <Loader /> : <TodoList />}</div>

<div className="block">{isLoading ? <Loader /> : <TodoList todos={filteredTodos} />}</div>

Like this

BudnikOleksii · 2024-09-13T06:38:59Z

src/api.ts

@@ -22,4 +22,4 @@ function get<T>(url: string): Promise<T> {

 export const getTodos = () => get<Todo[]>('/todos');

-export const getUser = (userId: number) => get<User>(`/users/${userId}`);
+export const getUser = (userId: number) => get<User>(`/users/${userId}`);


I think it will be more readable)

Not fixed

BudnikOleksii · 2024-09-13T06:39:39Z

src/components/TodoList/TodoList.tsx

+ return (
+ <tr
+ key={id}
+ data-cy="todo"


Create a todo component

BudnikOleksii · 2024-09-13T06:40:06Z

src/components/TodoList/TodoList.tsx

+ }
+ >
+ <span className="icon">
+ {currentTodo?.id === id ? (


You compare ids twice, create a variable

BudnikOleksii · 2024-09-13T06:40:43Z

src/components/TodoModal/TodoModal.tsx

+ setTimeout(() => {
+ getUser(userId)
+ .then(setUser)
+ .finally(() => setIsLoading(false));
+ }, 1000);


Why do you need a timeout?

BudnikOleksii · 2024-09-13T06:42:03Z

src/features/filter.ts

+ status: (state: State, action: PayloadAction<string>) => ({
+ ...state,
+ status: action.payload,
+ }),


Suggested change

status: (state: State, action: PayloadAction<string>) => ({

...state,

status: action.payload,

}),

status: (state: State, action: PayloadAction<string>) => {

state.status = action.payload,

},

In the redux toolkit you can mutate the state

DanilWeda

Good job.
Approved with a small comment.
Many thanks)

DanilWeda · 2024-09-28T10:57:49Z

src/App.tsx

- <div className="block">
- <TodoFilter />
- </div>
+ const filteredTodos = useMemo(() => {


You can move the function outside the file, and then use it as a helper func in this place.

AndriiZakharenko added 2 commits September 9, 2024 17:27

add task solution

9a0526b

add package.json

082957f

VolodymyrKirichenko suggested changes Sep 9, 2024

View reviewed changes

AndriiZakharenko added 2 commits September 13, 2024 00:31

absolutely redone

20e2c30

added json

1247b4e

AndriiZakharenko requested a review from VolodymyrKirichenko September 12, 2024 21:39

BudnikOleksii suggested changes Sep 13, 2024

View reviewed changes

AndriiZakharenko added 2 commits September 27, 2024 13:07

feat: updated

64ff04d

feat: updated reducers

e9e1357

AndriiZakharenko requested a review from BudnikOleksii September 27, 2024 13:37

DanilWeda approved these changes Sep 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add task solution #1025

add task solution #1025

AndriiZakharenko commented Sep 9, 2024

AndriiZakharenko commented Sep 9, 2024

VolodymyrKirichenko left a comment

VolodymyrKirichenko Sep 9, 2024

BudnikOleksii Sep 13, 2024

VolodymyrKirichenko Sep 9, 2024

VolodymyrKirichenko Sep 9, 2024

VolodymyrKirichenko Sep 9, 2024

VolodymyrKirichenko Sep 9, 2024

BudnikOleksii left a comment

BudnikOleksii Sep 13, 2024

BudnikOleksii Sep 13, 2024

BudnikOleksii Sep 13, 2024

BudnikOleksii Sep 13, 2024

BudnikOleksii Sep 13, 2024

BudnikOleksii Sep 13, 2024

BudnikOleksii Sep 13, 2024

BudnikOleksii Sep 13, 2024

DanilWeda left a comment

DanilWeda Sep 28, 2024

	export const getUser = (userId: number) => get<User>(`/users/${userId}`);
	export const getUserById = (userId: number) => get<User>(`/users/${userId}`);

	onChange={event => setStatus(event.target.value)}
	onChange={handleChangeStatus}

	const filteredTodo = useCallback(
	const filteredTodos = useMemo(

	<div className="block">{isLoading ? <Loader /> : <TodoList />}</div>
	<div className="block">{isLoading ? <Loader /> : <TodoList todos={filteredTodos} />}</div>

add task solution #1025

Are you sure you want to change the base?

add task solution #1025

Conversation

AndriiZakharenko commented Sep 9, 2024

AndriiZakharenko commented Sep 9, 2024

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

BudnikOleksii 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

DanilWeda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment