[조아라] sprint7 #80
[조아라] sprint7 #80
Conversation
…nd/21-Sprint-Mission into Basic-joara-sprint4
…ildfix: correct filename casing in ProductListItem module for Netlify build
humonnom
left a comment
humonnom
left a comment
There was a problem hiding this comment.
우선 구현은 높은 구현도로 잘 하셨습니다 👍
코드도 깔끔하고 컴포넌트 분리가 적절하게 잘 되어있습니다.
그런데 스타일이 모두 react-project/src/components/ProductDetail.jsx 파일에서 분리되어 있는데, 이 부분에 대해서 한가지 말씀드리고 싶은 것이 있네요.
지금처럼 JSX 로직과 스타일 로직이 분리되면 각자 집중해서 리뷰하거나 테스트하기 쉬울 수 있습니다.
그런데 파일을 계속 왔다갔다 해야해서 작성하거나 수정할때는 번거로울 수도 있어요.(탐색 비용 상승)
따라서 지금의 상태를 유지할지, 내부에서 선언을 할지 고민을 한번 해보시면 좋을 것 같네요.
저는 특정 컴포넌트에서만 쓰이는 스타일은 그 컴포넌트 파일에 함께 두는 게 읽기/리팩토링에 유리해서 하나의 파일로 관리하는 걸 선호하긴 합니다. 컴포넌트의 구현을 한 눈에 볼 수 있다는 장점이 있죠.
나머지 코드 레벨에서 사소한 리뷰를 몇가지 남겨두었으니 확인바랍니다 :)
- 전달해주신 배포 링크에 접속이 되지 않아서 이번에는 제가 아라님 레포를 클론해서 결과물을 확인했는데요, 혹시 프리뷰가 내부용으로 사용되도록 설정되어있지는 않은지 확인한번 해주시면 좋을 것 같습니다!
|
|
||
| return ( | ||
| <div className="item itemDetails"> | ||
| <Container> |
| </DetailNicknameWrap> | ||
| </DetailOwnerWrapper> | ||
| <DetailFavoritButton> | ||
| <DetailFavoritImg src={likeIcon} alt="하트 아이콘" /> |
There was a problem hiding this comment.
(권장) 심화 요구사항에서 모든 버튼에 hover 넣어보라는 내용이 있는데, 다른 부분은 다 잘 하셨는데 하트 아이콘에도 호버 효과 들어가면 좋을 것 같네요
2026-01-05.1.29.59.mov
| function ProductDetail ({ item }) { | ||
| return ( | ||
| <DetailWrap> | ||
| <DetailImg src={item.images[0]} alt="제품 이미지" /> |
There was a problem hiding this comment.
(권장) 주 이미지 대체 텍스트를 상품명 기반으로 개선하면 좋을 것 같습니다.
| <DetailImg src={item.images[0]} alt="제품 이미지" /> | |
| <DetailImg src={item.images[0]} alt=`${item.name} 이미지` /> |
| <DetailInfoTitle>상품 태그</DetailInfoTitle> | ||
| <LocalTagList> | ||
| {item.tags.map((tag, index) => ( | ||
| <Tag key={index}>#{tag}</Tag> |
There was a problem hiding this comment.
| <Tag key={index}>#{tag}</Tag> | |
| <Tag key={`tag-${tag}-${index}`}>#{tag}</Tag> |
(권장) index는 배열 순서가 바뀌거나 중간 삽입/삭제 시 값이 바뀌어 동일 엘리먼트를 다른 것으로 오인할 수 있습니다. 따라서 항목 자체의 고유 식별자를 우선적으로 사용하는 습관을 들이는 것이 좋습니다.
tag 값의 경우 중복될 수 있기에(같은 태그 존재시), 고유한 id가 있다면 id를 사용하는 것이 가장 좋습니다. 현재 서버에서 보내주는 tag 데이터는 배열이고, 각 tag에 id가 없기 때문에 key={tag-${tag}-${index}} 이런식으로 쓰시면 됩니다.
| <InquiryFormTitle>문의하기</InquiryFormTitle> | ||
| <InquiryForm> | ||
| <InquiryFormTextarea onChange={handleChange} placeholder='개인정보를 공유 및 요청하거나, 명예 훼손, 무단 광고, 불법 정보 유포시 모니터링 후 삭제될 수 있으며, 이에 대한 민형사상 책임은 게시자에게 있습니다.' /> | ||
| <InquiryFormButton className="button defaultButton" disabled={content !== '' ? false : true}>등록</InquiryFormButton> |
There was a problem hiding this comment.
| <InquiryFormButton className="button defaultButton" disabled={content !== '' ? false : true}>등록</InquiryFormButton> | |
| <InquiryFormButton className="button defaultButton" disabled={!content}>등록</InquiryFormButton> |
(필수) 단축표현 추천, 삼항 연산자 없이 더 간단하게 쓸 수 있습니다.
컨텐츠가 있으면 false이기 때문에 !content로 쓰셔도 됩니다.
(참고) 공백만 입력된 경우도 막고 싶다면 trim까지 적용하시면 됩니다.
!content.trim()
| function getRelativeTime (updatedAt) { | ||
| const now = new Date(); | ||
| const updatedDate = new Date(updatedAt); | ||
| const seconds = Math.floor((now.getTime() - updatedDate.getTime()) / 1000); // 차이를 초 단위로 계산 |
|
안녕하세요 강사님, 상세한 리뷰 감사합니다! 현재는 스타일 코드들을 별도의 styles 파일로 분리해두었는데, 다만 이번 미션에서는, 이 두 가지 이유로 현재와 같은 구조를 선택하게 되었습니다. 또한 배포 링크 관련해서는, 배포 접근 문제로 번거로움을 드려 죄송합니다. |
3 participants
요구사항
주요 변경사항
이전 스프린트 미션 PR 피드백 반영 사항
주강사님에게
배포 링크
※ 댓글이 많은 상품 상세 페이지는 /items/3 경로로 확인하실 수 있습니다.