[1단계 - 장바구니 미션] 엘라(김현주) 미션 제출합니다

[bucketHaneul]

안녕하세요 자프님 😊 이번에 @zereight 와 함께 장바구니 미션을 진행한 엘라입니다~!

데모 페이지 https://react-shopping-cart-6509f.firebaseapp.com

🙌 미션 필수 요구사항

GNB

• "로고"를 누르면 상품목록 페이지로 이동한다.
• "장바구니" 버튼을 누르면 장바구니 페이지로 이동한다.
• "주문목록" 버튼을 누르면 주문목록 페이지로 이동한다.

상품목록

• 상품들에는 사진, 이름, 금액이 보여진다.
• 장바구니 버튼을 클릭하면 장바구니 이동 선택 모달이 보여진다.
  • 장바구니 이동 버튼을 클릭하면 장바구니 페이지로 이동한다.
  • 추천 상품 3개를 보여준다.

장바구니

• 해당 상품의 수량을 변경할 수 있다.
  • 상품의 수량은 항상 1이상, 20이하여야 한다
  • 해당 상품의 총 금액이 변경된다.
  • 해당 상품이 체크되어있으면, 결제예상금액도 변경된다.
• 모두선택 버튼이 체크되면, 상품들이 모두 선택된다.
  • 모두선택 버튼이 체크가 풀리면, 상품들의 선택이 모두 해제된다.
• 상품 삭제 버튼을 누르면 해당 상품이 삭제된다.
• 🗑 버튼을 누르면 해당 상품이 삭제된다.
• 주문하기 버튼을 누르면 상품이 주문된다.
  • 확인을 누르면, 주문/결제 페이지로 이동한다.
  • 확인을 누르면, 장바구니에서 선택된 상품들이 삭제된다.

주문/결제

• 주문할 상품들의 정보가 보여진다.
• 총 결제금액을 보여준다.
• 결제하기 버튼을 클릭하면, confirm 메시지가 보여진다.
  • 확인 버튼을 누르면, 주문 목록페이지로 이동한다.

🙌 미션 심화 요구사항

주문목록

• 주문 정보들이 보여진다.
• 장바구니 버튼을 클릭하면, 해당 상품이 장바구니에 담기고 장바구니 이동 선택 모달이 보여진다.
  • 장바구니 이동 버튼을 누르면 장바구니 페이지로 이동한다.

UI/UX

• 반응형 레이아웃 구현
• 모바일 레이아웃 추가 구현

😁 중점적으로 봐주셨으면 하는 점

• redux가 잘 적용되었는지, 추가적인 적용이 필요한 부분은 없는지
• 비동기 통신이 존재하는데에 있어, 이에 맞게 action, reducer가 알맞은 역할을 하고 있는지
• 컴포넌트가 재사용성있게 잘 분리되었는지

🎇 궁금한 점

1. 비동기 통신에 따른 state 갱신이 필요한데 이러한 로직을 어디에 위치 시켜야하는지에 대해 고민이 많았습니다.

   • 상품 목록을 전부 가져와서 상태를 갱신한다. -> 비동기 통신 후 상태 갱신
   • 변경된 장바구니 상품 목록 (갱신된 상태)을 데이터 베이스에 저장한다. -> 상태 갱신 후 비동기 통신

   이렇게 되다보니 비동기 통신의 로직이 이곳 저곳에 위치하는 것 같아서 고민하다가, 무조건 비동기 통신을 먼저 실행한 후 성공했을시 상태를 변경하는 로직으로 수정하였는데요! 그러다보니 비동기 통신 전 새로운 update content를 만들어 내는 로직이 밖으로 빠지게 되면서 중복되고 있습니다 ㅜ 비동기 통신이 섞인 redux의 상태 업데이트의 흐름이 이렇게 흘러가는게 맞을까요? ㅜ ㅜ

// content를 update 하는 로직
const newContent = { productIdList: myShoppingCartProductIds.filter(productId => productId !== targetId) };
// 비동기 통신 하러 가는 로직
dispatch(updateShoppingCartItemsAsync(SCHEMA.SHOPPING_CART, myShoppingCartId, newContent));

잘 부탁 드리겠습니다 🥰🥰🥰

[wow9144]

데모 페이지 둘러본 결과 주문 플로우가 잘못 된 것 같습니다.
상품목록 -> 장바구니 -> 주문 -> 결제 여야 하는데, 주문과 결제가 뒤바뀐것 같습니다.
장바구니에서 주문서로 갔을 때는 여전히 장바구니의 아이템이 있어야 하고, 결제까지 끝나야 장바구니의 아이템을 비워야 할텐데, 결제 플로우가 먼저 나오면서 장바구니도 비우는것 같네요.

