-
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주차] 이가빈 미션 제출합니다. #4
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.
직관적인 UI가 인상깊은 것 같습니다! 코드 구조가 저랑 유사해서 더 재밌게 코드리뷰 할 수 있었던 것 같아요. 과제하시느라 수고많으셨습니다! 많이 배워가겠습니다!! 👍 😁
justify-content: center; | ||
&:hover { | ||
border-radius: 15%; | ||
border: 1px solid #91d1ff; |
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.
hover했을 때 border의 크기가 추가되어 캘린더에서 약간의 움직임이 있는 것 같습니다! border의 활용도 좋다고 생각되지만, 스타일이 변하지 않는 걸 신경쓸 때는 box-shadow: inset을 활용하는 방안도 괜찮았던 것 같아요!
useEffect(() => { | ||
loadTodoList(currentDate); | ||
}, [currentDate]); | ||
|
||
const loadTodoList = (date) => { | ||
const todos = getTodoList(date); | ||
setTodoList(todos); | ||
}; |
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.
함수의 선언 위치관련해서 기존 자바스크립트에서는 화살표 함수로 선언된 함수가 호출된 곳보다 아래에서 선언되었을 때 호이스팅 문제로 에러가 발생할 가능성이 있습니다. 다만 useEffect 특성상 렌더링 이후에 useEffect가 실행되기 때문에 이미 모든 함수가 렌더링된 후에 useEffect 안에서 함수가 호출되어 에러가 발생하지 않습니다. 코드상 에러가 발생하지는 않으나 호출 전에 선언하는 것도 코드 가독성같은 측면에서 좋을 수 있을 것 같습니다!
저도 습관을 들이는 중입니다!
<TodoList> | ||
{todoList.map((todo, index) => ( | ||
<TodoItem | ||
key={index} |
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.
index도 좋지만, todo에 고유한 id값을 넣어서 관리한다면 여러 방면으로 활용할 수 있는 범위가 넓어질 수 있을 것 같아요!
export const formatDate = (date) => { | ||
const year = date.getFullYear(); | ||
const month = (date.getMonth() + 1).toString(); | ||
const day = date.getDate().toString(); | ||
return `${year}년 ${month}월 ${day}일`; | ||
}; |
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.
날짜 포맷 맞춰주는 유틸함수 사용한 거 재사용성이 정말 좋은 것 같습니다!
padding-right: 20px; | ||
|
||
// 스크롤바 스타일링 | ||
scrollbar-width: thin; |
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.
오 저도 처음 알았네요...!
} | ||
`; | ||
|
||
const StyledCalendar = styled(BasicCalendar)` |
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.
styled에 BasicCalendar가 있는 것 배워가겠습니다!
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.
오 저도 BasicCalendar 배워갑니다👍🏻👍🏻
const Container = styled.div` | ||
display: flex; | ||
justify-content: space-between; | ||
align-items: center; | ||
min-width: 800px; | ||
max-width: 1000px; | ||
margin: 0 auto; | ||
` |
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.
가로 세로로 overflow가 조금 있는 것 같습니다! 여러 부분에서 미세하게 달라진 스타일들이 영향을 준 것 같아서 overflow가 사라진다면 더 깔끔한 투두리스트가 될 것 같아요!
const moveDate = (days) => { | ||
const newDate = new Date(currentDate); | ||
newDate.setDate(newDate.getDate() + days); | ||
setCurrentDate(newDate); |
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 newDate = addDays(currentDate, days);
라는 유틸함수도 있는 것 같아요! Date 메서드들 중에서 유용한 게 많은 것 같아서 같이 활용해보면 좋을 것 같습니다!
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.filter((todo) => todo !== changedTodo) | ||
// 완료 토글 처리 | ||
: todoList.map((todo) => | ||
todo === changedTodo ? { ...todo, completed: !todo.completed } : 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.
객체를 비교할 때 참조 동등성(===)을 사용하면 예상치 못한 결과가 발생할 수 있는데, 특히 데이터가 로컬 스토리지에서 불러와지는 경우 객체의 참조가 변경될 수 있다고 합니다.
id를 이용해서 고유한 값으로 객체를 비교하는 것도 예상치 못한 오류를 방지하는 방법일 것 같습니다!
padding-right: 20px; | ||
|
||
// 스크롤바 스타일링 | ||
scrollbar-width: thin; |
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 default TodoItem; | ||
|
||
const TodoItemContainer = styled.li` |
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이 ul 또는 ol 내부에서 사용되지 않는다면, 시맨틱을 위해 div로 변경하면 좋을 것 같아요
// 투두 완료 토글 | ||
const toggleTodoComplete = () => { | ||
onTodoChange(date, todo, false); | ||
}; | ||
|
||
// 투두 삭제 | ||
const deleteTodo = () => { | ||
onTodoChange(date, todo, true); | ||
}; |
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.
이 두함수는 공통요소가 2개이고, boolean값만 다른데, 하나의 함수로 통일해서 리팩토링하면 좋을 것 같아요
// 투두 완료 토글 | |
const toggleTodoComplete = () => { | |
onTodoChange(date, todo, false); | |
}; | |
// 투두 삭제 | |
const deleteTodo = () => { | |
onTodoChange(date, todo, true); | |
}; | |
const handleAction = (action) => { | |
onTodoChange({ date, todo, action }); | |
}; |
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.
코드리뷰 맡게된 김류웝입니다~!
전반적으로 ui가 너무 이뻤고, 캘린더 구현하신 부분 너무 유용할 거 같아요~!
코드리뷰를 다 하지 못해서 오늘 스터디 끝나고 추가적으로 하겠습니다!💗
} | ||
`; | ||
|
||
const StyledCalendar = styled(BasicCalendar)` |
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.
오 저도 BasicCalendar 배워갑니다👍🏻👍🏻
// 남은 할 일 개수 | ||
const updateLeftNum = () => { | ||
const leftNum = todoList.filter((todo) => !todo.completed).length; | ||
return `할 일 ${leftNum}개`; | ||
}; | ||
|
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 leftTodoText = useMemo(() => {
const leftNum = todoList.filter((todo) => !todo.completed).length;
return `할 일 ${leftNum}개`;
}, [todoList]);
useMemo를 사용해 todoList가 변경될 때만 계산하도록 하는 방법도 좋을 거 같습니다~!
배우고 느낀 점
많은 시간을 투자한 부분
배포 링크
https://react-todo-20th-gamma.vercel.app/
Key Question
1. Virtual-DOM은 무엇이고, 이를 사용함으로서 얻는 이점은 무엇인가요?
Virtual-DOM
✅비교(Diffing)
✅패치(Patching)
2. React.memo(), useMemo(), useCallback() 함수로 진행할 수 있는 리액트 렌더링 최적화에 대해 설명해주세요. 다른 방식이 있다면 이에 대한 소개도 좋습니다.
렌더링(rendering)
컴포넌트의 리렌더링 조건
메모이제이션(Memoization)
React.memo()
useMemo()
useCallback()
3. React 컴포넌트 생명주기에 대해서 설명해주세요.
React Component Life Cycle
마운트(Mounting)
1. constructor()
2. static getDerivedStateFromProps()
3. render()
4. componentDidMount()
업데이트(Updating)
1. static getDerivedStateFromProps()
2. shouldComponentUpdate()
3. render()
4. getSnapshotBeforeUpdate()
5. componentDidUpdate()
언마운트(Unmounting)
componentWillUnmount()