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

[하신혜] Sprint 5 #94

Conversation

Aventurine91
Copy link
Collaborator

@Aventurine91 Aventurine91 commented Oct 27, 2024

요구사항

기본 요구사항

공통

  • Github에 스프린트 미션 PR을 만들어 주세요.
  • React를 사용해 진행합니다.
    중고마켓 페이지
  • PC, Tablet, Mobile 디자인에 해당하는 중고마켓 페이지를 만들어 주세요.
  • 중고마켓 페이지 url path는 별도로 설정하지 않고, ‘/’에 보이도록 합니다.
  • 상품 데이터는 https://panda-market-api.vercel.app/docs/에 명세된 GET 메소드 “/products” 를 사용해주세요.
  • 상단 네비게이션 바, 푸터는 랜딩 페이지와 동일한 스타일과 규칙으로 만들어주세요.
  • 상품 데이터는 https://panda-market-api.vercel.app/docs/에 명세된 GET 메소드 “/products” 를 활용해주세요.
  • 상품 목록 페이지네이션 기능을 구현합니다.
  • 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 구현하세요.
  • 상품 목록 검색 기능을 구현합니다.
  • 베스트 상품 데이터는 https://panda-market-api.vercel.app/docs/에 명세된 GET 메소드 “/products”의 정렬 기준 favorite을 사용해주세요.

심화 요구사항

공통

  • 커스텀 hook을 만들어 필요한 곳에 활용해 보세요.
    중고마켓 페이지

  • 중고 마켓의 카드 컴포넌트 반응형 기준은 다음과 같습니다.
    베스트 상품
    Desktop : 4열
    Tablet : 2열
    Mobile : 1열
    전체 상품
    Desktop : 5열
    Tablet : 3열
    Mobile : 2열

  • 반응형에 따른 페이지 네이션 기능을 구현합니다.

  • 반응형으로 보여지는 물품들의 개수를 다르게 설정할때 서버에 보내는 pageSize값을 적절하게 설정합니다.

멘토에게

-아직 미완성이긴한데 최선을 다했숩니다 흑흑.. 수정할 내용 말씀해주시면 수정해보겠습니다!
-멘토님 제가 푸쉬할때 모듈오류가 떳었는데 갑자기 왜이럴까용?.. React라서 그럴까여?..😫
다시 시도해보니 제대로 푸쉬되긴했습니다 ㅠㅠ

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@Aventurine91 Aventurine91 changed the base branch from main to react-하신혜 October 27, 2024 16:32
@Aventurine91 Aventurine91 requested a review from pers0n4 October 27, 2024 16:37
@Aventurine91 Aventurine91 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 완성은 아니지만 제출합니다... labels Oct 27, 2024
Copy link
Collaborator

@pers0n4 pers0n4 left a comment

Choose a reason for hiding this comment

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

  1. 커밋 단위를 '코드에 의미 있는 변화가 생기는 지점'을 기준으로 잘 게 나누어서 커밋할 수 있도록 노력해보시면 좋을 것 같습니다.

멘토님 제가 푸쉬할때 모듈오류가 떳었는데 갑자기 왜이럴까용?.. React라서 그럴까여?..😫
다시 시도해보니 제대로 푸쉬되긴했습니다 ㅠㅠ

PR 내용을 보니 서브 모듈이 하나 보이는데 그래서 그런 건가 싶기도 하네요. 질문 내용만으로는 어떤 문제인지 정확하게 파악하기 힘들어서 다음에는 에러 메시지라도 남겨두면 좋을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

세트로 사용되는 CSS 파일이랑 파일 네이밍 컨벤션이 다르네요. JS 파일도 BestProducts.js 쪽으로 바뀌는 게 맞을 것 같네요.

Comment on lines +31 to +35
<img src={product.images[0]} alt={product.name} /> {/* 이미지 URL 수정 */}
<div className="product-info">
<h3>{product.name}</h3> {/* 상품 이름 */}
<p>{product.price.toLocaleString()}원</p> {/* 가격 */}
<p>♡ {product.favoriteCount}</p> {/* 좋아요 수 */}
Copy link
Collaborator

Choose a reason for hiding this comment

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

전체적으로 주석을 꼼꼼히 남겨두는 건 좋은 습관이라고 생각하는데, 너무 당연한 주석은 생략해도 괜찮을 것 같습니다.

.then(response => response.json())
.then(data => {
console.log('API 응답 데이터:', data); // API 응답 구조 확인
setProducts(data.list.slice(2, 6)); // 4개만 설정
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 주석은 괜찮을 것 같아요.

fetch('https://panda-market-api.vercel.app/products?offset=2&limit=4')
.then(response => response.json())
.then(data => {
console.log('API 응답 데이터:', data); // API 응답 구조 확인
Copy link
Collaborator

Choose a reason for hiding this comment

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

실제 배포 시에는 이렇게 사용자에게 노출되는 로깅 메시지는 제거하셔야 합니다.

const itemsPerPage = 5;

useEffect(() => {
fetch('https://panda-market-api.vercel.app/products?offset=0&limit=50')
Copy link
Collaborator

Choose a reason for hiding this comment

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

api 주소 값은 재사용되면서 환경에 따라 바뀔 수 있는 값이기에 우선은 상수로 정의해서 동일한 API 주소에 대해선 같이 사용할 수 있게 하면 좋을 것 같습니다.

const [currentPage, setCurrentPage] = useState(1);
const [searchTerm, setSearchTerm] = useState(''); // 검색어 상태
const [sortOption, setSortOption] = useState('latest'); // 정렬 옵션 상태
const itemsPerPage = 5;
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Comment on lines +15 to +22
.then(data => {
setProducts(data.list);
setLoading(false);
})
.catch(error => {
console.error('Error fetching best products:', error);
setLoading(false);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

then catch에서 사용하고 있는 setLoading은 finally로 옮기면 딱 적절하지 않을까 싶어요

Comment on lines +31 to +35
if (sortOption === 'latest') {
return new Date(b.createdAt) - new Date(a.createdAt);
} else if (sortOption === 'favorite') {
return b.favoriteCount - a.favoriteCount;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

sortOption 같이 enum처럼 사용되는 값은 리터럴 스트링 값을 그대로 사용하기보다는 상수 객체를 만들어서 사용하면 좋을 것 같네요.

Comment on lines +76 to +85
currentProducts.map(product => (
<div key={product.id} className="product-list-card">
<img src={product.images[0]} alt={product.name} />
<div className="product-list-info">
<h3>{product.name}</h3>
<p>{product.price.toLocaleString()}원</p>
<p>♡ {product.favoriteCount}</p>
</div>
</div>
))
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 map을 통해 매핑되는 부분은 컴포넌트로 정의해봐도 좋을 것 같습니다.

@pers0n4 pers0n4 merged commit 4f2683d into codeit-sprint-fullstack:react-하신혜 Oct 31, 2024
1 check passed
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