Feat/356 #369
Feat/356 #369
Conversation
- 변경된 로그인 로직 적용 - 상품 가격의 총합을 상태로 전달받아 적용 - Cart컴포넌트에서 체크박스 선택된 아이템만 영수증에 렌더링 되도록 props로 전달받음
* 체크박스 클릭 미적용상태
* 체크박스 클릭 미적용상태
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/pages/Cart/index.tsx
Outdated
| {isFetched && ( | ||
| <> | ||
| <Header /> | ||
| <section className={styles.area_cart}> | ||
| <MainWrapper> | ||
| <div className={styles.wrapper_cart}> | ||
| <div className={styles.wrapper_cart_box}> | ||
| <CartBoxHeader isItemInCart={isItemInCart} /> | ||
| {isItemInCart ? ( | ||
| <ul className={styles.wrapper_item}> | ||
| {cartItems!.map((item) => ( | ||
| <li key={item.cartId}> | ||
| <CartBoxItem | ||
| item={item} | ||
| handleSelect={handleSelect} | ||
| isSelected={selectedItems.some( | ||
| (selectedItem) => | ||
| selectedItem.productId === item.productId, | ||
| )} | ||
| /> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| ) : ( | ||
| <EmptyCartBoxBody /> | ||
| )} | ||
| <CartBoxFooter isItemInCart={isItemInCart} /> | ||
| </div> | ||
| <CartPay | ||
| selectedItems={selectedItems} | ||
| totalPayment={totalPayment} | ||
| /> | ||
| </div> | ||
| </MainWrapper> | ||
| </section> | ||
| </> | ||
| )} | ||
| {isLoading && <Spinner />} |
There was a problem hiding this comment.
엘리먼트 구조를 단순하게 표현하면 이런 식인 것 같은데요.
{isFetched && (
<>
<Header />
<section />
</>
)}
{isLoading && <Spinner />}보시면 로딩 중일 때에는 헤더가 나타나지 않도록 되어 있습니다.
저는 로딩 중이어서 스피너가 돌아갈 때에도 헤더가 나타나는 게 좋다고 생각해요. 헤더가 없으면 페이지가 지연되는 것처럼 보이거나 일관되지 않는 경험을 줄 것 같아서요.
그래서 헤더를 바깥으로 빼는 건 어떨까요..? 아래처럼요.
<>
<Header />
{isFetched && <section />}
{isLoading && <Spinner />}
</>There was a problem hiding this comment.
좋은 의견 같아요! 반영했습니다. 52fdfe6
| setSelectedItems((prevSelectedItems) => { | ||
| if (prevSelectedItems.some((item) => item.productId === productId)) { | ||
| return prevSelectedItems.filter((item) => item.productId !== productId); | ||
| } | ||
|
|
||
| const selectedItem = cartItems!.find( | ||
| (item) => item.productId === productId, | ||
| ); | ||
|
|
||
| return selectedItem | ||
| ? [...prevSelectedItems, selectedItem] | ||
| : prevSelectedItems; | ||
| }); |
There was a problem hiding this comment.
아이템 선택/선택해제가 되었을 때를 다루고 있는데, 서로 다른 케이스가 섞여 있는 것 같아요.
예를 들면 37~39줄은 이미 선택했던 아이템을 선택 해제하는 경우이고, 그 이후 줄에서는 아이템을 새롭게 선택하는 경우를 다루고 있어요.
그래서 코드를 읽을 때 하나하나 뜯어봐야 이해가 되는 느낌이에요.
각 케이스에 주석을 달아주면 이해하기 더 쉬울 것 같습니다!
There was a problem hiding this comment.
맞아요 이미 선택된 아이템을 체크하는 과정과 선택되어있지 않은 아이템을 체크하는 두가지를 한번에 다루는 함수라 헷갈릴 수 있겠네요... 근데 api가 변경된다면 사라질 코드라서 일단 따로 수정하지는 않겠습니다..!
| const selectedItem = cartItems!.find( | ||
| (item) => item.productId === productId, | ||
| ); | ||
|
|
||
| return selectedItem | ||
| ? [...prevSelectedItems, selectedItem] | ||
| : prevSelectedItems; |
There was a problem hiding this comment.
cartItems는 장바구니에 등록된 상품 목록을 나타내고, selectedItem은 그 목록 중에 선택된 상품을 나타내는 것 같습니다. 그래서 find()의 결과가 항상 truthy할 것 같은데, 반환부를 보니까 selectedItem이 falsy 값인 경우도 처리해주고 있습니다.
find()했을 때 조건에 맞는 아이템을 찾지 못하는 경우가 있는 건가요?
아니면 혹시 몰라서 falsy 값인 경우도 처리해주신 건가요?
제가 코드를 잘못 이해했을까봐 여쭤봅니다!
There was a problem hiding this comment.
혹시몰라서 falsy 값을 처리해주긴 했어요 왜냐면 delete 요청을 보냈을때 두개의 상태가 별개로 관리되니까 혹시나 데이터가 꼬일까봐요.... 근데 오늘 회의때 아예 api 수정요청을 드리는것이 나을것 같아요. 그때 되면 selectedItem 상태가 사라져서 사라지는 코드가 되지 않을까 싶습니다...! 이부분은 추후 수정하겠습니다.
| queryFn: () => getCartItems(), | ||
| }); | ||
|
|
||
| const isItemInCart = cartItems ? cartItems.length > 0 : false; |
There was a problem hiding this comment.
배열이 null, undefined이거나 빈배열인지 검사하는 건가요?
There was a problem hiding this comment.
상품이 없는 경우라면 헤더푸터가 노란색이미지가 아니라 회색 이미지로 변경되어야 하니까 배열의 길이를 통해 상품이 있는지 없는지 확인하기위한 변수입니다!
| @@ -32,6 +32,22 @@ const Cart = () => { | |||
|
|
|||
| const isItemInCart = cartItems ? cartItems.length > 0 : false; | |||
|
|
|||
| const handleSelect = (productId: number) => { | |||
| setSelectedItems((prevSelectedItems) => { | |||
| if (prevSelectedItems.some((item) => item.productId === productId)) { | |||
#️⃣연관된 이슈
close: #356
📝작업 내용
🙏리뷰 요구사항(선택)