-
Notifications
You must be signed in to change notification settings - Fork 39
[배수민] sprint10 #248
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
[배수민] sprint10 #248
The head ref may contain hidden characters: "Next-\uBC30\uC218\uBBFC-sprint10"
Conversation
| {editMode ? ( | ||
| <input | ||
| value={detailData.name} | ||
| onChange={handleNameChange} | ||
| className='focus:outline-none' | ||
| style={{ width: `${detailData.name.length + 1.4 || 1}ch` }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter') { | ||
| setEditMode(false); | ||
| } | ||
| }} | ||
| /> | ||
| ) : ( | ||
| <span className='truncate underline'>{detailData.name}</span> | ||
| )} |
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.
CheckListDetail 컴포넌트에서 체크 리스트 이름 수정 시 length를 통해 길이를 맞추려고 하였는데, 더 좋은 방법이 있을까요? 읽기 모드일때는 span으로 보여주고, 수정 모드일때는 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.
고정 너비를 가진 컨테이너로 감싸서 input과 span이 동일한 공간을 차지하도록해주면 해결될것같네요! :) 너비를 고정하기 애매하다면, flex-1 min-w-0을 사용해 컨테이너가 남은 공간을 모두 차지하게 할 수 있어요.
addiescode-sj
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.
수고하셨습니다!
깔끔하게 잘 작성하셨네요 👍
크리티컬하게 고치거나 해야하는 피드백은 없고, 설계 위주로 리뷰 드려봤어요!
주요 리뷰 포인트
- 본문 내 data fetching 로직 분리
- 리액트 쿼리를 & 별도 상태 관리에 대한 피드백
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.
data fetching 로직이 길어지는데 따로 로직만 분리해 커스텀 훅을 만들어 관리해보는건 어떨까요?
| }, | ||
| }); | ||
|
|
||
| const { detailData, setDetailData } = useItemStore(); |
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(() => { | ||
| if (data) { | ||
| const { name, memo, imageUrl, isCompleted } = data; | ||
| setDetailData({ name, memo, imageUrl, isCompleted }); | ||
| } | ||
| }, [data, setDetailData]); |
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.
별도의 상태관리가 필요하지않다면 이 effect도 없앨 수 있을 것 같아요 :)
|
|
||
| interface ItemPageProps { | ||
| params: Promise<{ itemId: string }>; | ||
| searchParams?: Promise<{ [key: string]: string | string[] | undefined }>; |
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의 경우 string이 아닌 number타입으로 들어가는 경우도 있을까요?
| {editMode ? ( | ||
| <input | ||
| value={detailData.name} | ||
| onChange={handleNameChange} | ||
| className='focus:outline-none' | ||
| style={{ width: `${detailData.name.length + 1.4 || 1}ch` }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter') { | ||
| setEditMode(false); | ||
| } | ||
| }} | ||
| /> | ||
| ) : ( | ||
| <span className='truncate underline'>{detailData.name}</span> | ||
| )} |
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과 span이 동일한 공간을 차지하도록해주면 해결될것같네요! :) 너비를 고정하기 애매하다면, flex-1 min-w-0을 사용해 컨테이너가 남은 공간을 모두 차지하게 할 수 있어요.
질문에 대한 답변
본문 내에 답변 드렸습니다! |
요구사항
기본
할 일 수정
할 일 삭제
주요 변경사항
스크린샷
배포 사이트 링크
https://summerdev-sprint-mission.vercel.app/
멘토에게