라는 댓글을 먼저 남기고 코드를 볼게요.

[wow9144]

궁금한 점에 대한 답변

세부 코멘트에서 답을 찾으실 수도 있을것 같고 추가로 보충하자면..
상품 목록을 가져와서 상태 갱신을 하는 부분은 지금처럼 하시면 될 것 같습니다.
다만 더 좋은 형태로 개선할 부분을 찾자면 useServerAPI([], SCHEMA.PRODUCT); 를 한 번 더 혹으로 감싸서 그 훅을 컴포넌트가 사용하도록 해서 컴포넌트는 스키마를 모르고 productList를 어디서 가져오는건지도 모르게 하는게 더 좋을 것 같긴 합니다.
그렇게 중간에 레이어가 하나 더 생기면 서버에서 가져온 데이터를 바로 컴포넌트에서 쓰는게 아니라 중간에 한 번 가공을 할 수도 있고, 다른 메서드를 추가로 제공할수도 있고.. 유연하게 확장성이 좋아집니다.

상태 갱신후 비동기 통신은, thunk 함수에서 하면 됩니다.
유저가 선택했거나 하는 정보들은 컴포넌트를 통해서 나오기 때문에 컴포넌트가 전달해야 하지만, 애초에 리덕스 스토어에 있는 정보는 thunk에서 getState로 꺼내서 주요로직은 그 함수 내에 두고 상태갱신을 앞에 하든 뒤에하든 여기서 하면 됩니다.

[wow9144]

updateShoppingCartItemsAsync 를 중심으로 수정이 조금 필요할 것 같습니다.
전반적으로 컴포넌트 분리는 잘 되어 있는것 같습니다.

[wow9144]

updateShoppingCartItemsAsync에 문제가 될 포인트가 몇 가지 보입니다.
이를 개선해봅시다.

1. 첫 번 째 파라미터인 schema에 "shoppingCart" 가 아닌 다른 값이 들어와도 됩니까? (또는 추후에라도 받아 줄 수 있도록 확장 가능합니까?)
2. 이 schema가 컴포넌트(로직을 갖는 페이지 컴포넌트라고 하더라도)의 관심사(알고 있어야 하는 정보)입니까?

저는 두 질문의 답이 no 라고 생각됩니다.
requestTable에는 스키마를 전달해야 하는 것이 분명하지만, 그 스키마를 알고 있는 것은 updateShoppingCartItemsAsync 면 충분 할 것 같습니다.
외부 통신을 위한 스키마는 컴포넌트가 알아서 누군가에게 전달해야 하는 정보가 아닙니다.
그래서 첫 번째 파라미터인 schema 는 제거 되어야 할 것 같습니다.
아래 getMyShoppingCartAsync도 같아 보입니다.

