2조 과제 제출 - #9
Conversation
|
|
||
| // 관리자 - 상품 조회 | ||
| export const adminFetchProducts = async () => { | ||
| const response = await adminInstance.get('/products') |
There was a problem hiding this comment.
IMP: 전반적으로 api 응답 에러 처리가 되어 있지 않고 있어요 try catch문을 사용해보시는 걸 추천드려요!
|
|
||
| //선택 가능한 은행 목록 조회 | ||
| export const getBankLists = async () => { | ||
| const res = await authInstance.get('/account/banks') |
There was a problem hiding this comment.
nit: 여기도 try catch 문 사용하시면 에러 처리되고 좋아요
| const customerGrade = useMemo(() => { | ||
| // 회원등급 (누적금액) | ||
| // 브론즈(0~10만) > 실버(10만~20만) > 골드(20만~50만) > 플래티넘(50만~100만) > 다이아몬드(100만~) | ||
| if (totalTransactionPrice < 100000) { |
There was a problem hiding this comment.
nit: if문 분기 처리보다 switch를 사용하시거나 따로 모듈로 빼셔도 좋을 것 같아요
| @@ -0,0 +1,18 @@ | |||
| import styled from 'styles/components/admin/adminSkeleton.module.scss' | |||
|
|
|||
There was a problem hiding this comment.
nit: skeleton은 따로 폴더 만들어서 import 해서 사용하시면 폴더링이 좀더 깔끔해지고 좋아요
| <ul> | ||
| <div className={styles.notice}>장바구니 이용안내</div> | ||
| <li> | ||
| 해외배송 상품과 국내배송 상품은 함께 결제하실 수 없으니 장바구니 |
There was a problem hiding this comment.
IMP: 하드코딩 하시는 것보다 json으로 목업 데이터 만들어서 바인딩 하시는 방법도 추천드려요
| export const PaymentMethods = () => { | ||
| // USER INFO | ||
| const { phoneNumber } = useContext(PhoneNumberContext) | ||
| const { bank } = useContext(BankContext) |
There was a problem hiding this comment.
IMP: paymentInfo라는 상태 하나로 추상화 하면 코드가 훨씬 간결해 지지 않을까요?
| import styled from 'styles/components/mypage/myWishItem.module.scss' | ||
| import { Link } from 'react-router-dom' | ||
| import { WishListContext } from 'contexts/index' | ||
| import noImage from 'public/no-photo.png' |
| }) => { | ||
| const navigate = useNavigate() | ||
| const { setUserCart } = useContext(CartContext) | ||
| const { address } = useContext(UserAddressContext) |
There was a problem hiding this comment.
IMP: 유저 관련 상태값을 하나의 각각 개별 context로 담은 이유가 있을까요? 컴포넌트 상태값이 여러개이고 동시에 바껴야 하는 경우 하나의 객체에 담아서 관리하면 코드가 더 깔끔해질 수 있어요
|
|
||
| export const DaumPostCode = () => { | ||
| const { address, setAddress } = useContext(UserAddressContext) | ||
| const { setAddressDetail } = useContext(AddressDetailContext) |
There was a problem hiding this comment.
nit: 이부분도 하나의 콘텍스트에 담아서 관리하는 걸 추천 드려요
| // 로그인 X + guest 장바구니 초기화 | ||
| localStorage.setItem(`cart-guest`, JSON.stringify([])) | ||
| } else { | ||
| if (guestItems !== null && guestItems.length > 0) { |
There was a problem hiding this comment.
nit: 중첩 조건문으로 인해 useEffect 콜백함수가 무거워 보여요 가능하면 따로 모듈로 빼서 관리하는게 좋아요
| </div> | ||
| ) : null} | ||
|
|
||
| {isModalShow && modalProps ? ( |
There was a problem hiding this comment.
modal을 props로 전달하지 않고, 하는방법은 없을까요? 매번 모달 프롭스를 전달하는방식보다 전역에서 선언해두고 상태로 관리하는게 좋을 것 같아요
| <AdminSalesSkeleton /> | ||
| <AdminSalesSkeleton /> | ||
| <AdminSalesSkeleton /> | ||
| <AdminSalesSkeleton /> |
There was a problem hiding this comment.
그냥 Array.fill 채우시고 하시는게 더 편안해보이는데여... 매번 복사를 하실필요가 있을까 생각들어요
| export const Cart = () => { | ||
| return ( | ||
| <> | ||
| <div className={styles.wrapper}> |
| /> | ||
| </div> | ||
| ) : ( | ||
| <div></div> |
There was a problem hiding this comment.
null로 처리해줘도되지않을까요? 또는 && 로 해도 될 것 같아요
| /> | ||
| </div> | ||
| ) : ( | ||
| <div></div> |
| <div className={styled.container}> | ||
| <div className={styled.path}> | ||
| <Link to="/">홈</Link> | ||
| <span>/</span> |
| <UserAddressContext.Provider value={{ address, setAddress }}> | ||
| <BankContext.Provider value={{ bank, setBank }}> | ||
| <PhoneNumberContext.Provider value={{ phoneNumber, setPhoneNumber }}> | ||
| <AccountNumberContext.Provider |
There was a problem hiding this comment.
Payment Provider를 하나만들어서 관리하는게 나중에 유지보수가 올라갈것 같아요 nested가 너무 depth가 깊어서 보기안좋아요
sangheon-kim
left a comment
There was a problem hiding this comment.
수고하셨습니다 부족한 부분 코멘트 달았어요
추가로, type선언한것에 그 어느것도 jsdoc으로 설명이 안달려있네요 유지보수 하기 어려울 것 같아요 그리고 declaration으로 선언하면 굳이 매번 import로 받을 필요 없을 것 같아요
| <div className="product-sort"> | ||
| <ul> | ||
| <li> | ||
| <a |
| @@ -0,0 +1,41 @@ | |||
| export interface Address { | |||
| @@ -0,0 +1,6 @@ | |||
| export type AdminDashboardCardProps = { | |||
There was a problem hiding this comment.
declaration type을 사용하지 않고, export를 사용하신이유가있나요?
Colley
🔗 배포주소
Colley
📄 프로젝트 소개
거래 API를 활용한 온라인 쇼핑몰입니다.
🗓 프로젝트 기간
2023.05.30 ~ 2023.07.02
🧑💻 기술 스택
🤝 개발팀 소개
관리자 페이지,
위시리스트,
마이페이지 - 구매
구매 페이지, 계좌,
장바구니
장바구니,
디자인 가이드,
마이페이지 - 조회,수정
상품 상세,
상품 카테고리별 페이지
📂 폴더구조