-
Notifications
You must be signed in to change notification settings - Fork 10
[2주차] 권동욱 미션 제출합니다. #10
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: main
Are you sure you want to change the base?
Conversation
default setting
initialize readme
modify favicon and title
define interface: todo, object type: todos by date
define action function types: add, toggle, and remove
load and save todos using local storage (Date <-> string)
generate id using crypto (uuid)
custom hook - state using useState, useEffect (to handle local storage)
custom hook - add, toggle, remove todo item using useCallback only use setState (<= use callback prev parameter)
custom hook - total and done count by date using useMemo (dependency = dateTodos) only use todo state
custom hook - state, actions, statistics
todo input, item, list and custom hooks (state, actions, stats)
calednar components (body, header) and custom hooks (state, action)
done count / total count if there is no todo, nothing is displayed
add calendar footer (= display selected date)
when all todos are deleted, do not display count
use theme and global style of styled components
prevent component props from being transmitted to DOM
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주차 과제하시느라 수고하셨습니다!
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.
저도 ico 파일로 했는데 괜히 반갑네요,, 다른 분들은 svg로 많이 하셨더라고요! 각각의 장단점이 있는 것 같으니 잘 찾아보고 맞는 방식으로 사용하면 좋을 것 같아요. 저는 모든 브라우저에서 완벽하게 지원된다는 점에서 ico 파일을 사용했습니다!
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.
저도 ico랑 svg 중에서 고민 많이 했었는데 해당 프로젝트에서는 좀 더 익숙한 ico 확장자를 사용했습니다! 찾아보니 svg가 더 확장성에 유리하다고 하여서 다음 번에는 svg 사용하는 방식도 고려해보고자 합니다!
<div> | ||
<ThemeProvider theme={theme}> | ||
<GlobalStyle /> | ||
<Home /> | ||
</ThemeProvider> | ||
</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></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.
그렇게 하면 더 깔끔한 코드가 될 것 같네요 감사합니다!
onKeyDown={(e) => { | ||
if (e.key === 'Enter') { | ||
handleClick(); |
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.
제가 영어나 숫자로만 테스트하다보니 이 부분을 고려하지 못했네요 ㅜㅜ 감사합니다!
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 LABELS = { | ||
placeholder: '할 일 추가', | ||
add: '추가', | ||
delete: '삭제', | ||
}; |
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.
placeholder나 버튼 내용은 상수로 빼는건 처음 보는데 오히려 코드 가독성이 떨어지는 것 같습니다ㅜㅜ
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.
paths를 활용해서 ../components 같은 방식 말고 @/components로도 import 할 수 있는데 알아보시면 좋을 것 같아요!
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 options: Intl.DateTimeFormatOptions = { | ||
year: 'numeric', | ||
month: 'long', | ||
day: 'numeric', | ||
weekday: 'long', | ||
}; |
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.
option이 상수니까 대문자로 OPTIONS라고 적어주면 좋을 것 같습니다!
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.
제가 알기로는 Intl.DateTimeFormat() 에서 options parameter가 소문자로 되어있어서 동일하게 소문자로 작성하였는데, 혹시 어떤 방식이 더 좋다고 생각하시나요?
제가 찾아본 refer는 다음과 같습니다!
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/DateTimeFormat#options
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleDateString
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.
사실 이 부분은 제가 바닐라 js로 투두리스트 구현할 때 코드리뷰 받았던 부분인데요, '해당 코드는 함수 바깥으로 FORMAT_OPTIONS 와 같은 상수로 빼 두고, 내부적으로 사용하는 것도 괜찮아요. 나중에 날짜 표기 정책이 바뀌었을 경우에 해당 함수를 전부 뜯어보지 않고 option만 변경하면 되니까요!' 라고 적어주셨고 저도 상수라고 생각해서 대문자를 썼는데, 또 문서를 보니까 헷갈리네요... 이 부분은 다른 분들 의견도 듣고 싶습니다
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.
개인의 정책에 따라 다를 것 같아요. 어찌되었던 코드, 협업에 있어 통일이 된다면 두 방법 모두 좋을 것 같습니다~
<List> | ||
{todos.map((todo) => ( | ||
<TodoItem | ||
key={todo.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.
오 다들 index로 하셨던데 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.
2주차 과제 수고하셨습니다 !!
<TodoInput | ||
selectedDateKey={calendarLogic.selectedDateKey} | ||
onAddTodo={dateTodos.addTodo} | ||
/> |
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 하나하나를 작성할때마다 인풋이 아래로 내려가는데, position: fixed로 화면 하단에 고정시키는 방법도 좋을 거 같습니다 !
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.
이 부분도 제가 좀 고민을 많이 했었는데요, 혹시 괜찮으시다면 의견 주시면 감사하겠습니다! 제가 생각한 방법이
- input -> todo list 순으로 배치
- todo list -> input 순으로 배치해서 유동적으로 (현재 방식)
- todo list -> input 순으로 배치하되 todo list 높이랑 input 위치 고정하기
세 가지였는데, 어떤 게 제일 보기 좋다고 생각하시나요??
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번이나 3번이 더 마음에 드는 거 같아요!
다만 강조하고 싶은 요소나 전체적인 디자인 스타일에 따라 달라질 수도 있을 것 같습니당
현재 디자인은 폰 사이즈처럼 중앙 정렬이 되어 있고 모바일에 특화된 느낌이 들어서 3번이 잘 어울린다고 생각했습니다 다만 가운데가 너무 비어 보일 수 있다는 점은 조금 걱정되긴 합니다 😅
return ( | ||
<HeaderContainer> | ||
<MonthText> | ||
{thisYear}. {thisMonth + 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.
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월 25일 눌렀을 때 todo list도 2월 25일로 바뀌는 거 말씀하신 걸까요 ??
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.
3월 캘린더에서 2월이나 4월 부분을 눌러도 할 일이 등록되는 거 같은데,
그렇다면 맨 위쪽 2025.03 부분이 선택한 날짜대로 2025.02나 2025.04로 변경되어야 할 거 같아서 말씀드렸어요 !! 정말 사소한 거긴 합니다,,,,ㅎㅎ
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.
아하 이것도 제가 참고한 앱에서 안 바뀌어서 그대로 사용한 거긴 한데, 한 번 고민해보면 좋을 것 같네요 감사합니다!
(저는 footer에 구체적인 날짜가 나오기 때문에 상단은 안 바뀌어도 괜찮을 거라고 생각했습니다!)
const Input = styled.input` | ||
flex: 1; | ||
font-size: ${({ theme }) => theme.fontSize.base}; | ||
border: 1px solid #ccc; | ||
border-radius: ${({ theme }) => theme.radius.small}; | ||
padding: ${({ theme }) => theme.spacing.sm}; | ||
`; |
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.
outline: none;
속성도 추가하면 input 필드에 마우스 포커스가 갔을 때 나타나는 테두리를 없앨 수 있습니다 !! 이건 제 취향이긴 합니다,,,ㅎㅎ
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.
저도 참고하고 갑니다...
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.
outline: none; 속성을 쓰면 키보드만으로 브라우저를 조작해야 하는 유저는 현재 초점을 알 수 없기 때문에 문제가 된다고 합니다! 아래 블로그처럼 :focus-visible 사용해도 좋을 것 같습니다!
https://velog.io/@ensun_p/outlinenone-%EB%A7%90%EA%B3%A0-focus-visible
const newDate = new Date(newYear, newMonth, 1); | ||
setSelectedDateKey(formatDateKey(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.
달을 옮길때 무조건 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.
아 이 부분은 제가 레퍼런스로 사용한 앱이 있어서 그 앱 따라했습니다 ..!
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 removeTodo: RemoveTodo = useCallback( | ||
(dateKey: string, id: string) => { | ||
setDateTodos((prev) => ({ | ||
...prev, | ||
[dateKey]: prev[dateKey]?.filter((todo) => todo.id !== id) || [], | ||
})); | ||
}, | ||
[setDateTodos], | ||
); |
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.
확실히 이렇게 하는 게 데이터 난잡해지지도 않고 더 좋을 것 같네요 감사합니다!
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.
removeDateTodos 함수는 localStorage.tsx파일에 같이 넣지 않는 이유가 있을까요? 같이 함수를 만들어서 둔다면 코드들 관리할 때 좋을 거 같아요!
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.
저는 localStorage 핸들링은 load/save만으로 기능들 다 구현할 수 있다고 생각해서 기본적인 함수만 작성하였는데, 말씀하신 것처럼 add/remove 정도는 추가해도 좋을 것 같습니다!
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.
저는 캘린더 라이브러리 사용해서, 캘린더를 만들생각을 못했는데 만들었다고 느꼈지 못할만큼 깔끔했습니다. 특히나, thead, tbody를 사용하면 데이터가 많은경우에 좋더라구요!! 이렇게 또하나 배워갑니다!!
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-component방식으로 사용안하고 이렇게 따로 뺀 이유가 있을까요?
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.
이거는 mayer's reset이라고 제가 인터넷에서 긁어온건데, 처음 프로젝트 생성했을 때 index.css 파일이 있어서 그냥 그대로 놔두고 내용만 초기화 파일 복붙했습니다 ..!
function useCalendarActions( | ||
state: UseCalendarStateReturn, | ||
): UseCalendarActionsReturn { |
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.
혹시 return type UseCalendarActionsReturn 에 대해 말씀해주시는 것일까요? 이 부분은 제가 좀 더 보기 편하게 + 나중에 다른 함수에서 import해서 사용하려고 이렇게 했습니다!
배포 링크입니다.
https://react-todo-21th-kwondu.vercel.app/
KQ 1. Virtual-DOM은 무엇이고, 이를 사용함으로서 얻는 이점은 무엇인가요?
실제 DOM의 가벼운 복사본.
변경 사항을 메모리에서 먼저 계산하고, 실제 DOM에는 최소한의 변경만 반영함.
x 빠른 렌더링
x 효율적인 업데이트
x 전체 DOM 재렌더링 방지
KQ 2. React.memo(), useMemo(), useCallback() 함수로 진행할 수 있는 리액트 렌더링 최적화에 대해 설명해주세요.
x React.memo()
컴포넌트를 memoization해서 props 변경 없으면 재렌더링 방지.
x useMemo()
연산 비용 큰 값을 캐싱. 의존값 변경 시에만 재계산.
x useCallback()
함수를 캐싱. 의존값 변경 없으면 기존 함수 참조 유지 → 불필요한 자식 렌더링 방지.
KQ 3. React 컴포넌트 생명주기에 대해서 설명해주세요.
x Mount: constructor → render → componentDidMount
x Update: shouldComponentUpdate → render → componentDidUpdate
x Unmount: componentWillUnmount