3. tragetId와 content 또한 정말 외부(컴포넌트)에서 받아야 할 정보입니까?
   => async dispatch => {
=> async (dispatch, getState) => {
   로 수정하시어 getState() 를 통해 이 함수 내부에서 결정 할 수 있지는 않을까요?

[bucketHaneul]

자프님의 질문에 대해서도 스스로 생각해보니 1번 2번 모두 No인 것 같습니다! 페이지 컴포넌트에서 스키마에 대한 정보를 알기보다는 비동기 로직을 사용하는 액션함수에서 직접 알고있어도 될 부분인 것 같습니다. 수정했습니다 ㅎ ㅎ

그리고 3번 질문에 대해서는 부끄럽지만 => async (dispatch, getState) => { 이렇게 해서 액션 함수에서 상태를 조회할 수 있는지 몰랐습니다.....😥.. 액션 함수로 상태를 조회할 수 있게되니 액션함수가 많이 수정되었습니다. 이에 맞춰서 아래 피드백에서 질문 주신 부분에 대한 답변 달았습니다. 😄😄😄

커밋 2f9443c

[wow9144]

내부에서 분기를 치는 방법 보다
모두 삭제하는 함수와 단건만 삭제하는 함수로 나누는게 더 좋습니다.
더 명시적(이름이 명확하니)이고 변화가 생겼을 때 수정하기도 더 용이합니다.

[bucketHaneul]

  const deleteAllShoppingCartItem = () => {
    if (!window.confirm(CONFIRM_MESSAGE.DELETE)) return;

    dispatch(deleteAllShoppingCartItemAsync(checkedIdList));
  };

  const deleteShoppingCartItem = targetId => {
    if (!window.confirm(CONFIRM_MESSAGE.DELETE)) return;

    dispatch(deleteShoppingCartItemAsync(targetId));
  };

위와 같이 분리하였습니다. 감사합니다😊

[wow9144]

이 코드를 보고 추측컨데...
상품 추가 할 때 ADD_ITEM, 지울 때 DELETE_ITEMS 을 쓰려고 했는데 막상 하다보니 UPDATE_MY_SHOPPING_CART_ITEMS 하나로 쓰는게 더 좋지 않을까(혹읔 귀찮아서) 해서 이걸로 퉁치신게 아닌가 하는 싸늘한 느낌이 듭니다.
그렇다면(그렇지 않다고 하더라도-_-;)...추가 할 때는 ADD 지울 때는 DELETE 로 액션 타입을 명확하게 나누는게 좋습니다.
액션이 명확(이라고 쓰고 코드가 장황....) 하도록 작성하는게 리덕스의 묘미(?) 입니다.

[bucketHaneul]

이번 미션에서 가장 고민이었던 부분은 기존 상품목록 id 리스트에서 새로 update된 상품목록 id 리스트로 변경하는 로직의 위치였습니다 ㅎ ㅎ

현재는 해당 로직이 page 컴포넌트에 있는데요!
해당 로직이 thunk 함수 밖에 있다보니, 장바구니에 상품을 추가하든 삭제하든 결국 thunk 함수는 최종 update된 id리스트를 firebase에 PUT 하기만하면 된다고 생각하였습니다. 그래서 두 case의 비동기 로직을 updateShoppingCartItemsAsync()로 공유하도록 작성했습니다!

그런데 현재는 해당 로직을 액션 함수 내로 옮겼고, 그러다보니 자연스럽게 비동기 로직을 가진 액션함수가 분리될 수 있었습니다!!!!!!!!!!!!!!!!!

const addShoppingCartItemAsync = newProductId => async (dispatch, getState) => {
  const { id, productIdList } = getState().myShoppingCartReducer.myShoppingCart;

  try {
    const newContent = { productIdList: [...new Set([...productIdList, newProductId])] };

    await requestTable.PUT(SCHEMA.SHOPPING_CART, id, newContent);

    dispatch({
      type: ADD_SHOPPING_CART_ITEM,
      newProductId,
    });
  } catch (error) {
    console.error(error);
  }
};

const deleteShoppingCartItemAsync = targetId => async (dispatch, getState) => {
  const { id, productIdList } = getState().myShoppingCartReducer.myShoppingCart;

  try {
    const newContent = { productIdList: productIdList.filter(productId => productId !== targetId) };

    await requestTable.PUT(SCHEMA.SHOPPING_CART, id, newContent);

    dispatch({
      type: DELETE_SHOPPING_CART_ITEM,
      targetId,
    });
  } catch (error) {
    console.error(error);
  }
};

그리고 여전히 고민인 부분은,,,, 액션 타입을 ADD_ITEM, DELETE_ITEM으로 나누어야할까? 하는 부분입니다 😥😥😥

위 두 액션함수 모두 update된 장바구니 상품Id List를 newContent로 가지게되는데, 리듀서에게 newContent로 넣어줄지 아니면 지금처럼 newProductId, targetId를 넘길지 고민이 됩니다.

• newContent 넘긴다 -> 액션 타입을 UPDATE_ITEM 하나만 가지게 돼서 명확하지 않다.
• newProductId, targetId를 넘긴다 -> 액션 타입은 명확해진다. 하지만, update된 장바구니 상품Id List를 다시 계산해야한다.

커밋 2f9443c

[wow9144]

상황에 따라 케바케일수도 있고해서 이건 다양하게 코드를 바꿔가면서 경험과 시간으로 결국 스스로 해결해야 할 것 으로 보이네요.
엘라님은 충분히 스스로 답을 찾아내실거라 믿기에 위 고민과 직접적인 연관이 없을 수는 있지만 여기서 알아두시면 좋은 부분을 알려드릴게요.

1. 이런 상황(하고자 하는게 추가, 삭제이지만 POST, DELETE가 아닌 업데이트)에서는 PUT 메서드보다는 PATCH 메서드가 옳습니다.(firebase에서 PATCH 메서드로 동작하는 API 가 있는지는 모르겠지만)
2. 객체에서 id만 따로 뽑아내서 상태를 관리하는것보다, id 를 가진 객체 그 자체를 상태로 관리하는게 대부분의 경우 더 용이합니다.
3. 추가라는 동작을 할 때는 추가하는 대상 객체가 필요하지만, 삭제라는 동작을 할 때는 삭제할 대상을 특정 할 수 있는 고유값(id) 또는 형태만 있으면 됩니다. 따라서 코드의 디자인상
   dispatch({ type: ADD_SHOPPING_CART_ITEM, newProductId, });
   이건 이상하다는 의미입니다.
   ADD_SHOPPING_CART_ITEM 타입과 함께 있어야 하는것은 id가 아니라 add될 아이템 객체여야 하는것이죠.
   이것은 위에 설명드린 2번과 맥락을 같이 하는 내용이기도 합니다.

글로 쓰니 내용 전달이 어려울 수 있지만 곰곰이 한 번 생각 해보시고 2단계에 한 번 시도 해보셔도 좋을 것 같습니다.

[wow9144]

shoppingCartItemList야 말로 리덕스로 관리하면 좋을텐데 왜 state로 하셨는지 의아합니다.

[bucketHaneul]

지금 1단계에서는 API가 제공되지 않아, mock data를 임시적으로 만들어 사용하고있습니다 😁😁😁

제가 생각하기에 장바구니에 담기는 상품목록은 담긴 상품에 대한 id를 list로만 가지고 있어도 충분하다고 생각하였습니다.
그러다보니, 장바구니에 상품을 추가하고 삭제할 때는 장바구니에 담긴 상품에 대한 id list를 수정하고, 따라서 shoppingCart에 담긴 productIdList를 리덕스로 관리하였습니다!

그리고 shoppingCartItemList은 장바구니에 담긴 상품에 대한 id list와 상품 목록의 productId를 join(?)하여 가져온 상품 정보 state입니다.
-> 아마 2단계에서 제공되는 API에서는 shoppingCartItemList를 이러한 과정없이 바로 받을 수 있을 것 같습니다 ㅎ ㅎ

[wow9144]

onClickAllCheckBox 에서 구현하셔도 되었을텐데 이렇게 useEffect로 하신 이유가 궁금합니다.
상태가 비동기적으로 갱신되어 어떤 타이밍 문제를 겪으셔서 얍삽이(?)로 이렇게 하신거라면 onClickAllCheckBox 내에서 해결을 보시는게 좋을 것 같습니다.
한 컴포넌트 내에 useEffect가 많으면 혼란(코드를 읽기도, 동작을 예측하기도 어려워서)스럽습니다.

참고로 UX적으로도 모두 선택의 현재 동작은 조금 이상합니다.
(모두 선택 체크 하고 아이템 각각을 체크 해제 했는데 여전히 모두 선택은 체크가 되어 있는 상태)

[bucketHaneul]

해당 로직에 대한 useEffect를 삭제했습니다. 자프님 말대로 충분히 onClickAllCheckBox()에서 할 수 있엇네요! 그리고 체크여부에 따른 UX 문제 역시 해결하였습니다 ㅎ ㅎ

커밋 1ed9a86

[bucketHaneul]

안녕하세요 자프님 🙌

자프님이 말씀하신대로 상품목록 -> 장바구니 -> 주문 -> 결제 flow에 있어서 잘못된 confirm 메세지와 장바구니 아이템을 비우는 로직의 잘못된 위치를 확인했습니다.

더 세밀하게 생각했어야 했는데, 장바구니 페이지에서 넘어간다고 바로 아이템을 삭제해버렸네요😥😥😥

수정했습니다. 감사합니다 ❗❗❗

커밋 6c13771

[JaeYeopHan]

엘라님 편지받고 놀러와봤어요 (ㅋㅋㅋ)

[JaeYeopHan]

1. 이 Button 컴포넌트를 submit이 아닌 button type으로 사용하려면 바로 사용하지 못하고 props를 또 추가해줘야할 것 같아요.
2. disabled만 주려고 해도 마찬가지구요

HTML button element의 props를 모두 받을 수 있게 확장해보면 어떨까요?

Suggested change

	const Button = ({ children, onClick }) => <Container onClick={onClick}>{children}</Container>;
	const Button = props => <Container {...props} />;

[bucketHaneul]

와 제이비님!!!!!!!!!!!!!!!!!! 지금 리덕스 비동기 로직에 허덕허덕 대고 있었는데,,,ㅠ ㅠ 알림 메일 받고 깜짝 놀랐네요 호호호호호호호호호호호

[bucketHaneul]

와 type이나 다른 props에 대한 확장성 고려를 못했네요 ㅎㅎㅎㅎ 피드백 감사합니다 >_<