-
Notifications
You must be signed in to change notification settings - Fork 13
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
[3주차] 전시원 미션 제출합니다. #23
base: master
Are you sure you want to change the base?
Conversation
1. 인풋 폼 관리 2. 데이터 입력받은 후 object => list 로 저장 3. list.map 을 이용하여 각 요소 랜더링
1. useEffect 를 이용하여 모든 데이터를 담고 있는 list 값이 변화할 때마다 localStorage 에 값을 넣어줌 2. 랜더링된 값을 누르게 되면 그 값을 추적하기 위하여 onToggle 함수를 만들었고 아직 기능 구현을 덜 하였음
1. submit => object 생성 => list 에 넣음 2. list 를 랜더링함 3. 이때 done, yet 리스트를 구분해서 isDone 상태에 따라 각각 랜더링 4. 내용을 누르게 되면 isDone 토글시켜줌 <아직 필요한 부분> 1. 세부적인 레이아웃 등 2. 삭제 버튼 추가해야 함 3. ** useEffect으로 list 값이 변경될 때마다 로컬스토리지에 데이터를 저장하고 그 값을 리랜더링했는데 이거 때문에 전체 데이터가 날아가고 있음
고친 이유 먼저 list 를 발고 list.map 을 돌리면서 하나 하나 isDone 을 확인함 그 후 isDone 의 상태에 따라 랜더링을 할지 정하는데 그렇게 되면 랜더링을 안해도 li의 . 이 화면에 나타나게 됨 이거 어떻게 고치지 고민하다가 그냥 div 로 바꿈 좋은 방법은 아닌 거 같긴함
삭제를 누르면 항목을 지움 <궁금한 점> onDelete 함수에서 onToggle 함수와 임시 변수 생성 부분이 겹치는데 이런 경우는 겹치는 코드를 어떻게 처리해야 하는가 (이 경우는 두세줄밖에 안되는데 이 부분이 길어지면 어떻게 처리해야할지 궁금함)
findNum 함수 사용: 로컬스토리지에 있는 list 불러옴 isDone 의 상태에 따라 값 세어줌 state 에 반영해서 화면에 렌더링
<Problem> 데이터가 계속 초기화 됐음 <Solution> 처음에 list 데이터를 빈 값으로 추지 않고 로컬스토리지에서 값을 불러온 후 존재 여부에 따라 초기화 시킴
Add <InputForm/>
Add <YetList/>
Add <DoneList/>
Add <Title/>
<Problem> 입력에 'a a' (세번 띄우고 입력) 하면 로컬스토리지에는 공백포함 그대로 들어가는데 node 에는 'a a' 로 들어감 즉, 아무리 많은 공백이 있어도 하나로 처리 그렇게 되면서 삭제가 안됨 <Solution> 아직 못 찾음. CSS 하다가 발견한 거라 우선 그냥 기록만 하고 넘김
input maxLength='23' 으로 설정 CSS 설정하다가 디자인 맞추려고 최대 글자수를 정하기로 함 아무 말 넣기 '친구 만나서 사당에서 밥약하고 로욜라 가기' 23자 라서 23자로 설정
<Problem> YetList/index.js 에서 list 를 랜더링할 때 Styled-component를 사용하였음 그렇게 하니까 onDelete 함수가 다르게 동작함 onDelete 함수 내부에서 원인을 찾아보니updatedDate 부분의 filter 가 제대로 동작하지 않음 type, value 모두 같은데 둘을 다른 값으로 인식해서 걸러지지가 않음 <Solution> 못 찾음
<AllContents/> component onDelete 함수가 제대로 동작하지 않음. <Problem> 원인 : 랜더링할 때 styled-component 를 적용하니 자동으로 공백이 생겨서 문제가 발생함 <Solution> .trim() 넣어서 해결
과제 끝 css commit 을 엉망으로 함 다음부턴 안그래야지
Component <Title/> - solve type error - Delete Tab close function
<InputForm/> - Check Type
<DoneList/> - type check
인라인으로 쓸 때 "" 나뒀다가 안지워서 styled-component 로만들때 다시 지워줌
인라인 스타일링 styled-component 로
인라인 스타일 없애고 스타일 컴포넌트로 변경
로컬스토리지 값을 매번 불러오는데 type 지정을 위해서 string 인지 아닌지 확인이 필요함. 이를위해 단순 getlocalStorage 가 아니라 반복 로직이 필요해서 커스텀 훅을 제작함
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.
export interface ITodoList { | ||
contents: string; | ||
id: number; | ||
isDone: boolean; | ||
} | ||
export interface IYetListProps { | ||
list: []; | ||
yetNum: number; | ||
onToggle: (e: React.MouseEvent<HTMLButtonElement>) => void; | ||
onDelete: (e: React.MouseEvent<HTMLButtonElement>) => void; | ||
} | ||
|
||
export interface IInputFormProps { | ||
handleFormSubmit: (e: React.SyntheticEvent) => void; | ||
handleContentsChange: (e: React.ChangeEvent<HTMLInputElement>) => void; | ||
contents: string; | ||
} | ||
|
||
export interface IDoneList { | ||
list: []; | ||
onToggle: (e: React.MouseEvent<HTMLButtonElement>) => void; | ||
doneNum: number; | ||
} | ||
// styled-component에 들어가는 property 에 대한 정의 | ||
export interface IListBtn { | ||
onClick: (e: React.MouseEvent<HTMLButtonElement>) => void; | ||
} |
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.
Props까지 모두 interface로 정리하신게 인상깊은 것 같아요. Type이 아닌 Interface로 정의하신 점이 궁금하게 되는 것 같습니다, 찾아보니까 props는 interface로 정의하는게 관례라고 하는군요. 감사합니다!
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 Index = () => { | ||
const listText = 'list'; | ||
|
||
const localData = getLocalValue(); |
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 onToggle = (e: React.MouseEvent<HTMLButtonElement>) => { | ||
// 클릭된 값 | ||
|
||
const text = (e.target as HTMLElement).textContent; |
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.
TS에 as라는 문법이 있는지 처음 알게되었습니다. 감사합니다!
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.
HTML 구조에 의존적인 로직보다는, 클릭한 요소의 id를 필터링해서 토글시키는 방법이 좋을 것 같습니다.
let indexClicked = 0; | ||
let obj; | ||
data.map((item: ITodoList, index: number) => { | ||
if (item.contents === text) { | ||
obj = { ...item }; | ||
obj.isDone = !item.isDone; | ||
indexClicked = index; | ||
} | ||
}); | ||
const updatedDate = [...data]; | ||
updatedDate[indexClicked] = obj; | ||
setList(updatedDate); |
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.
이런 식으로도 단순화 할 수 있을 것 같아요!
let indexClicked = 0; | |
let obj; | |
data.map((item: ITodoList, index: number) => { | |
if (item.contents === text) { | |
obj = { ...item }; | |
obj.isDone = !item.isDone; | |
indexClicked = index; | |
} | |
}); | |
const updatedDate = [...data]; | |
updatedDate[indexClicked] = obj; | |
setList(updatedDate); | |
setList([...data.map((item: ITodoList):ITodoList=>( | |
item.contents === text ? {...item, isDone: !item.isDone} : item | |
))]) |
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 text = ( | ||
(e.target as HTMLElement).parentNode as HTMLElement | ||
).innerText.slice(0, -4); |
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.
맞습니다, 이상한 공백을 끌고 오더라구요 innerText하면
let yetCtn = 0; | ||
let doneCtn = 0; | ||
|
||
// isDone === true, false 인 경우에 따라서 개수를 세어 줘야 함 | ||
data.map((item: ITodoList) => { | ||
if (item.isDone === false) { | ||
yetCtn += 1; | ||
} else { | ||
doneCtn += 1; | ||
} | ||
}); | ||
setYetNum(yetCtn); | ||
setDoneNum(doneCtn); |
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.
약간 이런 느낌으로 줄일 수 있을 것 같아요~
let yetCtn = 0; | |
let doneCtn = 0; | |
// isDone === true, false 인 경우에 따라서 개수를 세어 줘야 함 | |
data.map((item: ITodoList) => { | |
if (item.isDone === false) { | |
yetCtn += 1; | |
} else { | |
doneCtn += 1; | |
} | |
}); | |
setYetNum(yetCtn); | |
setDoneNum(doneCtn); | |
setYetNum(data.filter((item: ITodoList) => item.isDone === false).length); | |
setDoneNum(data.filter((item: ITodoList) => item.isDone === true).length); |
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.
아 이부분도 다른 분께 리뷰를 받았던 것 같은데 제가 바꾸질 못했네요 짚어주셔서 감사합니다!
export const ListBtn = styled.button<IListBtn>` | ||
cursor: grab; | ||
color: white; | ||
background: blue; | ||
border: 1px dotted blue; | ||
opacity: 0.5; | ||
text-decoration: line-through; | ||
margin-bottom: 5px; | ||
`; |
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.
세은님 파일에서도 generic 사용하셔서 뭔가 싶었는데 찾아보니 테마 만들때 자주 사용한다고 하네요.
디자인 시스템과도 연관이 있을지 궁금하네요~
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.
시원님 안녕하세요
프론트엔드 멘토 김선종입니다
타입스크립트, 어렵지 않으셨나요? 인터페이스 정의 해주신 것을 보니까 타입스크립트 공부를 열심히 하신것 같습니다. 제가 처음 타입스크립트를 공부할 때는 이렇게까지 하지 못했었는데,,, 대단하십니다. 또한 파일 구조를 세분화해서 잘 나눠주신 것도 인상적이었습니다. 다만 todo item을 핸들링하는 과정에서 데이터를 html에서 직접 가져오는 패턴이 보이던데, 이 부분은 최대한 지양하시는 것이 좋습니다. 모든 핸들링은 state를 기준으로 하는 것이 맞다고 생각이 듭니다. 이 부분을 염두해서 한번 더 리팩토링 해보시면 좋을 것 같아요.
과제 하느라 고생하셨습니다
isDone: boolean; | ||
} | ||
export interface IYetListProps { | ||
list: []; |
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.
어떤 array인지 좀더 정확하게...!
list: []; | |
list: ITodoList; |
const onToggle = (e: React.MouseEvent<HTMLButtonElement>) => { | ||
// 클릭된 값 | ||
|
||
const text = (e.target as HTMLElement).textContent; |
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.
HTML 구조에 의존적인 로직보다는, 클릭한 요소의 id를 필터링해서 토글시키는 방법이 좋을 것 같습니다.
@@ -0,0 +1,16 @@ | |||
const getLocalValue = (listName = 'list') => { |
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.
커스텀 혹은 useXXX()
로 네이밍하는게 컨벤션입니다. 그렇지 않으면 리액트에서 이 함수를 훅으로 인식하지 못하기 때문이에요.
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.
안녕하세요 시원님 이번 코드 리뷰 파트너가 유세은입니다.
시원님께서 코드를 파일별로 분리해주셔서 리뷰하기 좋았습니다 감사합니다 ! 저번 과제 때에도 느꼈지만 디자인에 신경 쓰신 부분이 느껴졌습니다 ㅎㅎ 다른 분들께서 코멘트를 잘 남겨주셔서 몇가지만 코멘트 남겨드렸습니다 ! 사실 시원님 코드를 보면서 제 코드도 반성하게 되었던 것 같아요 ㅠㅠ 고생 많으셨습니다~! 스터디 시간때 뵙겠습니다 ^_^
if (item.isDone === false) { | ||
yetCtn += 1; |
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.
(!item.isdone) 이런 식으로 바꾸어주면 훨씬 좋을 것 같아요!
<Etc>⌥⌘1</Etc> | ||
</DivTitle> |
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.
앗 이거 이모지 너무 기여워요
@@ -59,7 +64,7 @@ const Index = () => { | |||
|
|||
// 현재 데이터 가져오기 | |||
|
|||
const data = getLocalValue() | |||
const data = getLocalValue(); | |||
// 데이터 삭제 | |||
const updatedDate = data.filter((item: ITodoList) => { | |||
return item.contents.trim() !== text.trim(); |
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.
trim이라는 함수를 찾아보니까 문자열 양끝 공백을 사라지게 하는 함수군요 ! 디테일 배워갑니당
id: Date.now(), | ||
isDone: false, | ||
}; | ||
setList((prev: []) => [...prev, obj]); |
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.
타입을 string[] 이나 Array으로 해주면 명확해질 것 같아요!!
안녕하세요. 15기 프론트 전시원입니다.
타입스크립트 적용하고 생기는 오류를 해결하는 데 시간이 많이 걸리네요.
지각 제출해서 죄송합니다.
배포 링크 : https://react-todo-15th-ten.vercel.app/