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 #76

Merged

Conversation

kotaeho
Copy link
Collaborator

@kotaeho kotaeho commented Sep 6, 2024

요구사항

기본

  • 전체 상품에서 드롭다운으로 "최신 순" 또는 "좋아요 순" 을 선택해서 정렬을 할 수 있습니다.
  • '상품 등록하기' 버튼을 누르면" /additem" 로 이동합니다 (빈페이지)
  • 중고마켓 페이지 주소는 "/items" 입니다.

심화

주요 변경사항

스크린샷

image

멘토에게

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

@kotaeho kotaeho requested a review from GANGYIKIM September 6, 2024 14:18
@kotaeho kotaeho added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Sep 6, 2024
@kotaeho kotaeho changed the title 고태호[Sprint 5] [고태호] Sprint 5 Sep 7, 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.

태호님 첫 react 스프린트 미션 고생하셨습니다!
처음 배우는 프레임워크로 작업하시는 것이 결코 쉽지는 않으셨겟지만
이렇게 작업도 잘 하시고, 마감도 잘 지키셨다니 대단합니다.
다음 스프린트 미션도 화이팅입니다~


  • 중복되는 사항에 대해서는 한번만 리뷰를 답니다. 원하실 때 비슷한 사항들을 찾아서 같이 바꿔보세요~
  • 단복수에 대해 알 수 있도록 네이밍해주신 것이 좋았습니다.
  • 컴포넌트를 분리하신것이 좋았습니다. 다만 해당 컴포넌트에서만 사용되는 로직이라면 해당 컴포넌트 내부로 옮기시는 것을 추천드립니다. ex. BestProducts 컴포넌트
  • axios 를 쓰셔서 데이터 페치를 잘 수행하셨습니다. 다만 해당 API들의 param값에 따라 더 편하게 원하시는 데이터를 가지고 올 수 있으니 swagger로 테스트해보시면서 어떤 param이 있는지 확인해보시면 더 로직이 간결해질 것 같습니다.

}

body {
font-family: 'Pretendard'
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
폰트 패밀리는 Pretendard로 설정해주셨습니다만 해당 폰트를 import하는 부분은 어디에 있을까요?
제가 봤을때는 font 적용이 되지 않고 있는 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
코드가 없는 빈파일은 없는 것이 더 좋을 것 같습니다.

import BestProducts from './component/BestProducts';
import ProductList from './component/ProductList';
import Pagination from './component/Pagination';
import SortMenu from './component/SortMenu';
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
컴포넌트 명은 회사에서 사용하는 용어들이 따로 있습니다. 지금은 그러한 가이드 라인이 없으니 태호님이 원하시는 방식으로 네이밍을 하시면 된다고 생각합니다.
다만 일반적으로 어떤 버튼을 클릭했을 때 선택 메뉴가 나오는 방식의 컴포넌트를 dropdown이라고 합니다.
이러한 용어를 어떻게 사용하는지 궁금하다면 피그마에서 해당 컴포넌트의 이름을 확인해보시면 도움이 됩니다.

스크린샷 2024-09-09 오전 9 53 21

function App() {
const [bestProducts, setBestProducts] = useState([]);
const [isOpen, setOpen] = useState(false);
const [sortbtntext, setSortbtntext] = useState('최신순');
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 [sortbtntext, setSortbtntext] = useState('최신순');
const [sortBtnText, setSortBtnText] = useState('최신순');

Copy link
Collaborator

Choose a reason for hiding this comment

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

하단에 sortOrder 가 있어서 따로 sortBtnText를 state로 관리하지 않아도 될 것 같습니다.

<SortMenu
    sortOrder={order === 'recent' ? '최신순' : '좋아요순'}
    DropDown={DropDown}
    isOpen={isOpen}
    handleSortClick={handleSortClick}
/>

Comment on lines +17 to +19
const pagebuttons = [1, 2, 3, 4, 5];
const [activePage, setActivePage] = useState(1);
const [sortOrder, setSortOrder] = useState('recent');
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 pagebuttons = [1, 2, 3, 4, 5];
const [activePage, setActivePage] = useState(1);
const [sortOrder, setSortOrder] = useState('recent');
const [activePage, setActivePage] = useState(1);
const [sortOrder, setSortOrder] = useState('recent');
const pagebuttons = [1, 2, 3, 4, 5];

Comment on lines +39 to +45
const response = await axios.get(`https://panda-market-api.vercel.app/products?page=${activePage}`);
const sortedProducts = response.data.list.sort((a, b) => {
return sortOrder === 'favorite'
? b.favoriteCount - a.favoriteCount
: new Date(b.updatedAt) - new Date(a.updatedAt);
});
setSortedProducts(sortedProducts);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
이부분도 위와 동일하게 orderBy param을 통해서 정렬된 채로 데이터를 받으실 수 있습니다.

Suggested change
const response = await axios.get(`https://panda-market-api.vercel.app/products?page=${activePage}`);
const sortedProducts = response.data.list.sort((a, b) => {
return sortOrder === 'favorite'
? b.favoriteCount - a.favoriteCount
: new Date(b.updatedAt) - new Date(a.updatedAt);
});
setSortedProducts(sortedProducts);
const response = await axios.get(`https://panda-market-api.vercel.app/products?page=${activePage}&orderBy=${sortOrder}`);
setSortedProducts(response.data.list);

<p>No image available</p>
)}
<p className="itemTitle">{product.name} 팝니다</p>
<p className="price">{product.price} 원</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
디자인상에서, 그리고 일반적으로 돈을 표시할때 정해진 자리마다 콤마를 찍어줍니다.
자바스크립트 내장 메소드를 사용하면 편리하세 변환할 수 있습니다.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toLocaleString

Suggested change
<p className="price">{product.price}</p>
<p className="price">{product.price.toLocaleString()}</p>

flex-direction: column;
align-items: flex-end;
margin-top:42px;
right: 360px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
드롭다운 메뉴 위치가 좀 다르게 작업된 것 같습니다.
위치값을 아래와 같이 바꾸고
.Recently_favorite_sortmenu 컴포넌트에 position: relative 추가해보세요.

Suggested change
right: 360px;
right: 0;
top: 0;

border: 1px solid #E5E7E8;
width: 130px;
height: 42px;
border-radius: 12px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
디자인대로 작업하시려면 borderRadius가 각 메뉴 아이템마다(최신순, 좋아요순) 있는 것이 아니라 해당 메뉴 아이템의 부모에게 borderRadius가 들어가야 할 것 같습니다.

Suggested change
border-radius: 12px;


return (
<div className="pagination">
<button>&lt;</button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
button 사용시 type을 명시해서 사용해주세요.

Suggested change
<button>&lt;</button>
<button type='button'>&lt;</button>

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