Feat: 투두리스트 기능 구현 #2
Conversation
src/components/create-todo-box.tsx
Outdated
| const handleSubmit = (e: SyntheticEvent) => { | ||
| e.preventDefault(); | ||
|
|
||
| const form = e.target as HTMLFormElement; | ||
| const titleInput = form.elements.namedItem("title") as HTMLInputElement; | ||
| const descInput = form.elements.namedItem( | ||
| "description" | ||
| ) as HTMLInputElement; | ||
|
|
||
| if (e.target) { | ||
| const newTodo: Omit<Todo, "id" | "createdAt" | "completed"> = { | ||
| title: titleInput.value ?? "", | ||
| description: descInput.value ?? "", | ||
| }; | ||
|
|
||
| onCreate(newTodo); | ||
|
|
||
| titleInput.value = ""; | ||
| descInput.value = ""; | ||
| } else { | ||
| alert("등록에 실패했습니다."); | ||
| } |
There was a problem hiding this comment.
form구조가 복잡하지는 않아 보이는데, unontrolled로 작성하신 이유가 있으실까요 ~?
There was a problem hiding this comment.
controlled 방식을 의도하고 작성하진 않았습니다!
사실 dom에서 직접 값을 받아서 쓰는 방식이라서 uncontrolled라고 생각했어요.
어떤 점에서 controlled 방식이 되는 건지 좀더 말씀해주실 수 있나요?
There was a problem hiding this comment.
state사용해서 작성하고, 성능상의 이슈나, 복잡한 폼일 경우 uncontrolled로 작성하긴 합니다.
|
| const handleUpdate = (e: SyntheticEvent) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (e.target || !params.id) { |
There was a problem hiding this comment.
| if (e.target || !params.id) { | |
| if (!e.target || !params.id) { |
|
추가 코멘트 |
| @@ -1,4 +1,4 @@ | |||
| import { PAGE_SIZE } from "@/components/pagintaion-box"; | |||
| export const PAGE_SIZE = 5; | |||
There was a problem hiding this comment.
페이지네이션 UI에 PAGE_SIZE를 정의하는 게 맞지 않다 느껴져 유틸 쪽으로 옮겼습니다.
아직 상수가 많진 않아 모아두려구요!
There was a problem hiding this comment.
src/constants/index.ts 파일에 추후에는 분리하면 더 좋을 것 같아요
| import { Todo, NewTodo } from "@/types/todo-type"; | ||
|
|
||
| const ModifyTodoBox = () => { | ||
| const params = useParams(); |
There was a problem hiding this comment.
고치다 보니, route로 들어오는 값을 UI 컴포넌트에서 검사하는 게 적절하지 않을 수 있을 것 같습니다.
멘토님 의견이 궁금합니다!
페이지 컴포넌트에서 params.id를 검사하고, ModifyTodoBox에 props로 전달하는 게 어떨까 싶었습니다.
There was a problem hiding this comment.
params 의 값이 채워지기 전까지 로딩을 띄운다던지 요런 조건부 렌더링이 필요해보입니다. 👍
There was a problem hiding this comment.
if (!params) {
return <Loading />
} else {
..
return <ModifyTodoBox />
}
|
|
||
| const LOCAL_STORAGE_KEY = "todos"; | ||
| const useTodos = () => { | ||
| const [todos, setTodos] = useState<Todo[] | null>(null); |
There was a problem hiding this comment.
Todo는 id값을 가지고 있으니 업데이트시 id를 활용하는 방법으로 리팩토링해보면 어떨까 싶습니다!
투두리스트 기능 구현
변경 사항
기타
그런데 생각해보니 이 과제에서 굳이 Vite를 쓸 필요가 없었고, 더욱이 Jest는 공식적으론 Vite 환경에서의 사용을 비추천하고 있네요.
리서치를 먼저 하고 세팅했어야 하는데 죄송합니다.
Vite 사용이 과제 범위에 어긋난다면 다시 세팅해서 올릴게요!