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

[조규진] Sprint5 #157

Conversation

gjrefa9139
Copy link
Collaborator

@gjrefa9139 gjrefa9139 commented Jun 21, 2024

요구사항

기본

중고마켓

  • 중고마켓 페이지 주소는 “/items” 입니다.
  • 페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
  • 상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.
  • 상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.
  • '상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )
  • 전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.

중고마켓 반응형

  • 베스트 상품
  • Desktop : 4개 보이기
  • Tablet : 2개 보이기
  • Mobile : 1개 보이기
  • 전체 상품
  • Desktop : 10개 보이기
  • Tablet : 6개 보이기
  • Mobile : 4개 보이기

심화

  • 페이지 네이션 기능을 구현합니다.

주요 변경사항

스크린샷

멘토에게

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

@gjrefa9139 gjrefa9139 changed the title 스프린트 미션 5 제출 [조규진] Sprint5 Jun 21, 2024
@gjrefa9139 gjrefa9139 requested a review from 201steve June 21, 2024 14:41
@gjrefa9139 gjrefa9139 added the 미완성🫠 죄송합니다.. label Jun 21, 2024
Copy link
Collaborator

@201steve 201steve left a comment

Choose a reason for hiding this comment

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

수고많으셨습니다!!

마이그레이션이 남아있는 컴포넌트들도 차근차근 해보시면 금방 될꺼에요! 화이팅!!!


<div className='bestItemsCardSection'>
{itemList?.map((item) => (
<ItemCard item={item} key={`best-item-${item.id}`} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<ItemCard item={item} key={`best-item-${item.id}`} />
<ItemCard item={item} key={`best-item-${item.id}`} />

중복되지 않은 고유한 key로 잘 해주셨습니다!
react에서 map에 사용되는 key는 고유한 값이고, 요소마다 중복되지않으면 됩니다.
하드코딩 되어있는 best-item- 부분은 계속 같은 값이라 영향을 주지않고, item.id만 key값으로 쓰더라도 문제없이 잘 동작합니다. :-)

<img src={item.images[0]} alt={item.name} className='itemCardThumbnail' />
<div className='itemSummary'>
<h2 className='itemName'>{item.name}</h2>
<p className='itemPrice'>{item.price.toLocaleString()}원</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<p className='itemPrice'>{item.price.toLocaleString()}</p>
<p className='itemPrice'>{item.price.toLocaleString("ko-KR")}</p>

꿀팁 공유~
number.toLocaleString에 첫번째 인자로 "ko-KR"을 넘겨주면 천 단위마다 자동으로 콤마를 찍어주는 기능이 있어요!:-)

Comment on lines +31 to +35
const fetchSortedData = async ({ orderBy, page, pageSize }) => {
const products = await getProducts({ orderBy, page, pageSize });
setItemList(products.list);
setTotalPageNum(Math.ceil(products.totalCount / pageSize));
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금도 물론 잘해주셨는데,
함수는 하나의 일만 하도록 만들면 좋습니다.

products를 불러오는일,
state를 업데이트 하는일 이렇게 두 함수로 나눠서 관리하면 디버깅하거나 유지보수 하는데에 도움이 됩니다.

//products를 fetching 하는일
const fetchProducts = async ({ orderBy, page, pageSize } => {
  return await getProducts({ orderBy, page, pageSize });
};
//state를 업데이트 하는일
const updateItemList = (products) => {
    setItemList(products.list);
    setTotalPageNum(Math.ceil(products.totalCount / pageSize));
  };


useEffect(()=>{
const handleResize = ()=>{}
..
.. 
생략

//분리된 함수를 정의한 함수
const loadData = async () => {
      const products = await fetchProducts({ orderBy, page, pageSize });
      updateItemList(products);
    };
//useEffect 안에서 실행
    loadData();
   
    return ()=>{}
생략
...
},[])

Comment on lines +74 to +75
<div className='sortButtonWrapper'>
<button className='sortDropdownTriggerButton' onClick={toggleDropdown}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

className이나 변수 이름짓는건 항상 어렵죠.. 어느선까지 구체적으로 해야할지 애매모호해서 더 자세하게 붙이다보면 본의아니게 길어지곤 합니다.
저는 이름이 길어지면 일단 의미는 지킬 수 있으면서 줄일 수 있는 단어가 있는지 찾는편이에요.

예를들면

Suggested change
<div className='sortButtonWrapper'>
<button className='sortDropdownTriggerButton' onClick={toggleDropdown}>
<div className='sortWrapper'>
<button className='sortButton' onClick={toggleDropdown}>

이렇게요 :-)

import { ReactComponent as RightArrow } from '../../../images/icons/arrow_right.svg';

const PaginationBar = ({ totalPageNum, activePageNum, onPageChange }) => {
const maxVisiblePages = 5;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const maxVisiblePages = 5;
const MAX_VISIBLE_PAGES = 5;

잘 해주셨습니다! 어떤 숫자인지 변수로 잘 처리해주셨어요!
이렇게 변하지 않는 값을 의미하는 네이밍 컨벤션은 UPPER_SNAKE_CASE로 보통 이름짓습니다 참고해 주세요~

@201steve 201steve merged commit b9f9d99 into codeit-bootcamp-frontend:React-조규진 Jun 24, 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.

5 participants