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

ToDoのstate管理方法の変更 #17

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
20 changes: 8 additions & 12 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,23 @@
import { Container } from '@chakra-ui/react';
import React, { useState } from 'react';

Choose a reason for hiding this comment

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

良いコードですね!


import TodoInput from './conponents/TodoInput';

Choose a reason for hiding this comment

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

素敵なコードですね!

import TodoList from './conponents/TodoList';

type TodoType = {
title: string;
};
import TodoInput from '@/conponents/TodoInput';
Copy link

Choose a reason for hiding this comment

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

素敵です!

import TodoList from '@/conponents/TodoList';
import { TodoType } from '@/types/TodoType';

const initialTodos: TodoType[] = [
{ title: '資料を作る' },
{ title: '歯医者に行く' },
{ title: '課題をやる' },
{ id: '0', title: '資料を作る', isCompleted: false },
{ id: '1', title: '歯医者に行く', isCompleted: false },
{ id: '2', title: '課題をやる', isCompleted: false },
];

const App = () => {
const [todos, setTodos] = useState<TodoType[]>(initialTodos);
const [status, setStatus] = useState<boolean[]>([false, false, false]);

return (
<Container>
<TodoInput todos={todos} setTodos={setTodos} status={status} setStatus={setStatus} />
<TodoList todos={todos} setTodos={setTodos} status={status} setStatus={setStatus} />
<TodoInput todos={todos} setTodos={setTodos} />
<TodoList todos={todos} setTodos={setTodos} />
</Container>
);
};
Expand Down
18 changes: 7 additions & 11 deletions src/conponents/Todo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,32 @@ import { CloseIcon } from '@chakra-ui/icons';
import { Flex, Spacer, Editable, EditablePreview, EditableInput, Checkbox } from '@chakra-ui/react';
import React, { Dispatch, SetStateAction } from 'react';

type TodoType = {
title: string;
};
import { TodoType } from '@/types/TodoType';

type Props = {
index: number;
title: string;
todos: TodoType[];
setTodos: Dispatch<SetStateAction<TodoType[]>>;
status: boolean[];
setStatus: Dispatch<SetStateAction<boolean[]>>;
};

