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

[NEXTLEVEL 클린코드 리액트 조성륜] 장바구니 미션 Step1 #5

Merged
merged 6 commits into from
Mar 1, 2022

Conversation

ocipap
Copy link

@ocipap ocipap commented Feb 27, 2022

step2 를 진행하면서 기타 여러 기능들을 구현해보도록 하겠습니다!
vite + jest 환경 셋팅에 시간을 많이 사용한 것 같아 step2 를 하면서 기능에 초점을 맞춰보도록 하겠습니다 :)

필수 요구사항

  • React Testing Library & Jest를 활용해 자유로운 단위의 테스트를 진행한다.

GNB

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

상품목록

  • 상품들은 n x 4 레이아웃으로 보여진다.
    • 반응형 레이아웃을 구현한다.
  • 상품들에는 사진, 이름, 금액이 보여진다.
  • 장바구니 버튼을 클릭하면 장바구니 페이지로 이동한다. (간단한 함수 실행만 작업)
  • 페이징을 적용한다.

심화 요구사항

  • 도출된 요구사항을 기반으로 User Flow Diagram 혹은 Flow Chart 작성
  • UI/UX
    • 사용자를 위한 로딩 환경 개선
    • 페이징 혹은 인피니티 스크롤 적용 (별도의 API 없음)
      • 뒤로가기 및 페이지 전환시 기존 페이지 및 스크롤 위치 기억
    • 상품이 없을 때와 같은 다양한 Edge Case 대응
    • 반응형 레이아웃 구현
    • 별도의 모바일 레이아웃 추가 제공
    • 배민상회를 참고하여 추가 개선 사항 반영
  • 매출 증대 및 마케팅을 위해 별도의 기능 구현 (별도의 API 없음)
    • 브라우저 새로고침시 모든 상태 유지
    • 흐름을 고려한 맞춤 큐레이팅 상품 추천 기능
    • 구매 유도를 위한 상품 찜 페이지
  • 매출 증대 및 마케팅을 위한 별도의 도구 추가
    • Google Analytics
    • Google Tag Manager

@ocipap ocipap self-assigned this Feb 27, 2022
@joel-jo-querypie joel-jo-querypie changed the title [NEXTLEVEL 클린코드 리액트 위영민] 장바구니 미션 Step1 [NEXTLEVEL 클린코드 리액트 조성륜] 장바구니 미션 Step1 Feb 27, 2022
@joel-jo-querypie joel-jo-querypie self-assigned this Feb 27, 2022
@joel-jo-querypie joel-jo-querypie self-requested a review February 27, 2022 09:56
Copy link
Member

@joel-jo-querypie joel-jo-querypie left a comment

Choose a reason for hiding this comment

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

안녕하세요 성륜님~~ 이렇게도 테스트하는구나 배울 수 있었네요.
미션 구현하시느라 고생 많으셨어요!
추가로 테스트 이름은 영어로 하는 걸 선호하시는지 궁금합니다ㅎㅎ

https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#using-wrapper-as-the-variable-name-for-the-return-value-from-render

Kent C 님 블로그에 테스팅 관련해서 한번 쯤 읽어보면 좋은 내용인 것 같아 공유드립니다!
그럼 다음 스텝에서 뵙겠습니다!

고생 많으셨어요~~!

Comment on lines +9 to +12
beforeEach(() => {
handleClickItem.mockClear();
handleClickCart.mockClear();
});
Copy link
Member

Choose a reason for hiding this comment

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

오 mockClear는 이렇게 습관적으로 해주는게 좋은건가요? 조금 찾아보니까 모든 테케가 독립적으로 실행되게 하기 위해서는 mock을 정리하는게 좋다고 하네요. .mockClear, mockReset, .mockRestore 미묘한 차이가 있나보네용. https://jestjs.io/docs/mock-function-api#mockfnmockclear

Comment on lines +28 to +35
it('Click card item, call handleClickItem', () => {
const { getByRole } = render(
<ProductCard
item={{ id: 1, name: 'item', imageUrl: '', price: 1000 }}
onClickItem={handleClickItem}
onClickCart={handleClickCart}
/>
);
Copy link
Member

Choose a reason for hiding this comment

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

유저 이벤트를 이렇게 테스트 할 수도 있네요. 미팅때 공유해주신 내용 이렇게 코드로 보니까 좀 더 이해가 잘 되는군욬ㅋㅋ

height = 24,
fill = 'current',
}: {
// eslint-disable-next-line react/require-default-props
Copy link
Member

Choose a reason for hiding this comment

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

요건 왜죠??!

Comment on lines +19 to +23
<ProductCard
item={item}
onClickItem={handleClickItem}
onClickCart={handleClickCart}
/>
Copy link
Member

Choose a reason for hiding this comment

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

클릭 후 어떤 작업을 하는지 명시해주는 네이밍이면 좋지 않을까요?

Suggested change
<ProductCard
item={item}
onClickItem={handleClickItem}
onClickCart={handleClickCart}
/>
<ProductCard
item={item}
onClickItem={아이템상세보기로이동한다}
onClickCart={장바구니에추가한다}
/>

저는 이런 식으로 되면 좀 더 사용하는쪽에서 ProductCard 가 어떤 놈인지 파악하기 좋은 것 같아요!

import ky from 'ky';
import { Product } from '../types/dto';

const api = ky.create({ prefixUrl: 'http://localhost:3003/' });
Copy link
Member

Choose a reason for hiding this comment

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

prefixUrl 같은 경우는 상수로 관리하거나 환경 변수로 관리되면 어떨까용? 실수도 종종하고, 다른 곳에서 종종 쓸 때가 있어 따로 상수로 관리해두면 어떨까 제안해봅니당

@@ -0,0 +1,27 @@
import React from 'react';

export function IconCart({
Copy link
Member

Choose a reason for hiding this comment

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

Icon 컴포넌트로 조금 더 재활용 할 수 있는 구조로 만들 수 있지 않을까요? props 로 path 값을 받는 등의 방법으로 만들어도 괜찮을 것 같습니다! 다른 아이콘이 또 추가될 경우를 고려해서요!

"tailwindcss-debug-screens": "^2.2.1",
"ts-jest": "^27.1.3",
"ts-node": "^10.5.0",
"vite": "^2.8.0"
Copy link
Member

Choose a reason for hiding this comment

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

vite!! 나중에 후기 공유해주세요~~~

it('render', () => {
const { container } = render(
<ProductCard
item={{ id: 1, name: 'item', imageUrl: '', price: 1000 }}
Copy link
Member

Choose a reason for hiding this comment

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

계속 사용되는 item 같은 경우는 fixture로 빼도 될 것 같긴한데 이렇게 하는게 더 명시적인 것 같아 좋은 것 같기도 하고 모르겠네요 ㅎㅎ

/>
);

userEvent.click(getByRole('listitem'));
Copy link
Member

Choose a reason for hiding this comment

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

listitem role도 있군요. getByRole 를 조금 더 학습해봐야겠네요

Copy link
Member

Choose a reason for hiding this comment

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

fireEvent, useEvent 구분하지 않고 막 썼는데 userEvent 사용을 권장하네용

@joel-jo-querypie joel-jo-querypie merged commit 6f4f713 into nextlevel-2022:ocipap Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants