-
Notifications
You must be signed in to change notification settings - Fork 10
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
[2주차] 최민주 미션 제출합니다. #9
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
민주님 안녕하세요! 프론트운영진 안채연입니다
우선 이번주 과제 너무 수고많으셨어요! 열심히 고민한 흔적이 잘 보여서 좋았습니다 ㅎㅎ
저도 처음 리액트를 다룰때 너무나도 어려웠던 경험이 있는데 아마 이렇게 열심히 하시다보면 금방 익숙해지실거라고 생각합니다!! (저도 아직은 많이 부족한 ㅜㅜㅎㅎ)
이번주 과제 너무 수고하셨고 스터디 화이팅입니다!
margin-left: 5%; | ||
margin-top: 3rem; | ||
margin-bottom: 1.5rem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 각각 따로 지정하고 싶으실 경우
margin: 5px 10px 15px 20px
이렇게 한 줄로 작성하시면 더 좋을 것 같아요! 참고자료 놓고 갑니다 %
와rem
을 혼용해서 쓰셨는데 혹시 이유가 있을까요?
const TodoTitle = styled. div` | ||
width: 82%; | ||
height: 32px; | ||
color: white; | ||
font-size: 28px; | ||
text-align: left; | ||
font-weight: bold; | ||
margin-left : 15%; | ||
margin-top : 1.5rem; | ||
margin-bottom: 4px; | ||
display: flex; | ||
` | ||
const DoneTitle = styled. div` | ||
width: 82%; | ||
height: 32px; | ||
color: white; | ||
font-size: 28px; | ||
text-align: left; | ||
font-weight: bold; | ||
margin-left : 15%; | ||
margin-top : 1.5rem; | ||
margin-bottom: 4px; | ||
display: flex; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 TodoTitle
과 DoneTitle
의 스타일이 동일한데 이럴 경우 굳이 컴포넌트를 나누지 않고 하나의 컴포넌트로 사용하는게 코드를 줄일 수 있을 것 같네요!
+) 밑에 있는 컴포넌트들도 중복된게 많아보이는데 하나의 컴포넌트로 사용해주세요!
const TodoBox = styled.div` | ||
position: relative; | ||
width: 82%; | ||
height: 30%; | ||
margin-left: 8%; | ||
margin-top: 1rem; | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
할 일의 개수가 box 크기보다 많아졌을 경우 처리가 안되어서 밑의 영역까지 넘어가게 되는데, overflow: auto
사용해서 스크롤 적용해주시는게 좋을 것 같아요!
const onRemove = id => { | ||
setTodos(todos => todos.filter(todo => todo.id !==id)) | ||
} | ||
const onRemove_dn = id =>{ | ||
setDones(dones => dones.filter(done => done.id !==id)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드가 중복되어 보이는데 하나의 함수로 만들면 어떨까요!
<body> | ||
<Container> | ||
<Title>T O - D O  L I S T</Title> | ||
<TodoInput | ||
addTodoList={addTodoList} | ||
/> | ||
<TodoTitle>To-Do ({todos.length})<TodoRldBtn onClick = {() => {setTodos([])}}>🔄</TodoRldBtn></TodoTitle> | ||
<TodoBox> | ||
<TodoList todos={todos} | ||
onRemove={onRemove} | ||
addDoneList={addDoneList} | ||
onChecked = {onChecked}/> | ||
</TodoBox> | ||
<DoneTitle>Done ({dones.length})<DoneRldBtn onClick = {() => {setDones([])}}>🔄</DoneRldBtn></DoneTitle> | ||
<DoneBox> | ||
<DoneList dones={dones} | ||
onRemove_dn={onRemove_dn} | ||
addTodoList={addTodoList}/> | ||
</DoneBox> | ||
</Container> | ||
</body> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 여기 body
태그는 왜 작성하셨는지 이유가 궁금합니다!
App.css
에 적용한 css
때문이라면 Container
에도 같은 속성이 적용되어 있어서 굳이 써주지 않아도 될 것 같아요~
const RtnBtn = styled.div` | ||
background-color: black; | ||
border: 1.5px solid red; | ||
border-radius: 5%; | ||
height: 23px; | ||
width: 60px; | ||
color : red; | ||
font-size: 19px; | ||
font-weight: medium; | ||
margin-right: 25px; | ||
text-align: center; | ||
` | ||
const DltBtn = styled.div` | ||
background-color: black; | ||
border: none; | ||
border-radius: 5%; | ||
height: 22px; | ||
width: 22px; | ||
color : white; | ||
font-size: 20px; | ||
font-weight: medium; | ||
margin-left : 10px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RtnBtn
과 DltBtn
처럼 완전히 동일한 css
속성은 아니지만 비슷한 속성이 많을 경우 props
를 이용하면 효율적으로 코드 작성이 가능합니다!
참고자료 놓고 갑니다
{dones.map(done =>( | ||
<DoneItem done={done} key = {done.id} onRemove_dn = {onRemove_dn} addTodoList={addTodoList}/> | ||
// 아 뭔가 이렇게 이중으로 안 넘기고 바로 지우는 방법도 있을 것 같은데... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DoneItem
의 리스트를 따로 컴포넌트로 만들지 않고 App.js
에 바로 map
문을 적용하면 한번 더 props
로 넘기는 과정을 생략할 수 있을 것 같네요!
|
||
} | ||
|
||
export default TodoItem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금 TodoItem
과 DoneItem
의 역할이 동일해보이는데, 이럴경우 예를 들어 Item.js
이라는 하나의 파일 안에 두 개의 컴포넌트로 분리하여 작성하는 방법도 있습니다!
그렇게 되면 공통적으로 쓰이는 styled-component
를 재사용함으로써 중복코드를 줄일 수 있고 파일 개수를 줄일 수 있겠죠!
); | ||
}; | ||
|
||
export default TodoList; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마찬가지로 굳이 TodoList
컴포넌트를 만들지 않고 App.js
에 바로 적용하는 방법이 있을 것 같고, 위에서 말씀드린대로 이것도 DoneList
와 TodoList
를 하나의 파일로 묶음으로써 조금 더 효율적으로 작성할 수 있을 것 같네요!
background-color: black; | ||
border: none; | ||
border-radius: 5%; | ||
height: 22px; | ||
width: 22px; | ||
color : white; | ||
font-size: 20px; | ||
font-weight: medium; | ||
margin-left : 10px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
각 버튼에 cursor: pointer
를 추가하면 동작이 좀 더 명확하게 보일 것 같습니다!
@@ -1,8 +1,180 @@ | |||
import {React, useState} from 'react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 이렇게 사용했었는데 React
를 적어주지 않아도 되는 걸 이번 코드 리뷰 하면서 처음 알게 됐어요!!
예린님 과제에서 문기님이 코드 리뷰 해주신 부분 참고하시면 도움 되실 것 같아요!!! 코드 리뷰
text, | ||
checked: false | ||
} | ||
setTodos(todos => todos.concat(todo)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
concat 함수보다 push나 ...(spread) 연산자를 사용하시는 것도 좋을 것 같아요!! 이것도 문기님 코드 리뷰 보고 알았습니당!! 참고하시면 좋을 것 같아요💪🏻💪🏻 코드 리뷰
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
민주님 안녕하세요~~
완료/복귀라고 워딩 다르게 하신거랑 투두 리스트 내부에서도 체크할 수 있는 기능 두신거 좋았어요!
디자인도 게임화면 같고 재밌네요 ㅎㅎ
이번 주차도 고생하셨습니다 ㅎㅎ 스터디 때 뵐게요!
const {id, text, checked} = todo; | ||
const addDone = (text, id) =>{ | ||
onRemove(id); | ||
return addDoneList(id, text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doneList에 추가할 때 addDoneList
를 리턴하지 않고 실행만 시켜도 될 것 같아요!
import React from 'react'; | ||
import styled from 'styled-components'; | ||
|
||
const TextBox = styled.div` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주로 div
를 사용하셨는데 TextBox
의 경우 h1
같이 의미를 담고 있는 태그들을 사용하는게 가독성이 좋을거 같아요!!
const onRemove = id => { | ||
setTodos(todos => todos.filter(todo => todo.id !==id)) | ||
} | ||
const onRemove_dn = id =>{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수명에 camelCase와 underscore를 혼용하셨는데 camelCase로 통일하는건 어떨까요~?
addDoneList={addDoneList} | ||
onChecked = {onChecked}/> | ||
</TodoBox> | ||
<DoneTitle>Done ({dones.length})<DoneRldBtn onClick = {() => {setDones([])}}>🔄</DoneRldBtn></DoneTitle> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
버튼 누르면 한번에 초기화시키는 기능 좋네요 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 민주님!
투두리스트에 다양한 기능들이 있어서 좋았어요~!!
리액트 거의 처음 다뤄보셨다고 하셨는데 잘 구현하신 것 같아요!👍
수고 많으셨고, 다음 과제도 같이 화이팅해봐요~!💪💪
text, | ||
checked: false | ||
} | ||
setTodos(todos => todos.concat(todo)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예지님 말씀처럼 spread(...) 연산자를 사용하는 게 좋을 것 같아요!
setTodos(todos => todos.concat(todo)); | |
setTodos([...todos,todo]); |
margin: 0; | ||
` | ||
const DoneBox = styled.div` | ||
position: relative; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
position속성도 좋지만 flex-box도 많이 활용해보셨으면 좋겠어요! 정말 편리한 아이랍니다ㅎㅎ
border-style: solid; | ||
border-width: 1px; | ||
border-color: white; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 한 줄로 적을 수 있어요!
border-style: solid; | |
border-width: 1px; | |
border-color: white; | |
border: 1px solid white; |
import {MdCheckBoxOutlineBlank} from "react-icons/md"; | ||
import {BsSquareFill} from "react-icons/bs"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
react-icons 저도 나중에 사용해보고 싶어요..!
리액트 거의 처음 만져보는 거라 걱정이 이만저만이 아니었는데, 계속 붙잡고 있다보니 어..? 이게 왜 돌아가지.. 하며 버텨낸 한 주였습니다 ^ㅡ^ 해보고 싶은 게 많았는데, 확실히 아는 게 적다보니 기본적인 것에서 막히고, 공부하고, 분석하고.. 아쉬움이 많이 남네요. 구조적인 것 많이 봐주시면 감사하겠습니다! 지금 다른 분들 어떻게 코드 짰는지 배우고 싶어서 근질근질..
일단 다 구현은 했는데 아 그냥 toggle로 display: none 값 줄 걸 그랬나, 다른 방법 있지 않을까 자꾸 생각이 납니다. 열심히 공부할 결심!! 참고로, 빨간 박스 체크하면 맨 위로 정렬되게 하는 기능을 갑자기 넣고 싶어서 계속 붙들고 있었는데 뭐가 잘 안 돼서... ㅠ 일단 제출하고 추가해보겠습니다.
아, 갑자기 trim 함수랑 저번에 알려주신 스크롤 함수 안 넣은 것도 생각나네요.. 열심히 수정해놓겠습니다
배포 링크 : https://react-todo-17th-hmuri.vercel.app/
Virtual-DOM은 무엇이고, 이를 사용함으로서 얻는 이점은 무엇인가요?
아직 조금은 추상적으로 와닿지만, 운영체제에서 각 사용자들에게 가상의 컴퓨터 머신이 하나씩 부여된다고 여기는 것처럼 리액트의 Virtual-Dom 또한 본래의 Dom에 매번 접근하는 수고 대신 가상의 돔을 만들어 객체를 제어하게 된다. 이렇게 virtual DOM을 사용할 때의 장점은 그 무게가 상당히 가벼워진다는 점인데, 이유는 react가 가상의 돔을 만들어 상태를 비교하고, 변화된 부분만을 찾아내기 때문에 실제 DOM이 전체가 새로고침 되는 것이 아니라 일정 부분만 렌더링하여 운용이 가능하기 때문이다.
미션을 진행하면서 느낀, React를 사용함으로서 얻을수 있는 장점은 무엇이었나요?
뭔가 바닐라를 쓸 때는 한 편의 글을 쓰는 느낌이었다. 서론이 이렇고, 본론이 이런 식으로 전개되어 결론적으로는 이러한 페이지가 나왔습니다! 하는 느낌. 그래서 그 논리 구조를 풀어가는 맛이 있는 대신, 상당히 조심스럽고, 논리를 따지며 한 자 한 자 적게 된달까? 반면에 React는 퍼즐 놀이를 하는 기분이었다. 정확히는 중학교 때 했던 스크래치 놀이? 내가 필요한 내 기능들을 Component로 만들고, 그 친구들을 능동적으로 app.js에 가져와 사용하고, 내 마음대로 빈 칸을 생성해놓고 그 안을 뭘로 채울까 고민하며 퍼즐을 맞춰가는 느낌이었다. 퍼즐을 맞춘다는 것은 조각조각 나 있는 부분들로 큰 그림을 완성시킬 수 있다는 것! 게다가 그 퍼즐 퍼즐을 내가 원하는 색으로 칠할 수 있으니까 그 편리함이 이루 말할 수 없다.
React에서 상태란 무엇이고 어떻게 관리할 수 있을까요?
[a, b] = useState(). b라는 함수를 통해 a라는 변수의 값을 조정해줄 수 있다. a의 상태를 조정해주는 b라는 함수. 약간 a는 음료수 컵 같다. b는 레시피고? 그래서 레시피에 따라서 내가 원하는 음료수를 만들어 그 결과물을 a에 넣어주는 것이다. 보통 b의 이름을 set'a' 로 저장을 하고, 그 때 그 때 b의 함수 내용을 바꾸어서(레시피를 바꾸어서) 다른 방식으로 a의 값을 도출해낼 수 있다.
Styled-Components 사용 후기 (CSS와 비교)
일단, 한 파일 안에 내가 제어해야 할 박스들과 그 박스들을 꾸며줄 style이 함께 들어있다는 건 상당히 편리하다. 내가 고치고 싶은 스타일이 생겼을 때 어디 함수인지 찾고, 그 파일이 저장된 html과 거기에 해당하는 css를 찾는 두 번의 일을 한 번으로 줄여줄 수 있다. 게다가 스타일이 각 components들에 포함되어 있기 때문에 컴포넌트 파일만 다른 파일에 넘겨주면 css 파일을 따로 연결시켜줄 필요 없이 함께 스타일까지 따라갈 수 있다. 정말 기능이 많은 것 같은데, 폰트 관리할 때도 편하고 특히 media query도 theme.js 사용해서 넣어보고 싶었는데... 다음 과제 때는 지금 기술한 것 이상으로 꼭 더 다양한 기능을 적용시켜보고 싶다.