[2주차] 고은 미션 제출합니다.#3
Conversation
gywlsp
left a comment
There was a problem hiding this comment.
이번 미션 버거울 수 있었을 텐데 잘 구현해주셔서 좋습니다 👍
수정하면 좋을 만한 것들 리뷰 달았으니 읽어보시고 리뷰마다 커밋 남기고 푸쉬 부탁드려용
자세히 설명하면 좋을 것 같아 쓰다 보니 넘 길어졌네요..ㅎ
고생하셨습니당 :)
| const Subject = styled.div` | ||
| font-size: 3rem; | ||
| font-weight: 600; | ||
| padding: 0px; | ||
| margin: 0px 0px 3rem; | ||
| color: white; |
There was a problem hiding this comment.
| const Subject = styled.div` | |
| font-size: 3rem; | |
| font-weight: 600; | |
| padding: 0px; | |
| margin: 0px 0px 3rem; | |
| color: white; | |
| const Title = styled.h1` | |
| font-size: 3rem; | |
| font-weight: 600; | |
| margin-bottom: 3rem; | |
| color: white; |
- 제목을 나타내는 태그이니까 네이밍을
Title로 하면 좋을 것 같아요. - 제목을 적는 태그라서
div보단h1을 사용하면 좋습니다. padding: 0px코드는 꼭 필요해 보이진 않네요!- 아래에만 margin을 주고 싶다면
margin-bottom: 3rem과 같이 써주시면 돼용
| font-size: 0.1rem; | ||
| display: flex; | ||
| flex-direction: row; | ||
| justify-content: space-between; |
There was a problem hiding this comment.
| justify-content: space-between; |
<Space/>를 통해 여백을 준다면 굳이 justify-content: space-between속성 안 주셔도 됩니당
| const Space = styled.div` | ||
| flex: 0.3; | ||
| `; |
There was a problem hiding this comment.
| const Space = styled.div` | |
| flex: 0.3; | |
| `; | |
| const Space = styled.div` | |
| flex: 1; | |
| `; |
flex 값을 1로 하나 0.3으로 하나 차이는 없겠지만, 보통 이런 경우 1을 많이 씁니다! 참고
| const [date, setDate] = useState(); | ||
| const [todo, setTodo] = useState(); |
There was a problem hiding this comment.
| const [date, setDate] = useState(); | |
| const [todo, setTodo] = useState(); |
이 state들이 pages/index.js에 존재해야 할 필요가 있을까요?
components/todo-input.js에 props로 넘겨줄 필요 없이 그 컴포넌트 안에서 선언해주시면 됩니다.
| const [date, setDate] = useState(); | ||
| const [todo, setTodo] = useState(); | ||
| const [todos, setTodos] = useState([]); | ||
| const [id, setID] = useState(0); |
There was a problem hiding this comment.
| const [id, setID] = useState(0); |
id 없이 todo를 삭제할 수 있어요! 리뷰 참고해주세요. 이 state는 불필요합니당.
| return ( | ||
| <Form> | ||
| <InputSection> | ||
| <P>시간</P> | ||
| <Input | ||
| maxlength="8" | ||
| oninput="maxLengthCheck(this)" | ||
| type="number" | ||
| placeholder="날짜를 입력하세요 (ex.20200404)" | ||
| value={date} | ||
| onChange={onChangeDate} | ||
| /> | ||
| </InputSection> | ||
|
|
||
| <InputSection> | ||
| <P>TODO</P> | ||
| <TextArea | ||
| placeholder="할 일을 입력하세요" | ||
| value={todo} | ||
| onChange={onChangeTodo} | ||
| /> | ||
| </InputSection> | ||
| <Button onClick={onSubmit}>등록</Button> | ||
| </Form> | ||
| ); |
There was a problem hiding this comment.
| return ( | |
| <Form> | |
| <InputSection> | |
| <P>시간</P> | |
| <Input | |
| maxlength="8" | |
| oninput="maxLengthCheck(this)" | |
| type="number" | |
| placeholder="날짜를 입력하세요 (ex.20200404)" | |
| value={date} | |
| onChange={onChangeDate} | |
| /> | |
| </InputSection> | |
| <InputSection> | |
| <P>TODO</P> | |
| <TextArea | |
| placeholder="할 일을 입력하세요" | |
| value={todo} | |
| onChange={onChangeTodo} | |
| /> | |
| </InputSection> | |
| <Button onClick={onSubmit}>등록</Button> | |
| </Form> | |
| ); | |
| return ( | |
| <Wrapper> | |
| <Row> | |
| <Label>시간</Label> | |
| <Input | |
| type="number" | |
| name="date" | |
| placeholder="날짜를 입력하세요 (ex.20200404)" | |
| onChange={handleFormChange} | |
| /> | |
| </Row> | |
| <Row> | |
| <Label>TODO</Label> | |
| <TextArea | |
| name="todo" | |
| placeholder="할 일을 입력하세요" | |
| onChange={handleFormChange} | |
| /> | |
| </Row> | |
| <Button onClick={onSubmit}>등록</Button> | |
| </Wrapper> | |
| ); |
- self-closing 태그 잘 사용해주셨네요 👍
- 컴포넌트 이름 바뀐 건 아래 리뷰들을 참고해주세요.
- 위에 handleFormChange가 잘 작동하게 하기 위해
nameattribute를 추가해줬어용 - onInput 문법이 존재하긴 하지만 리액트 팀에서는 이것의 사용을 추천하지 않고 있습니다. 참고
onChange가 있는데 onInput을 또 써줄 필요는 없으므로 삭제해주시면 됩니다. input에value를 넣어줄 필요는 없어요!value를 써주지 않았는데handler가 어떻게e.target.value를 파악하는지 궁금하다면 다음을 참고해주세요. 참고
| </Wrapper> | ||
| ); | ||
| export default function TodoList(props) { | ||
| const { data, onRemove } = props; |
There was a problem hiding this comment.
| const { data, onRemove } = props; | |
| const { todos, onDelete } = props; |
왜 이렇게 바뀌었는지는 pages/index.js의 리뷰를 참고해주세용
| const list = data | ||
| .sort((todo1, todo2) => { | ||
| return todo1.date - todo2.date; | ||
| }) | ||
| .map((todolist) => ( | ||
| <TodoItem key={todolist.id} {...todolist} onRemove={onRemove} /> | ||
| )); | ||
| return <TodoListWrapper> {list} </TodoListWrapper>; |
There was a problem hiding this comment.
| const list = data | |
| .sort((todo1, todo2) => { | |
| return todo1.date - todo2.date; | |
| }) | |
| .map((todolist) => ( | |
| <TodoItem key={todolist.id} {...todolist} onRemove={onRemove} /> | |
| )); | |
| return <TodoListWrapper> {list} </TodoListWrapper>; | |
| return( | |
| <Wrapper> | |
| {todos.sort((todo1, todo2) => { | |
| return todo1.date - todo2.date; | |
| }) | |
| .map((todoObj, index) => ( | |
| <TodoItem key={JSON.stringify(todo)} {...{todoObj}} onDelete={onDelete(index)} /> | |
| )) | |
| } | |
| </Wrapper> | |
| ); |
- 가장 바깥 태그 이름은
Wrapper로 적어주시면 됩니다!
어떤 역할을 하는지는 file 이름에서 알 수 있으니까요 :) - javascript prototpe function으로만 이루어진 코드(
sort(), map()등)는 그냥 리턴 값 안에 넣어주시면 돼요! - key에 id 값을 넣으셨는데 id가 사실 index 값이기 때문에 이를 key로 쓰는 건 좋지 않아요.
그리고 id는 미션 내용을 구현하는데 필요하진 state가 아니라고 말씀 드렸죠!
date, todo 값 모두 다른 값들과 겹칠 우려가 있어서 이런 경우에는 그냥 객체를 string으로 만들어 key로 설정해주기도 한답니다. 참고 todos를 map했기 때문에 매개변수 이름은todolist가 아닌todo가 되어야 합니다.
그런데 todo의 내용 property 이름을 todo라고 정의하셔서 TodoItem에서 이름이 겹치기 때문에todoObj와 같은 이름으로 써주시면 됩니다!
사실 property 이름을content로 바꾸는 게 제일 좋아요 :)pages/index.js에서 handleDelete를 다음과 같이 정의했었죠.
const handleDelete = (index) => () => {
setTodos([...todos.slice(0,index), ...todo.slice(index+1, todos.length)]);
};
todos를 map할 때 매개변수 index 값을 넣어서 onDelete={onDelete(index)}와 같이 전달하면
() => {
setTodos([...todos.slice(0,index), ...todo.slice(index+1, todos.length)]);
};
위와 같은 함수에 index 값이 채워진 채로 TodoItem에게 전달됩니다.
그러면 자식 컴포넌트가 자신의 index를 몰라도 onDelete를 잘 사용할 수 있겠죵
6. 저번 미션 같은 경우 profile 변수만 props로 넘겨줬기 때문에 spread operator를 이용해 넘겨줬지만
지금과 같은 경우 todo와 onDelete가 엄연히 다르기 때문에 위와 같이 넘겨주는 게 좋습니다.
| const { id, date, todo, onRemove } = props; | ||
|
|
||
| return ( | ||
| <TodoItemWrapper> | ||
| <Todo>{todo}</Todo> | ||
| <Date> | ||
| {date} | ||
| <Button onClick={() => onRemove(id)}>완료</Button> | ||
| </Date> | ||
| </TodoItemWrapper> | ||
| ); | ||
| } |
There was a problem hiding this comment.
| const { id, date, todo, onRemove } = props; | |
| return ( | |
| <TodoItemWrapper> | |
| <Todo>{todo}</Todo> | |
| <Date> | |
| {date} | |
| <Button onClick={() => onRemove(id)}>완료</Button> | |
| </Date> | |
| </TodoItemWrapper> | |
| ); | |
| } | |
| const { todo, onDelete } = props; | |
| const { date, todo } = todo; | |
| return ( | |
| <Wrapper> | |
| <Row> | |
| <P>{todo}</P> | |
| <P>{date}</P> | |
| </Row> | |
| <DeleteButton onClick={onDelete}>완료</Button> | |
| </Wrapper> | |
| ); | |
| } |
props, todos에서 왜 위와 같이 뽑는지는components/todo-list의 리뷰 확인해주세용- 컴포넌트 네이밍 바뀐 거는 아래 styled-component 리뷰들 참고해주세용
- 날짜를 나타내는 태그 안에 버튼이 있는 게 어색해요. 위와 같은 코드가 어떻게 작동하는지는 아래 stylec-component 리뷰 참고해주세용.
- 맨 바깥 태그는 Wrapper라고 네이밍 해주시면 됩니다.
| const Date = styled.div` | ||
| font-size: 1.5rem; | ||
| display: flex; | ||
| color: white; | ||
| flex-direction: column; | ||
| `; | ||
| const Todo = styled.div` | ||
| font-size: 1.5rem; | ||
| color: white; | ||
| display: flex; | ||
| flex-direction: column; | ||
| `; | ||
| const Button = styled.button` | ||
| color: white; | ||
| background-color: rgb(97, 97, 97); | ||
| font-size: 1.5rem; | ||
| outline: none; | ||
| border-width: initial; | ||
| border-style: none; | ||
| padding: 0.5rem 1rem; | ||
| border-radius: 0.3rem; | ||
| `; |
There was a problem hiding this comment.
| const Date = styled.div` | |
| font-size: 1.5rem; | |
| display: flex; | |
| color: white; | |
| flex-direction: column; | |
| `; | |
| const Todo = styled.div` | |
| font-size: 1.5rem; | |
| color: white; | |
| display: flex; | |
| flex-direction: column; | |
| `; | |
| const Button = styled.button` | |
| color: white; | |
| background-color: rgb(97, 97, 97); | |
| font-size: 1.5rem; | |
| outline: none; | |
| border-width: initial; | |
| border-style: none; | |
| padding: 0.5rem 1rem; | |
| border-radius: 0.3rem; | |
| `; | |
| const Row = styled.div` | |
| display: flex; | |
| flex-direction: row; | |
| justify-content: space-between | |
| `; | |
| const P = styled.p` | |
| font-size: 1.5rem; | |
| color: white; | |
| `; | |
| const DeleteButton = styled.button` | |
| color: white; | |
| background-color: rgb(97, 97, 97); | |
| font-size: 1.5rem; | |
| outline: none; | |
| border: none; | |
| border-radius: 0.3rem; | |
| padding: 0.5rem 1rem; | |
| width: fit-content; | |
| margin-left: auto; | |
| `; |
Date, Todo의 style이 겹치네요!p태그로 P 선언해서 써주시면 될 것 같아요 :)- 버튼의 역할을 정확하게 나타내기 위해
DeleteButton으로 구체화하면 좋을 것 같아요~ - 결과 화면에서
TodoItem의 구조를 살펴보면,
첫 번째 행 양 끝에 내용과 날짜가 있고, 그 다음 행 오른쪽 끝에 완료 버튼이 있죠!
그래서Date태그 안에 완료 버튼을 넣는 게 어색하게 느껴져요.
내용, 날짜를 나타내는 태그를 묶으면서 한 행에 정렬하는Row태그 선언한 후 그 안에P태그들 넣어주시고,
DeleteButton에margin-left: auto를 주어 화면과 같은 스타일을 구현할 수 있어요. 참고
이번주차는 저번보다 확실히 더 어렵고 시간도 많이 걸린것같습니다.
이벤트 핸들링을 다뤄볼수있어서 흥미로웠습니다.
input을 두개를 다르게 받는부분에서 시간을 많이 잡아먹었던것 같아요
아직 state에 대한 이해가 많이 부족하다는 것을 느꼈습니다.
함수형으로도 다시 만들어봐야할것같아요ㅜㅜ
->완료했습니다!
https://react-todo-11th.eun-ko.now.sh