const Todo = ({ index, title, todos, setTodos, status, setStatus }: Props) => {
const Todo = ({ index, title, todos, setTodos }: Props) => {
const removeTodo = () => {
const newTodos = todos.filter((_, i) => i !== index);
setTodos(newTodos);
};

const handleStatus = () => {
const newStatus = [...status];
newStatus[index] = !newStatus[index];
setStatus(newStatus);
const newStatus = [...todos];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

const newStatus = todos;
上記の書き方だとうまく動作しませんでしたが、理由はあんまりわかってないです。

Choose a reason for hiding this comment

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

自分も原因分からないです...。
const をいじってるのが怪しい?とも思ったけど、変更は配列の中身なので問題ないし。
あえて言うなら、基本 const で定義しているように、React のスタイル??は一度定義した変数を書き換えて使い回すのではなく、どんどん変更がある度に新しい変数に放り込んでいくので、そういう実装にしてみても良いかもしれません。
と言ってもここの場合だと、やるとしたらこんな感じ?であまり良い例&実装ではありませんが。

const handleStatus = () => {
  const newStatus = todos.map((todo: TodoType, i: number): TodoType => {
    return i === index
      ? { id: todo.id, title: todo.title, isCompleted: !todo.isCompleted }
      : todo;
  });
  setTodos(newStatus);
};

Copy link
Contributor

@watagit watagit Oct 10, 2021

Choose a reason for hiding this comment

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

ちょっと調べた感じだと、一般的にはslice()を使うみたいです(逆に今まで私どうやってたんだって感じですが)

[Qiita] ReactのsetStateで配列の一部を変更する
[Qiita]【React】Reactで便利な配列操作まとめ

Copy link

@mikiya1130 mikiya1130 Oct 10, 2021

Choose a reason for hiding this comment

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

じゃあやっぱここはコピーが必要なのかな。
[...status]status.slice() には何か違いあるのかな?
コピーの深さも同じ?気がするけど。
status.slice() の方が引数で範囲指定できる点で上位互換ってことかな。

newStatus[index].isCompleted = !newStatus[index].isCompleted;
setTodos(newStatus);
};

return (
<Flex align="center" w="90%" m="0 auto">
<Checkbox mr={2} onChange={() => handleStatus()} />
{<Checkbox mr={2} onChange={() => handleStatus()} />}
<Editable defaultValue={title}>
{status[index] ? <EditablePreview as="s" /> : <EditablePreview />}
{todos[index].isCompleted ? <EditablePreview as="s" /> : <EditablePreview />}
<EditableInput />
</Editable>
<Spacer />
Expand Down
15 changes: 6 additions & 9 deletions src/conponents/TodoInput.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
import { Input } from '@chakra-ui/react';
import React, { useState, ChangeEvent, Dispatch, SetStateAction, KeyboardEvent } from 'react';

type TodoType = {
title: string;
};
import { TodoType } from '@/types/TodoType';

type Props = {
todos: TodoType[];
setTodos: Dispatch<SetStateAction<TodoType[]>>;
status: boolean[];
setStatus: Dispatch<SetStateAction<boolean[]>>;
};

const TodoInput = ({ todos, setTodos, status, setStatus }: Props) => {
const TodoInput = ({ todos, setTodos }: Props) => {
const [title, setTitle] = useState<string>('');

return (
Expand All @@ -24,10 +20,11 @@ const TodoInput = ({ todos, setTodos, status, setStatus }: Props) => {
onChange={(e: ChangeEvent<HTMLInputElement>) => setTitle(e.target.value)}
onKeyPress={(e: KeyboardEvent<HTMLInputElement>) => {
if (e.key === 'Enter') {
const newTodos = [...todos, { title: title }];
const newStatus = [...status, false];
const newTodos = [
...todos,
{ id: String(todos.length), title: title, isCompleted: false },
Copy link

@mikiya1130 mikiya1130 Oct 10, 2021

Choose a reason for hiding this comment

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

must
todos.length から map の key で使う id を生成すると、id が重複することがあります 😢
(例:末尾以外の Todo を削除 → Todo を新規追加)
タスク追加ごとにひたすらインクリメントのみしてく値を用意するなど、他に対策が必要そうです。
map の key 専用で、なんでもいいなら作成時のタイムスタンプや、場合によっては乱数でも問題ないかもだけど。

(動画は useEffect 追加してます。)

2021-10-10.20-55-36.mp4

Copy link
Contributor

Choose a reason for hiding this comment

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

これ私が指示して書いてもらったんだけど、全然考慮できてなかった 🙄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

今回は「タスク追加ごとにひたすらインクリメントのみしてく値を用意する」というもので解消しました。
ご指摘ありがとうございました!

];
setTodos(newTodos);
setStatus(newStatus);
}
}}
/>
Expand Down
18 changes: 3 additions & 15 deletions src/conponents/TodoList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,18 @@ import React, { Dispatch, SetStateAction } from 'react';

import Todo from './Todo';

type TodoType = {
title: string;
};
import { TodoType } from '@/types/TodoType';

type Props = {
todos: TodoType[];
setTodos: Dispatch<SetStateAction<TodoType[]>>;
status: boolean[];
setStatus: Dispatch<SetStateAction<boolean[]>>;
};

const TodoList = ({ todos, setTodos, status, setStatus }: Props) => {
const TodoList = ({ todos, setTodos }: Props) => {
return (
<Stack divider={<StackDivider borderColor="gray.200" />} spacing={4}>
{todos.map((todo, i) => (
<Todo
key={todo.title}
index={i}
title={todo.title}
todos={todos}
setTodos={setTodos}
status={status}
setStatus={setStatus}
/>
<Todo key={todo.id} index={i} title={todo.title} todos={todos} setTodos={setTodos} />
))}
</Stack>
);
Expand Down
5 changes: 5 additions & 0 deletions src/types/TodoType.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export type TodoType = {
id: string;
title: string;
isCompleted: boolean;
};