-
Notifications
You must be signed in to change notification settings - Fork 10
[2주차] 정성훈 과제 제출합니다. #6
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
base: master
Are you sure you want to change the base?
Conversation
hammoooo
left a comment
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.
전반적으로 컴포넌트와 타입의 분리가 잘 되어있어 좋은 코드라고 생각합니다! 과제 고생하셨습니다
| {todos.length > 0 ? ( | ||
| todos.map((todo, index) => ( | ||
| <ListItem key={index}> | ||
| <TodoText>{todo.text}</TodoText> | ||
| <DeleteButton onClick={() => onDelete(index)}>삭제</DeleteButton> | ||
| </ListItem> | ||
| )) | ||
| ) : ( | ||
| <ListItem> | ||
| <TodoText>비었어요</TodoText> | ||
| </ListItem> | ||
| )} |
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.
파비콘 적용하셨네요! 👍
| )) | ||
| ) : ( | ||
| <ListItem> | ||
| <TodoText>비었어요</TodoText> |
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.
비어있다고 알려주는 거 좋은 거 같아요..! 저는 그냥 비어있는 상태로 두었는데, 이렇게 바꿔봐야겠다고 생각했습니다!👍
| value={value} | ||
| onChange={(e) => onChange(e.target.value)} | ||
| /> | ||
| <button onClick={onAdd}>등록</button> |
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.
성아님이 좋은 말씀 해주신 거 같아요. 이처럼 UX 향상을 위해서 고민해보시면 더 좋은 결과물을 얻을 수 있습니다
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.
미리 말씀드리면 이 과정에서 한글이 두 번 입력되는 현상이 발생할 수 있는데요, isComposing 속성에 대해 공부해 보시면 좋을 것 같습니다
| {todos.length > 0 ? ( | ||
| todos.map((todo, index) => ( | ||
| <ListItem key={index}> | ||
| <TodoText>{todo.text}</TodoText> |
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.
투두 리스트이다 보니, 완료 체크 표시할 수 있는 기능을 추가하면 좋을 거 같습니다..! 그리고 혹시 제가 못 찾는걸 수도 있지만, 총 투두 개수와 완료 투두 개수를 보여주는 기능이 없는 거 같아요 이것도 추가하면 좋을거 같습니다..!!
KWONDU
left a comment
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.
좋은 코드 감사합니다 -! 사정상 이번 주 성훈님 코드 리뷰는 채영님 대신 제가 담당하게 되었습니다
| @@ -1,69 +1,3 @@ | |||
| # 2주차 과제: React 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.
readme 작성해 주신 거 좋은 것 같아요. 다음 과제에는 조금 더 많은 내용을 작성해보아도 좋을 거 같아요
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 환경 구축하실 때 현재 폴더 기준으로 하실지, 이렇게 새로운 폴더를 만드실지 고려해보시는 것도 좋을 거 같아요
| "styled-components": "^6.1.19" | ||
| }, | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.33.0", |
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.
다양한 plugin도 사용하셨네요 좋은 거 같아요
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.
favicon도 그렇고 이미지는 svg 확장자로 사용하시는 게 좋습니다 (jpg, png는 pixel 기반인데 반해 svg는 vector 기반이어서 확대/축소 해도 이미지 깨짐이 없음)
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 환경 설정 시 생긴 더미 데이터는 삭제하시면 더 좋을 거 같아요
| currentDate: string; | ||
| onPrev: () => void; | ||
| onNext: () => 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.
이렇게 기능/구조 별로 컴포넌트 분리하시고 interface 통해서 props 조절하신 것 너무 좋은 습관입니다
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.
추가적으로 components 폴더 내에 모든 파일을 몰아넣는 것이 맞을까? Header, Input은 semantic tag 네이밍인데 List는 그렇지 않다. 등 폴더 구조에 대해서 더 고민해보시면 너무 좋은 코드가 될 거 같아요
| value={value} | ||
| onChange={(e) => onChange(e.target.value)} | ||
| /> | ||
| <button onClick={onAdd}>등록</button> |
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.
성아님이 좋은 말씀 해주신 거 같아요. 이처럼 UX 향상을 위해서 고민해보시면 더 좋은 결과물을 얻을 수 있습니다
| value={value} | ||
| onChange={(e) => onChange(e.target.value)} | ||
| /> | ||
| <button onClick={onAdd}>등록</button> |
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.
미리 말씀드리면 이 과정에서 한글이 두 번 입력되는 현상이 발생할 수 있는데요, isComposing 속성에 대해 공부해 보시면 좋을 것 같습니다
| @@ -0,0 +1,63 @@ | |||
| import React from 'react'; | |||
| import styled from 'styled-components'; | |||
| import { type Todo } from '../Type'; | |||
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.
이렇게 하셔도 되긴 합니다만, 이 경우는 보통 컴포넌트랑 같이 혼합해서 가져올 때 많이 사용합니다
| <h2>투두리스트</h2> | ||
| <Container> | ||
| <Button onClick={onPrev}>이전</Button> | ||
| {currentDate} |
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.
개인적으로 완성도가 떨어진다고 느낀 부분인데요, 이처럼 currentDate 도 text string이다보니 <p> 또는 <span> 태그로 감싸주시는 게 좋습니다.
배포링크 : https://react-todo-22nd-oja9.vercel.app/
타입스크립트를 사용해본 적이 없어서 이번 기회에 사용해보려고 노력했는데, 생각보다 많이 어려웠다… 타입을 만드는 것도 그렇고 이걸 prop으로 넘기는 것도 아직 잘 모르겠어서 고생을 했다. 생각보다 시간이 더 필요할 거 같다.
Virtual-DOM은 무엇이고, 이를 사용함으로서 얻는 이점은 무엇인가요?
웹 페이지는 일종의 Document인데, 웹 페이지의 요소들에 접근하기 위해 DOM을 이용한다. 하지만 DOM은 상당히 무겁다. 각각에 요소에 한번 접근하면 다시 리렌더링이 일어나기 때문에 10개에 요소를 변경한다면 리렌더링이 총 10번 리렌더링이 일어나게 되는 것.
리액트는 이를 해결하기 위해 버추얼 돔을 도입했다. 가상 DOM을 통해 1번의 리렌더링으로 여러 요소를 사용할 수 있게 된 것이다.
React.memo(), useMemo(), useCallback() 함수로 진행할 수 있는 리액트 렌더링 최적화에 대해 설명해주세요.
React.memo는 마지막으로 렌더링된 값을 메모이제이션 해 props가 변하지
않고 결과도 같다면 불필요한 렌더링 없이 기억한 값을 반환해 줍니다.
useMemo는 특정 값을 메모이제이션 해, 인자로 전달받는 dependencies 변하지 않는 이상 리렌더링하지 않고, 그 값을 반환합니다
useCallback 은 특정 함수를 메모이제이션해, 인자로 전달받는 dependencies가 변하지 않는 이상 리렌더링 하지 않고 그 함수를 반환하게 됩니다.
이들을 잘 적용하면 리렌더링을 할 필요가 없어지기 때문에 최적화되어 더 나은 성능을 기대할 수 있게 됩니다.
React 컴포넌트 생명주기에 대해서 설명해주세요.
생명주기란 컴포넌트가 생성되고 사용되고 소멸될 때 까지 과정을 말한다.
컴포넌트는 마운팅 업데이트 언마운팅의 과정을 거치는데, 마운팅은 컴포넌트가 가상 돔에 삽입되는 과정을 의미하고, 업데이트는 마운팅 된 컴포넌트가 prop이나 state가 변경될 때, 다시 리렌더링을 거치는 과정이다. 또 언마운팅은 모든 일을 마친 컴포넌트가 DOM에서 무사 잘 제거 될 수 있도록 하는 과정이다.