Skip to content
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

[김창희] sprint7 #103

Conversation

changhui-chan
Copy link
Collaborator

요구사항

기본

상품 상세

  • 상품 상세 페이지 주소는 “/items/{productId}” 입니다.

  • response 로 받은 아래의 데이터로 화면을 구현합니다.
    => favoriteCount : 하트 개수
    => images : 상품 이미지
    => tags : 상품태그
    => name : 상품 이름
    => description : 상품 설명

  • 목록으로 돌아가기 버튼을 클릭하면 중고마켓 페이지 주소인 “/items” 으로 이동합니다

상품 문의 댓글

  • 문의하기에 내용을 입력하면 등록 버튼의 색상은 “3692FF”로 변합니다.
  • response 로 받은 아래의 데이터로 화면을 구현합니다
    => image : 작성자 이미지
    => nickname : 작성자 닉네임
    => content : 작성자가 남긴 문구
    => description : 상품 설명
    => updatedAt : 문의글 마지막 업데이트 시간

주요 변경사항

  • 스프린트 7 미션 진행

배포 사이트

https://cheerful-salamander-19dcbf.netlify.app/

멘토에게

  • 저번 코드리뷰 내용은 반영하지 못했습니다

@changhui-chan changhui-chan changed the base branch from main to React-김창희 September 21, 2024 14:48
@changhui-chan changhui-chan added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Sep 21, 2024
Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

창희님 이번 스프린트 미션하시느라 고생하셨습니다.
코드리뷰는 조언일 뿐이니 원하시는 방향으로 작업하시면 좋겠습니다~

  • 이번에 상품 상세 페이지를 구현해주셨는데, products/{productId} api를 확인해보니 해당 데이터에 isFavorite 값이 있습니다. 제 생각에는 items 페이지에서는 좋아요 버튼 클릭이 안되게 하고(해당 데이터 값 -products api-에서는 isFavorite`가 없기 때문에) 상품 상세 페이지에서 해당 데이터 기반으로 좋아요 버튼 로직을 구현하시면 더 좋을 것 같습니다.


return (
<div className={styles['container']}>
<Link to='/items'><button className={styles['back-button']}>목록으로 돌아가기<Back /></button></Link>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
link 태그안에 기능없는 button 태그를 사용하시기 보다 link태그를 원하시는 디자인으로 스타일링해서 사용하시길 추천드립니다.
의미없는 button, link태그는 가독성에 좋지 않습니다.

Suggested change
<Link to='/items'><button className={styles['back-button']}>목록으로 돌아가기<Back /></button></Link>
<Link to='/items' className={styles['back-button']}>목록으로 돌아가기<Back /></Link>

Comment on lines +5 to +7
<div>
<img className={styles['container']} src={productImage} alt="상품 이미지" />
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
div 가 아무런 역할이 없는 것 같은데 필요한가요? 만약 불필요한 태그라면 없는 것이 더 좋겠습니다.


const Line = ({marginTop = 0, marginBottom = 0}) => {
return (
<hr style={{marginTop:marginTop, marginBottom: marginBottom}} className={styles['line']} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:

Suggested change
<hr style={{marginTop:marginTop, marginBottom: marginBottom}} className={styles['line']} />
<hr style={{marginTop, marginBottom}} className={styles['line']} />

const [commentCount, setCommentCount] = useState();
const [commentData, setCommentData] = useState([]);
const [createAt, setCreateAt] = useState();
const [updateAt, setUpdateAt] = useState('2024. 01. 02');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
default value인 2024.01.12 가 어떤 의미일까요? 만약 고정값이 아니라면 createAt처럼 초기값을 undefined하시는게 더 좋지 않을까합니다.

<span key={index}className={styles['product-tag']}>#{tag}</span>
))}
</div>
{productTags.length === 0 && <span className={styles['empty-tag']}></span>}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3:
아래 코드도 동일동작합니다.

Suggested change
{productTags.length === 0 && <span className={styles['empty-tag']}></span>}
{!productTags.length && <span className={styles['empty-tag']}></span>}

Comment on lines +45 to +50
setProductName(product.name);
setProductPrice(product.price);
setProductDescription(product.description);
setProductImage(product.images);
setProductTags(product.tags);
setProductFavoriteCount(product.favoriteCount);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
각 데이터의 항목을 다 state로 쪼개 저장하니 코드를 파악하기 어려운 것 같습니다.
하나의 state로 관리하시기를 추천드려요.

Suggested change
setProductName(product.name);
setProductPrice(product.price);
setProductDescription(product.description);
setProductImage(product.images);
setProductTags(product.tags);
setProductFavoriteCount(product.favoriteCount);
setProductData(product);

Comment on lines +86 to +104
{
commentData && commentData.map((user, index) => (
<>
<InquireItem
key={index}
content={user.content}
userNickname={user.writer.nickname}
userImage={user.writer.image}
updateAt={user.updatedAt}
commentData={commentData}
setCommentData={setCommentData}
/>
<Line marginTop={12} marginBottom={24}/>
</>
))
}
{
commentData.length <= 0 && <div className={styles['no-inquire']}><NoInquire />아직 문의가 없어요</div>
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
해당 페이지에서 comment부분과 상품 데이터 부분을 분리하면 위의 로직들도 분리되어서 더 깔끔할 것 같습니다.

Suggested change
{
commentData && commentData.map((user, index) => (
<>
<InquireItem
key={index}
content={user.content}
userNickname={user.writer.nickname}
userImage={user.writer.image}
updateAt={user.updatedAt}
commentData={commentData}
setCommentData={setCommentData}
/>
<Line marginTop={12} marginBottom={24}/>
</>
))
}
{
commentData.length <= 0 && <div className={styles['no-inquire']}><NoInquire />아직 문의가 없어요</div>
}
<ProductComment /> // 컴포넌트 분리하며 관련 데이터 호출 함수도 분리

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

만약 dataFetch를 page에서만 하기로 규칙을 세우신 거라면 코멘트와 상품 관련 부분을 나눠 커스텀 훅으로 분리하시면 더 깔금할 것 같습니다.

useEffect(() => {
const asyncFunction = async () => {
const response = await fetchData(`${API_USER_COMMENTS}/${id}/comments`, {productId: id, limit: 3});
setCommentCount(response.data.list.length);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
따로 state로 빼지않고 commentData.length로 판별가능한 사항이라 없으면 더 좋을 것 같네요~

Suggested change
setCommentCount(response.data.list.length);

@GANGYIKIM GANGYIKIM merged commit c6905e6 into codeit-bootcamp-frontend:React-김창희 Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants