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

[넥스트레벨 Woobottle] 페이먼츠 미션 STEP1-1 #9

Open
wants to merge 7 commits into
base: woobottle
Choose a base branch
from

Conversation

woobottle
Copy link

@woobottle woobottle commented Jan 26, 2022

안녕하세요 인수님!!
리뷰 잘 부탁드려요!!

제가 인수님과 논의하고 싶은 부분들은 아래와 같습니다. (고민하던 시기에 적은 날것 그대로 적어두었습니다. 양해 한번만 부탁드릴게요!)

  • form elements 이동들 vs ref로 일일히 지정
  • 한 페이지에서 보여주는 추상화 레벨을 동일하게 하자(읽는이에게 편하게 하기 위해?)
  • 폴더 구조(page, component, share component)
  • 프로젝트내에서 css-in-js만 쓰든지 css만 쓰든지 통일하는게 좋은건지??(경우에 따라 다른것 같은데 정확한 기준이 혹시 있을지)
  • setter를 일일히 넘겨주는것이 반복되고 있는데 이걸 context provider를 이용하면 좋은건가?? 일일히 입력해주지 않아도 되서?? context provider를 적용하면 나중에 접했을 때 모를수도 있지 않을까? ==> 이래서 redux, recoil이 나온건가??
  • type과 interface중 일단은 interface로 전부 통일했는데 뭐가 어떤 상황에 쓰이는게 적절한지 알아야겠다.
  • cardNewPage에서 컴포넌트를 생성하는것을 굉장히 고민했는데 컴포넌트 자체의 생성 기준이 뭘지 찾아봐야겠다.
  • cardNumberInput 로직에 대한 개선

test영상

필수 요구사항

  • Storybook 상호 작용 테스트
  • 재사용 가능한 Component 작성

카드 추가

  • <(뒤로가기) 버튼 클릭 시, 카드 목록 페이지로 이동한다.
  • 카드 번호를 입력 받을 수 있다.
    • 카드 번호입력 폼에 라벨을 보여준다.
    • 카드 번호는 숫자만 입력가능하다.
    • 카드 번호 4자리마다 -가 삽입된다.
    • 카드 번호는 실시간으로 카드 UI에 반영된다.
    • 카드 번호는 앞 8자리만 숫자로 보여지고, 나머지 숫자는 *로 보여진다.
    • 카드 번호 앞 8자리로 카드사를 추정하여 그 테마를 카드 UI에 반영한다.
    • 카드사가 선택되고 유효한 카드 번호 16자리를 모두 입력하면, 자동으로 만료일로 focus된다.
    • 카드 앞 8자리 숫자를 입력하고 카드사가 선택되지 않은 경우, 나머지 카드 번호 입력 시도 시, 카드사 선택 모달이 보여진다.
    • 유효하지 않은 카드 번호를 입력하면, 입력 폼 아래에 에러 메시지를 보여준다.
  • 만료일을 입력 받을 수 있다.
    • 만료일 입력 폼에 라벨을 보여준다.
    • MM / YY 로 placeholder를 적용한다.
    • 월, 년 사이에 자동으로 /가 삽입된다.
    • 만료일은 실시간으로 카드 UI에 반영된다.
    • 월은 1이상 12이하 숫자여야 한다.
    • 월, 년 입력이 유효하면 보안코드 입력으로 focus된다
    • 유효하지 않은 만료일을 입력하면, 입력 폼 아래에 에러 메시지를 보여준다.
  • 보안코드를 입력 받을 수 있다.
    • 카드 보안코드 입력 폼에 라벨을 보여준다.
    • 보안코드는 *으로 보여진다.
    • 보안코드 3자리가 입력되면 카드 비밀번호 입력으로 focus된다.
    • 보안코드는 숫자만 입력가능하다.
    • 유효하지 않은 보안코드를 입력하면, 입력 폼 아래에 에러 메시지를 보여준다.
  • 카드 비밀번호의 앞 2자리를 입력 받을 수 있다.
    • 카드 비밀번호 입력 폼에 라벨을 보여준다.
    • 카드 비밀번호는 각 폼마다 한자리 숫자만 입력가능하다.
    • 첫자리 입력이 완료되면 둘째자리 입력으로 focus된다.
    • 카드 번호 입력 시, *으로 보여진다.
    • 유효하지 않은 카드 비밀번호를 입력하면, 입력 폼 아래에 에러 메시지를 보여준다.
  • 카드 소유자 이름을 입력 받을 수 있다.
    • 카드 소유자 이름 입력 폼 라벨을 보여준다.
    • 이름은 30자리까지 입력할 수 있다.
    • 이름 입력 폼 위에, 현재 입력 자릿수와 최대 입력 자릿수를 실시간으로 보여준다.
  • 클릭 시, 카드 등록 완료 페이지로 이동한다.

심화 요구사항 (선택사항)

  • Storybook 단위 테스트
  • 유효성 검증 실패에 대한 UI/UX 추가
  • 카드사 선택
    • 카드사를 선택하면 모달이 닫히고, 그 테마를 카드 UI에 반영한다.
    • 카드사를 선택하지 않아도 모달을 닫을 수 있다.
  • 보안코드 툴팁
    • 클릭 시, 보안코드 관련 안내 메시지를 보여준다.
    • focusout 시, 툴팁이 닫힌다.
  • 가상 키보드
    • 마스킹 처리된 값 입력시 사용
    • 숫자를 랜덤으로 배열

@woobottle woobottle requested a review from Outwater January 26, 2022 11:45
Copy link

@Outwater Outwater left a comment

Choose a reason for hiding this comment

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

우병님 코드 정말 잘 봤습니다.
저보다 훨씬 수준이 높아서 읽으면서도 많이 배우게 되었습니다. 감사합니다.

  • Typescript로 작성되어 있어서, 코드리뷰하기에 정말 편하다는 생각이 들었습니다. (저도 ts 리팩토링 고려해보아야겠습니다..ㅎㅎ)
  • 변수명에 공을 들이신 것이 그대로 느껴졌습니다. 전반적으로 네이밍에 통일성이 느껴져서, 나중에는 더욱 빠르게 이해할 수 있었습니다.
    • publicCardNumber, is--- , get---, ...
  • Utils, Validation 등 관심사 분리가 정말 잘 되어있다고 느꼈습니다. 작게 관심사 별로 구분되어 있어, 가독성과 유연성 모두 확보된 것 같습니다 👍

기능관련

  • 잘못된 값이 입력되었더라도, 다음페이지로 그대로 이동되는 것 같습니다.
    • string으로 cardNumber를 입력하여도, errorMsg는 뜨지만 다음 페이지 넘어가네요. (cvc값이 2자리만 입력이 되어도)

src/assets/styles/index.scss Outdated Show resolved Hide resolved
src/common/utils/index.ts Outdated Show resolved Hide resolved
src/components/card/Card.tsx Show resolved Hide resolved
src/components/card/index.ts Show resolved Hide resolved
src/components/card/inputs/CardNumberInput.tsx Outdated Show resolved Hide resolved
src/pages/card/new.tsx Outdated Show resolved Hide resolved
src/assets/styles/index.scss Outdated Show resolved Hide resolved
@joel-jo-querypie
Copy link
Member

안녕하세요 우병님 PR이 지금 nextlevel-2022:main from woobottle:woobottle 이런 상태인데 main 말고 woobottle 브랜치로 머지 부탁드릴게요~ 편하시라고 제가 만들어두겠습니다~

@woobottle woobottle changed the base branch from main to woobottle February 6, 2022 07:25
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