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

[페이먼츠 미션 Step 3] 가브리엘(윤주현) 미션 제출합니다. #291

Merged
merged 243 commits into from
May 8, 2023

Conversation

gabrielyoon7
Copy link
Member

@gabrielyoon7 gabrielyoon7 commented May 6, 2023

안녕하세요?

페이먼츠 미션 Step 3 제출하겠습니다!
사실 이번 Step에서는 많은 기능이 들어오거나 완전히 새로운 도전을 해보는 것 보다는 기존 기능을 개선하거나 강화하는데 시간을 많이 들였는데요, 따라서 궁금한 내용보다는 어떤 점이 어떻게 바뀌었는지 위주로 말씀드리겠습니다.

앱 바로 실행하기

스토리북 바로 실행하기

배포한 모달 라이브러리

신규 및 수정된 기능

  • 신용카드 목록 초기화 버튼 추가

    • 메인 화면에서 신용카드 목록을 초기화 하는 버튼(휴지통)을 추가하였습니다. 기능 요구 사항에는 없지만 매번 로컬스토리지를 수동으로 비워주는 것이 불편하여 구현해놨습니다.
  • 카드 조회/등록 기능에 mockAPI 연결

    • 실제 서비스 환경이라면 모든 과정은 서버와 연동이 될 것이고, 반드시 딜레이가 발생할 수 밖에 없다고 생각했습니다. 또, 이번 스텝에서 구현해야 하는 로딩 스피너를 구현하기 위해서는 반드시 지연시간을 발생시켜야 했습니다. 따라서 모든 데이터 처리는 mockAPI라는 파일을 거치게 수정하였습니다. 기존에는 로컬스토리지에 단순히 데이터를 파킹하는 정도의 처리를 하였다면, 이제는 그 과정에서 반드시 지연시간(0.5~2초)이 발생합니다. 이 가상의 API는 실제의 환경처럼 지연시간을 발생시키는 흉내를 내고, 그 결과를 반환하는 역할을 하도록 했습니다.
  • 로딩 화면 표시

    • 앞서 말씀드린 것 처럼 데이터가 저장/수정/삭제 되는 모든 과정에서 지연시간이 발생하게 되는데, 이 때 로딩화면이 보이게 됩니다.
  • 카드의 정보가 비어있을 때 기본 정보를 출력하도록 기능 추가

    • 기존에는 카드의 내용이 비어있을 때 텍스트 부분이 비어있었으나, 이제는 예제를 출력하도록 변경하였습니다.
  • 프로젝트 구조 일부 개선

    • Layout과 Provider가 Story에서도 사용되는 경우들이 생겨 모듈화 하였습니다.
  • 모달 관리 주체 변경

    • 모달을 앞으로 React.Portal에서 관리하도록 변경하였습니다.

기타

  • 스토리북 개선

    • 스토리북을 chromatic에 배포하였습니다. 이 과정도 github page와 마찬가지로 push하는 경우 자동으로 배포되도록 설정하였습니다.
    • 스토리북에 인터렉션 기능을 넣었습니다.
  • 개발 환경을 vscode에서 webstorm으로 이전하는 과정에서 필수 설정 파일 몇 개가 프로젝트에 추가되었습니다.

gabrielyoon7 and others added 30 commits April 18, 2023 14:28
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
- CreditCardRegisterLayout를 특정 페이지에만 적용하도록 구조 개선
- 주소 접근 방식 변경
@gabrielyoon7 gabrielyoon7 changed the title Step3 [페이먼츠 미션 Step 3] 가브리엘(윤주현) 미션 제출합니다. May 6, 2023
@HyeonaKwon HyeonaKwon self-requested a review May 8, 2023 01:59
@HyeonaKwon HyeonaKwon self-assigned this May 8, 2023
Copy link

@HyeonaKwon HyeonaKwon left a comment

Choose a reason for hiding this comment

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

안녕하세요 가브리엘!

오우 이번 미션에서는 별다른 코멘트가 없어서 바로 머지해도 될 것 같네요! 👍👍
스토리북 render, play 나눠서 구현하신 것도 보기 좋았구요. Outlet 사용하신 것도 좋았고 배포된 사이트에서 확인한 로딩화면도 잘 동작하는 것 확인했구요, UX 에 고민하신 것도 보기 좋았습니다! 하나만 추가로 말하자면 isLoading 과 관련해서 React 의 Suspense 에 대해 공부해보시면 좋을 것 같네요!

이제 리액트에 어느정도 감이 오시나요?! 잘 성장하고 계신 것 같아 보기 좋습니다 ㅎㅎ 고생 많으셨습니다!

Comment on lines +46 to +49
if (targetIndex !== -1) {
copiedCreditCards[targetIndex].nickname = newNickname;
localStorage.setItem('creditCards', JSON.stringify(copiedCreditCards));
}

Choose a reason for hiding this comment

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

index 가 -1 이면 에러를 반환해주는 것도 좋았겠네요!

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 그렇네요..!

</S.CreditCardNumber>
<S.CreditCardContainer>
<S.CreditCardBox>{owner}</S.CreditCardBox>
<S.CreditCardBox>{markExpiry(expiry)}</S.CreditCardBox>
<S.CreditCardBox>{(owner !== undefined) && (owner !== '') ? owner : 'WOOWA COURSE'}</S.CreditCardBox>

Choose a reason for hiding this comment

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

Suggested change
<S.CreditCardBox>{(owner !== undefined) && (owner !== '') ? owner : 'WOOWA COURSE'}</S.CreditCardBox>
<S.CreditCardBox>{owner || 'WOOWA COURSE'}</S.CreditCardBox>

요렇게 커버가 안되는 케이스가 있었나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

수정해봤는데 동일한 결과가 출력됩니다...! 제안해주신 코드가 훨씬 간결해보이네요

Comment on lines +55 to +60
creditCard={{
companyId: creditCardForm.companyId,
number: creditCardForm.number,
expiry: creditCardForm.expiry,
owner: creditCardForm.owner,
}}

Choose a reason for hiding this comment

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

Suggested change
creditCard={{
companyId: creditCardForm.companyId,
number: creditCardForm.number,
expiry: creditCardForm.expiry,
owner: creditCardForm.owner,
}}
creditCard={...creditCardForm}

이렇게 해도 되나요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

            <CreditCard
              fullFilled={false}
              creditCard={creditCardForm}
            />

이렇게 수정하니깐 동일하게 동작합니다

@HyeonaKwon HyeonaKwon merged commit d70bc6e into woowacourse:gabrielyoon7 May 8, 2023
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.

2 participants