Skip to content

Conversation

@minji9029
Copy link
Collaborator

@minji9029 minji9029 commented May 28, 2025

체크리스트 [기본]
중고마켓

  • 중고마켓 페이지 주소는 “/items” 입니다.

  • 페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.

  • 상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.

  • 상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.

  • '상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )

  • 전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.

  • 중고마켓 반응형
    베스트 상품
    Desktop : 4개 보이기
    Tablet : 2개 보이기
    Mobile : 1개 보이기
    전체 상품
    Desktop : 12개 보이기
    Tablet : 6개 보이기
    Mobile : 4개 보이기

체크리스트 [심화]

  • 페이지 네이션 기능을 구현합니다.
    screencapture-localhost-5173-items-2025-06-18-12_39_02

멘토에게

css 분리가 필요한데
styled-components로 작성하는 방식이랑
css파일로 따로 분리하는 방식을 같이 사용해도 괜찮을까요?

스타일은 일정한게 좋을 거같긴한데 css가 너무 짧을때는 styled-components가 가독성이 좋아보여서 고민스럽네용

@minji9029 minji9029 marked this pull request as draft May 28, 2025 10:11
@minji9029 minji9029 requested a review from addiescode-sj May 28, 2025 10:11
@minji9029 minji9029 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label May 28, 2025
@minji9029 minji9029 marked this pull request as ready for review June 2, 2025 11:36
Copy link
Collaborator

@addiescode-sj addiescode-sj left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!
첫 리액트 미션인데 컴포넌트도 잘 분리하시고 어렵지않게 사용하시네요.
제가 코멘트 드린 것보고 리팩토링도 경험해보시면 좋을거같아요!

주요 리뷰 포인트

  • 유지보수를 고려한 개발
  • 코드 중복 제거

throw new Error("상품을 불러오는데 실패했습니다");
}
const body = await response.json();
console.log("🔥 API 응답:", body);
Copy link
Collaborator

Choose a reason for hiding this comment

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

불필요한 로그는 PR 올리실때 지워주세요! :)

Comment on lines 59 to 103
input {
width: 470px;
height: 42px;
background-color: #f3f4f6;
border: 1px solid #f3f4f6;
border-radius: 12px;
padding: 9px 20px 0 16px;
font-size: 16px;
font-weight: 400;
color: #9ca3af;
}

button {
width: auto;
height: 42px;
background-color: #3692ff;
border: 1px solid #3692ff;
border-radius: 8px;
color: #f3f4f6;
font-size: 16px;
font-weight: 600;
padding: 8px 23px;

&:hover {
background-color: #1e5bb8;
}
}
select {
width: 130px;
height: 42px;
background-color: #ffffff;
border: 1px solid #e5e7eb;
border-radius: 12px;
padding: 12px 20px;
font-size: 16px;
font-weight: 400;
color: #1f2937;

option {
text-align: center;
color: #1f2937;
font-size: 16px;
font-weight: 400;
}
}
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 +121 to +122
const [error, setError] = useState(null);
const [isLoading, setIsLoading] = useState(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

비동기 data fetching 작업이 있을때마다 로딩과 에러 처리가 반복될텐데,
코드 중복 제거를 위해 data fetching을 위한 커스텀 훅을 만들어보는건 어떨까요? :)

Comment on lines 175 to 178
onChange={(e) => {
if (e.target.value === "createdAt") handleNewestClick();
if (e.target.value === "favoriteCount") handleFavoriteClick();
}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 인라인 형식으로 조건문을 쓰며 함수를 따로 쪼갤 필요없이, 핸들러 하나로 통합해 switch case를 써주는게 나을것같네요 :)

e.g

  const handleOrderChange = (e) => {
    switch (e.target.value) {
      case "createdAt":
        onSortChange("createdAt");
        break;
      case "favoriteCount":
        onSortChange("favoriteCount");
        break;
      default:
        break;
    }
  };

Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서 한번 더 나아가자면 이벤트 핸들링의 일관성을 위해 옵션 객체를 컴포넌트 외부에서 만들어주고, value와 label을 활용한다면 이벤트 핸들러에서는 이것만 해주면 되겠죠?

const OPTIONS = [
  { value: "createdAt", label: "최신순" },
  { value: "favoriteCount", label: "좋아요순" },
];

// map을 사용해 <option />에 value, label 활용하기
<select onChange={handleOrderChange}>
  {OPTIONS.map(({ value, label }) => (
    <option key={value} value={value}>
      {label}
    </option>
  ))}
</select>

// handleOrderChange 함수
const handleOrderChange = (e) => {
  onSortChange(e.target.value);
};

Comment on lines +50 to +55
<div>
<StyledProductImage src={item.images[0]} alt={item.name} />
<ProductName>{item.name}</ProductName>
<ProductPrice>{item.price.toLocaleString()}원</ProductPrice>
<ProductFavorite>♡ {item.favoriteCount}</ProductFavorite>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT: 보통 Block 레벨 엘리먼트를 표현해야할때는 div, 불필요한 DOM 노드를 추가하지 않고 여러 요소를 그룹화할 때는 Fragment (<>{...}</>)를 활용하는 편입니다. 지금과 같은 경우는 Card 아이템을 나타내는 블록 요소에 가깝기때문에 div를 쓰는게 맞는 선택인데, 단순 그룹화 용도라면 Fragment를 쓸수도 있다는점 참고해보세요! :)

Comment on lines 31 to 37
<PaginationButton>&lt;</PaginationButton>
<PaginationButton>1</PaginationButton>
<PaginationButton>2</PaginationButton>
<PaginationButton>3</PaginationButton>
<PaginationButton>4</PaginationButton>
<PaginationButton>5</PaginationButton>
<PaginationButton>&gt;</PaginationButton>
Copy link
Collaborator

Choose a reason for hiding this comment

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

아직 페이지네이션 기능을 구현하진 않으셨군요!

UI

  • 한페이지당 아이템 개수
  • 스타일링 및 데이터 표시

로직

  • Prev & Next 동작
  • 현재 페이지 계산
  • 이전 페이지, 다음 페이지 계산
  • 예외 처리 (e.g 음수 인덱스 방지, 최대 페이지 수 이상 못 넘어가게 방지)

페이지네이션의 경우 여러 컴포넌트에서 반복되어 사용되다보니, 재사용이 필수적인데요!
재사용 가능한 UI는 컴포넌트로, 로직은 커스텀 훅으로 쪼개볼까요? :)

@minji9029 minji9029 closed this Jun 18, 2025
@minji9029 minji9029 reopened this Jun 18, 2025
@minji9029 minji9029 requested a review from addiescode-sj June 18, 2025 03:57
@minji9029 minji9029 closed this Jun 18, 2025
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.

4 participants