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

[고태호] Sprint6 #90

Merged

Conversation

kotaeho
Copy link
Collaborator

@kotaeho kotaeho commented Sep 14, 2024

요구사항

기본

  • 베스트 상품 좋아요 순으로 4개 정렬 배치
  • 전체상품 10개 2 x 5 그리드 레이아웃으로 배치
  • 최신순 좋아요순 정렬 기능 구현

심화

  • 페이지 네비게이션 구현
  • 반응형 구현

주요 변경사항

스크린샷

멘토에게

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

@kotaeho kotaeho changed the title React 고태호 [고태호] Sprint6 Sep 14, 2024
@kotaeho kotaeho added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Sep 14, 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.

태호님 이번주 스프린트 미션 고생하셨습니다~
아직 작업중이신 것 같아 구현하셨다고 적어주신 것 위주로 코드리뷰 드렸습니다.

}) => {
return (
<Main>
<MainH1>베스트 상품</MainH1>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
클래스명에 태그명을 넣게 되면 나중에 태그를 바꾸게 될 때 클래스명도 같이 바꿔야합니다.
가능하면 MainTitle 같은 식으로 태그와 연관없는 이름을 추천드립니다.

setSortOrder,
activePage,
setActivePage,
totalpage,
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
totalpage,
totalPage,

return (
<Main>
<MainH1>베스트 상품</MainH1>
<BestProducts bestProducts={bestProducts} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
좋아요순으로 정렬된 상품을 부모에서 필요한 이유가 있을까요?
부모에서 해당 데이터가 필요한 것이 아니라면 BestProducts 에서 데이터를 불러오시는걸 추천드립니다.

<EntireHeader>
<MainH1>전체상품</MainH1>
<EntireDiv>
<SearchInput type="text" placeholder="검색할 상품을 입력해주세요" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
해당 input에 값이 입력되었을때 해당 값 기반으로 products 를 다시 호출하는 로직을 추가해주시면 좋겠네요.

`;

const Pagination = ({ activePage, setActivePage, totalpage }) => {
const [pagebuttons, setPagebuttons] = useState([1, 2, 3, 4, 5]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:

Suggested change
const [pagebuttons, setPagebuttons] = useState([1, 2, 3, 4, 5]);
const [pageButtons, setPageButtons] = useState([1, 2, 3, 4, 5]);

{pagebuttons.map((number) => (
<button
<PaginationButton
key={number}
onClick={() => handlePageClick(number)}
style={getButtonStyle(number)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
styledComponent를 사용하시니 해당 라이브러리에서 props를 넘겨서 조건부 렌더링하는 방식으로 작성해주세요.

https://styled-components.com/docs/basics#passed-props

<div className="pagination">
<button>&lt;</button>
<PaginationContainer>
<PaginationButton onClick={() => handlePreviousClick()} type="button">
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
해당 함수에 arg가 필요없다면 아래처럼 사용해주세요.

Suggested change
<PaginationButton onClick={() => handlePreviousClick()} type="button">
<PaginationButton onClick={handlePreviousClick} type="button">

Comment on lines +51 to +53
if (activePage === 1) {
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
버튼을 클릭했을때 첫번째 페이지일때 함수를 실행하지 않는 것도 좋지만, activePage가 1일때 prev button에 disabled 속성을 주어서 사용자가 해당 버튼이 동작하지 않는다는 것을 알려주는 것이 사용자 경험 측면에서 더 좋을 것 같습니다.

<Nav>
<NavLink to={"/"}>자유게시판</NavLink>
<NavLink to={"/additem"}>중고마켓</NavLink>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
해당 컴포넌트의 to 속성과 동일한 url일때 fontColor가 디자인처럼 파란색이 될 수 있게 처리해주시면 좋겠습니다.

Comment on lines +23 to +26
const data = await fetchProducts(1, "favorite", pageSize);
const totalpage = Math.ceil(data.totalCount / 10);
setTotalpage(totalpage);
setBestProducts(data.list);
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
const data = await fetchProducts(1, "favorite", pageSize);
const totalpage = Math.ceil(data.totalCount / 10);
setTotalpage(totalpage);
setBestProducts(data.list);
const {totalCount, list} = await fetchProducts(1, "favorite", pageSize);
const totalpage = Math.ceil(totalCount / 10);
setTotalpage(totalpage);
setBestProducts(list);

@GANGYIKIM GANGYIKIM merged commit ee23197 into codeit-bootcamp-frontend:React-고태호 Sep 19, 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