[1단계 - 페이먼츠] 라이언(박한영) 미션 제출합니다. #366

Parkhanyoung · 2024-04-18T08:54:32Z

안녕하세요 케빈, 만나서 반갑습니다~! 😄
일주일 방학이 순식간에 지나가고 레벨 2가 시작되어 정신이 없네요 😂
케빈은 잘 지내고 계신가요~?
리액트 사용 경험이 부족해 아직 부족하지만 열심히 배우겠습니다!
이번 페이먼츠 미션 잘 부탁드리겠습니다 🙇🏻‍♂️

실행 방법

로컬: 1) npm i 2) npm run dev
배포

프로덕트: 바로가기
스토리북: 바로가기

컴포넌트 소개

CardPreview: 입력한 카드 정보를 담아 보여주는 프리뷰 컴포넌트
CardNumbersContainer: 카드번호 입력 섹션을 담은 컴포넌트
CardholderNameContainer: 카드 소유자명 입력 섹션을 담은 컴포넌트
CardExpiryDateContainer: 카드 유효기간 입력 섹션을 담은 컴포넌트
common/Input: 공통 input 컴포넌트
common/RegistrationLayout: 입력 받는 양식을 재사용하기 위한 컴포넌트

이런 점을 신경썼어요

UI/비즈니스 로직 분리: UI 로직(컴포넌트)에서 비즈니스 로직을 최대한 분리하고, 컴포넌트는 정보를 보여주는 일에만 집중할 수 있도록 설계하려 노력했습니다.
유효성 검증/에러 메시지 처리: 유효성 검증 및 에러 메시지 처리가 생각보다 복잡했습니다. 이를 최대한 유지보수하기 좋은 형태로 설계하려 고민했습니다!

질문 사항

유효성 검증/에러 메시지 처리 로직: 나름대로 고민해서 열심히 처리했지만, 한편으로는 적절한 처리였을까하는 의문이 남기도 했습니다. 케빈에게 납득이 가능한 구조였는지, 만약 아니었다면 어떤 점에서 아쉬웠는지 혹은 어떻게 개선할 수 있을지 조언을 구하고 싶습니다!
프론트엔드 로직의 복잡성을 극복하는 노하우: 컴포넌트 로직을 짜다보니 간단한 페이지였음에도 로직이 복잡했습니다. 프론트엔드 코드는 처리해야 하는 로직의 유형이 다양하고, 처리하는 정보의 종류가 다양하다보니 본질적으로 복잡할 수밖에 없겠다는 생각이 들었습니다. 케빈도 일을 하시면서 코드의 복잡성으로 고민하신 적이 있으실 것 같은데, 혹시 케빈에게 프론트엔드에서의 복잡성을 극복하기 위한 팁이 있는지, 혹은 꼭 지키려 하는 원칙이 있는지 궁금합니다.
실무에서의 스토리북: 실무에서 스토리북을 적극적으로 활용하는 추세인지 궁금합니다! (만약 스토리북을 활용하신다면) 스토리북의 사용 목적이 여러 가지가 있는 것 같은데(비개발 동료와의 협업 / 독립적인 컴포넌트 생성을 고민하게 됨 / 컴포넌트 단위의 빠른 피드백 등), 그 중 어떤 목적이 가장 크다고 생각하시는지도 궁금합니다. 스토리북을 만드는 데 추가 코드를 많이 작성해야 하고 그에 따른 리소스 소모도 꽤 발생할 것 같아서, 사용한다면 그 이유를 확실하게 알고 사용하는 게 맞는 것 같아서 질문 드립니다!

리뷰를 위해 긴 글과 코드 읽어주셔서 감사드립니다 🙂

Co-authored-by: Parkhanyoung <[email protected]>

Co-authored-by: Parkhanyoung <[email protected]> Co-authored-by: Parkhanyoung <[email protected]>

Co-authored-by: Parkhanyoung <[email protected]>

JeongBin0227

안녕하세요 라이언~

어떤것을 고민하고 있는지 잘 적어주셔서 어떤 포인트로 리뷰를 드려야할지 명확해서 좋았습니다! 추가적으로 코드적인 고민도 좋지만, 사용자 경험측면도 함께 고민해보시면 실력향상에 더 도움이 될거 같아요!

질문 주신 부분들 코멘트 남겼는데 혹시 이해가 안되는 부분이 있으면 언제든지 편하게 물어봐주세요~
미션하느라 고생하셨습니다 👍 💯

JeongBin0227 · 2024-04-19T07:59:09Z

src/App.tsx

+
+const CardInfoWrapper = styled.section`
+  margin-top: 50px;
+`;

 export default App;


해당 파일은 App 컴포넌트인데, App 컴포넌트 역할에 비해 상태 관리와 로직 처리를 포함하여 많은 책임을 지고 있는거 같아요.
이러한 복잡성을 줄이기 위해, 관련 로직을 더 작은 함수나 커스텀 훅으로 분리할 수 있습니다.

예를 들어, 카드 관련 정보 처리를 전담하는 커스텀 훅을 만들 수 있을거 같네요

처음에 코드 읽을때, 잘 안읽혀서 원인을 고민을 해봤는데, useInput과 useInputs 훅 사용에 있어서 반환 값의 구조가 서로 달라 헷갈렸던거 같아요. 비슷한 훅이고 네이밍도 비슷하지만 일관성 없어 비슷한 형태의 반환 값을 가지도록 표준화하면 어떨까요

✅ 반영 완료 & ❓ 질문

두 코멘트 모두 크게 공감이 됐고, 적극 반영하려 노력했습니다.

비슷한 형태의 반환 값을 가지도록 표준화하면 어떨까요

먼저, useInput/useInputs는 useValidation/useValidations로 대체했습니다.
그리고 useValidation과 useValidations는 동일한 형태의 결과를 반환하도록 수정했습니다.

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체

카드 관련 정보 처리를 전담하는 커스텀 훅을 만들 수 있을거 같네요

카드 정보를 전담하는 useCardInfo를 생성해, App은 useCardInfo에만 의존하도록 수정했습니다.
케빈이 의도하신 방향과 일치하는 리팩토링이었는지, 혹은 이 리팩터링에서 아쉽다고 느끼신 점은 없는지 궁금합니다. 🤔

관련 커밋
[commit 3719a2] refactor: container 컴포넌트들의 interface를 통일 / 카드번호, 유효기간, 소유자명에 대한 커스텀훅을 묶어주는 useCardInfo 추가

딱 제가 의도한 방향이였습니다. 동일한 형태의 결과를 반환해서 사용하는 측면에서 좀더 쉽게 사용할 수 있을거라 생각했습니다.

카드를 전담하는 useCardInfo 를 만든것도 좋은거 같아요

JeongBin0227 · 2024-04-19T08:00:59Z

src/App.tsx

+    errorMessage: cardNumbersErrorMessage,
+    errorStatus: cardNumbersErrorStatus,


라이언이 생각하는 errorMessage 와 errorStatus 차이는 뭘까요?

✅ 반영 완료

// errorStatus (카드 번호의 각 필드 별 오류 여부를 저장) { first: false, second: false, third: false, fourth: false } // errorMessage (카드 번호의 관련 에러 메시지) "카드번호는 네 자리여야 합니다."

errorStatus는 에러 여부를 카드 번호 각 자리 별로 관리하기 위해 만들었던 객체입니다.
errorMessage는 말그대로 에러 메시지 스트링인데, 메시지는 모든 자리를 통틀어 하나로만 보여지기 객체가 아닌 스트링으로 관리했습니다.

다만 케빈이 말씀해주신 것처럼,

useValidation과 useValidations의 인터페이스가 달라 혼동된 점

변수명만으로 쉽게 차이가 구별되지 않아 혼동된 점

을 고려해 다음과 같이 수정했습니다.

// errorStatus { isError: { first: false, second: false, third: false, fourth: false }, errorMessage: "카드번호는 네 자리여야 합니다." }

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체

JeongBin0227 · 2024-04-19T08:06:43Z

src/App.tsx

+  const {
+    value: expiryMonth,
+    handleChange: handleChangeExpiryMonth,
+    updateErrorMessage: updateExpiryMonthErrorMessage,
+    errorMessage: expiryMonthErrorMessage,
+  } = useInput('', inquireExpiryMonth);
+
+  const {
+    value: expiryYear,
+    handleChange: handleChangeExpiryYear,
+    updateErrorMessage: updateExpiryYearErrorMessage,
+    errorMessage: expiryYearErrorMessage,
+  } = useInput('', inquireExpiryYear);


유효기간을 나타내는 month와 year를 별도의 상태로 관리하는 것은 적절할거 같아요
당연한 얘기지만 month 와 year은 서로 다른 유효성 검증 로직이 필요하고 각각 독립적인 필드로 존재하기 때문에 독립적으로 상태를 관리해야한다 생각합니다.

이렇게 하면 만약 다른 곳에서 월 또는 년도 입력만 필요로 하는 경우, 이미 분리된 상태로 각각의 컴포넌트를 재사용할 수 있고, 상태 업데이트 로직에서도 둘다 바뀌지 않고 하나만 변경해도 됨으로 성능측면에도 좋을거같아요

JeongBin0227 · 2024-04-19T08:09:12Z

src/App.tsx

-    <>
-      <h1>React Payments</h1>
-    </>
+    <AppLayout>


StyledComponent의 이름을 짓는 것은 일관성을 유지하는게 제일 중요하다고 생각합니다.
저는 보통 모든 외부 컨테이너는 Container로 끝나고, 내부 레이아웃을 위한 컨테이너는 Wrapper로 끝나도록 사용합니다.

저도 예전에 이런 고민들이 많아, 유명 라이브러리들을 보며 영감을 얻거나,
https://css-tricks.com/ , https://www.smashingmagazine.com/ 등을 많이 참고해본거 같아요

JeongBin0227 · 2024-04-19T08:12:35Z

src/components/CardPreview.tsx

+          {isVisa && <StyledImage src={VisaCard} />}
+          {isMaster && <StyledImage src={MasterCard} />}


각각의 이미지 컴포넌트를 렌더링하는 시점에서 매번 확인되는데, 이미지를 결정하는 로직을 하나의 함수나 변수로 둬도 괜찮을거 같네요

✅ 반영 완료 & ❓질문

브랜드 로고 결정을 담당하는 별도의 컴포넌트를 생성했습니다.
그 안에서 getCardBrandLogo 함수를 통해 이미지를 결정합니다.

const getCardBrandLogo = (firstTwoDigits: string): LogoImageSrc | null => { if (firstTwoDigits[0] === VISA_FIRST_DIGIT) { return VisaCardLogo; } else if ( Number(firstTwoDigits) >= MIN_MASTER_FIRST_TWO_DIGITS && Number(firstTwoDigits) <= MAX_MASTER_FIRST_TWO_DIGITS ) { return MasterCardLogo; } return null; };

다만 이렇게 수정해도 책임이 더 명확히 나뉠 뿐 렌더링하는 시점에 매번 확인하는 것은 똑같다고 생각하는데, 혹시 케빈도 이런 방향의 수정을 생각하신 게 맞을까요?

관련 커밋
[commit 4a29bc] refactor(CardPreview): 카드 브랜드 로고를 별도의 컴포넌트로 분리

네 매번 시점에서 관리하지만 앞으로 새로운 이미지가 추가되도 getCardBrandLogo 함수에서만 수정할수 있고,

{isVisa && <StyledImage src={VisaCard} />} {isMaster && <StyledImage src={MasterCard} />}

이런 코드도 없어서 유지보수에 훨씬 좋다고 생각했습니다

JeongBin0227 · 2024-04-19T08:16:36Z

src/hooks/useInputs.ts

+
+type ErrorMessage = string;
+
+const convertArrayIntoObject = (keys: string[]) => {


2단계에서 분리해줄거라 기대하고 있습니다~

JeongBin0227 · 2024-04-19T08:18:57Z

src/hooks/useInputs.ts

+      ...value,
+      [targetKey]: e.target.value,
+    });
+  };


컴포넌트가 렌더링될 때마다 새로운 함수를 반환하는데, useCallback 을 사용해서 메모이제이션해보면 어떨까요

✅ 반영 완료

useInput이 없어진 관계로 useValidation에서 updateErrorStatus 함수를 생성할 때 useCallback을 적용했습니다.

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체

JeongBin0227 · 2024-04-19T08:19:32Z

src/hooks/useInputs.ts

+
+type ErrorMessage = string;
+
+const convertArrayIntoObject = (keys: string[]) => {


추가로 forEach 보다는 reduce를 사용하면 더 간결하게 표현될거 같아요

JeongBin0227 · 2024-04-19T08:20:21Z

src/inquiry/inquireExpiryYear.ts

+const inquireExpiryYear = (expiryYear: string) => {
+  const isValidLength = expiryYear.length === 0 || expiryYear.length === 2;
+  const isValidYear = Number(expiryYear) > 23 && Number(expiryYear) < 41;


매직 넘버보다는 이런 값들을 변수로 관리하면 어떨까요

✅ 반영 완료

유효성 검증 관련 매직넘버를 모두 변수로 선언했습니다.

관련 커밋
[commit e0b929] refactor(validator): 유효성 검증 관련 상수를 변수로 선언

JeongBin0227 · 2024-04-19T08:26:24Z

src/inquiry/inquireCardNumber.ts

@@ -0,0 +1,16 @@
+const inquireCardNumber = (cardNumber: string) => {


유효성 검사와 오류 메시지 반환을 동시에 처리하는건 일반적이지만,빈 문자열('')을 사용하여 에러가 없음을 나타내는 방식에는 확장성도 떨어지고, 명시적이지 않아 가독성이 떨어져 다른 방법이 더 좋을거 같습니다.

메시지와 함께 오류 플래그(boolean)를 사용해서 반환하는게 더 좋아보입니다. 아니면 JavaScript의 Error 객체를 사용하여 에러 상황을 표현하고, 에러가 없는 경우 null을 반환하는 방법도 있겠네요

… 묶어주는 useCardInfo 추가

- useValidation/useValidations의 updateErrorStatus 메서드 인터페이스 수정(검증 대상 값을 인자로도 받을 수 있도록)

…t으로 수정

Parkhanyoung

1차 리뷰에 좋은 코멘트를 많이 남겨주셔서, 좋은 고민들을 많이 해볼 수 있었습니다!
케빈 덕분에 스스로는 보지 못했던 많은 개선 사항을 발견할 수 있었습니다.
너무 너무 감사드립니다 케빈 🙂 🚀

질문은 케빈이 남겨주신 코멘트에 대한 답변과 함께 달아두었습니다!
그리고, 끝으로 케빈이 괜찮으시다면 질문 하나만 더 드리고 싶은데요.

Q. CardPreview에서 ExpiryDate와 CardNumber를 별도의 컴포넌트로 분리했는데, 이것이 과도한 분리로 느껴지셨는지 궁금합니다!
컴포넌트를 나누는 단위가 중요하다고 생각하는데요. 저는 연산이 포함되는 부분이거나 포함된 Element의 숫자가 많다면, 크기가 그리 크지 않더라도 별도의 컴포넌트로 분리하는 게 좋다고 판단했습니다. 하나의 컴포넌트가 최소한의 일에 집중하는 형태가 되어야, 이후의 기능 추가나 수정이 원활할 것이라고 생각했기 때문입니다. 그래서 CardPreview에서도 CardNumber와 ExpiryDate를 분리했습니다. 그런데 한편으로는 누군가에게는 이것이 과도한 분리라고 느껴질 수 있겠다는 우려가 들었습니다. 케빈이 보셨을 땐 이 정도의 분리가 적당했나요?

혹시 수정 사항을 보시면서 부족한 점이 있다면 편히 말씀해주시면 감사하겠습니다!
리뷰를 위해 긴 글과 코드를 읽어주셔서 다시 한 번 감사드립니다. 🙇🏻‍♂️

+ 변경 사항 반영하여 재배포도 해두었습니다 ⚙️

Parkhanyoung · 2024-04-21T11:22:36Z

src/App.tsx

+    errorMessage: cardNumbersErrorMessage,
+    errorStatus: cardNumbersErrorStatus,


✅ 반영 완료

// errorStatus (카드 번호의 각 필드 별 오류 여부를 저장) { first: false, second: false, third: false, fourth: false } // errorMessage (카드 번호의 관련 에러 메시지) "카드번호는 네 자리여야 합니다."

errorStatus는 에러 여부를 카드 번호 각 자리 별로 관리하기 위해 만들었던 객체입니다.
errorMessage는 말그대로 에러 메시지 스트링인데, 메시지는 모든 자리를 통틀어 하나로만 보여지기 객체가 아닌 스트링으로 관리했습니다.

다만 케빈이 말씀해주신 것처럼,

useValidation과 useValidations의 인터페이스가 달라 혼동된 점

변수명만으로 쉽게 차이가 구별되지 않아 혼동된 점

을 고려해 다음과 같이 수정했습니다.

// errorStatus { isError: { first: false, second: false, third: false, fourth: false }, errorMessage: "카드번호는 네 자리여야 합니다." }

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체

Parkhanyoung · 2024-04-21T11:33:06Z

src/App.tsx

+
+const CardInfoWrapper = styled.section`
+  margin-top: 50px;
+`;

 export default App;


✅ 반영 완료 & ❓ 질문

두 코멘트 모두 크게 공감이 됐고, 적극 반영하려 노력했습니다.

비슷한 형태의 반환 값을 가지도록 표준화하면 어떨까요

먼저, useInput/useInputs는 useValidation/useValidations로 대체했습니다.
그리고 useValidation과 useValidations는 동일한 형태의 결과를 반환하도록 수정했습니다.

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체

카드 관련 정보 처리를 전담하는 커스텀 훅을 만들 수 있을거 같네요

카드 정보를 전담하는 useCardInfo를 생성해, App은 useCardInfo에만 의존하도록 수정했습니다.
케빈이 의도하신 방향과 일치하는 리팩토링이었는지, 혹은 이 리팩터링에서 아쉽다고 느끼신 점은 없는지 궁금합니다. 🤔

관련 커밋
[commit 3719a2] refactor: container 컴포넌트들의 interface를 통일 / 카드번호, 유효기간, 소유자명에 대한 커스텀훅을 묶어주는 useCardInfo 추가

Parkhanyoung · 2024-04-21T11:39:17Z

src/components/CardPreview.tsx

+          {isVisa && <StyledImage src={VisaCard} />}
+          {isMaster && <StyledImage src={MasterCard} />}


✅ 반영 완료 & ❓질문

브랜드 로고 결정을 담당하는 별도의 컴포넌트를 생성했습니다.
그 안에서 getCardBrandLogo 함수를 통해 이미지를 결정합니다.

const getCardBrandLogo = (firstTwoDigits: string): LogoImageSrc | null => { if (firstTwoDigits[0] === VISA_FIRST_DIGIT) { return VisaCardLogo; } else if ( Number(firstTwoDigits) >= MIN_MASTER_FIRST_TWO_DIGITS && Number(firstTwoDigits) <= MAX_MASTER_FIRST_TWO_DIGITS ) { return MasterCardLogo; } return null; };

다만 이렇게 수정해도 책임이 더 명확히 나뉠 뿐 렌더링하는 시점에 매번 확인하는 것은 똑같다고 생각하는데, 혹시 케빈도 이런 방향의 수정을 생각하신 게 맞을까요?

관련 커밋
[commit 4a29bc] refactor(CardPreview): 카드 브랜드 로고를 별도의 컴포넌트로 분리

Parkhanyoung · 2024-04-21T11:56:09Z

src/components/common/RegistrationLayout.tsx

+  font-size: 13px;
+  font-weight: 300;
+  margin: 10px 0 12px 0;
+  color: #8b95a1;


✅ 약간 반영 완료 & ❓ 약간의 고민

반복하여 사용되는 색상 코드에 한해 변수로 선언했습니다.
그런데 한 번만 사용되는 색상 코드를 모두 변수로 선언하는 것에 대해서는 고민이 되어 일단 변수로 선언하지 않았습니다.

그 이유는,

만약 색상 코드를 모두 변수로 선언한다고 하면, 하나도 빠짐 없이 변수로 선언해주어야 할 텐데, 미묘하게 다른 색이 너무 많다(ex. 검정색, 약간 연한 검정색, 약간 더 연한 검정색, 진한 회색 ...). 이 모두를 다 이름 붙여줄 수 있을까? 그렇게 미묘하게 다른 이름들로 제대로 식별이 될까?

vscode 상에서 색상 코드 옆에 색상 블록이 보여지기 때문에 그것으로 색을 파악할 수 있다. 물론, 깃허브에서는 안 보이지만 깃허브에서 색상까지 파악해야 할 필요성은 잘 모르겠다.

그래서 고민 끝에 반복되는 색상 코드에 한해서만 변수로 선언했습니다.
이에 대한 케빈의 의견이 궁금합니다! 🤔

관련 커밋
[commit b55be3] refactor(Input): 반복 사용되는 색상 코드를 변수로 선언

Parkhanyoung · 2024-04-21T12:10:07Z

src/hooks/useInput.ts

+
+type ErrorMessage = string;
+
+const useInput = (initialValue = '', inquireValidity?: (value: string) => ErrorMessage) => {


✅ 반영 완료
너무 동의되는 코멘트였고, 반영 완료했습니다!

useInput이 많은 책임을 맡고 있어 함수의 초점이 또렷하지 않았습니다.
이에 useInput을 없애고, useValidation이라는 간소화된 hook을 만들었습니다.

const useValidation = (state: string, validate: TValidate) => { const [errorStatus, setErrorStatus] = useState<IErrorStatus>({ errorMessage: '', isError: false, }); const updateErrorStatus = useCallback( (targetValue: string = state) => { setErrorStatus(validate(targetValue)); }, [state, validate], ); return { errorStatus, updateErrorStatus }; };

useValidation은 state와 validate 함수를 인자로 받아, 에러 상태 및 에러 상태 업데이트 함수를 반환합니다.
이로써 유효성 검증 및 에러 상태 관리만을 담당하는 초점이 보다 명확한 hook이 되었습니다.

혹시 이 리팩토링에 대해서 유지보수성이나 성능 면에서 아쉬운 점이 있다면 가감없이 공유해주시면 감사드리겠습니다 🙇🏻‍♂️

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체

Parkhanyoung · 2024-04-21T12:15:20Z

src/hooks/useInputs.ts

+
+type ErrorMessage = string;
+
+const convertArrayIntoObject = (keys: string[]) => {


✅ 반영 완료

함수 분리 및 forEach -> reduce 변경 완료했습니다!

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체
[commit 78905d] refactor(convertKeysIntoObject): forEach가 아닌 reduce를 사용하는 방식으로 수정

Parkhanyoung · 2024-04-21T12:16:32Z

src/hooks/useInputs.ts

+      ...value,
+      [targetKey]: e.target.value,
+    });
+  };


✅ 반영 완료

useInput이 없어진 관계로 useValidation에서 updateErrorStatus 함수를 생성할 때 useCallback을 적용했습니다.

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체

Parkhanyoung · 2024-04-21T12:19:24Z

src/inquiry/inquireCardNumber.ts

@@ -0,0 +1,16 @@
+const inquireCardNumber = (cardNumber: string) => {


✅ 반영 완료

메시지와 함께 isError라는 오류 플래그를 두어 명확하게 표현했습니다.

{ isError: true, errorMessage: '카드 번호는 4자리로 입력해주세요' };

관련 커밋
[commit 720406] refactor: useInput/useInputs를 useValidation/useValidations로 대체

Parkhanyoung · 2024-04-21T12:37:04Z

src/inquiry/inquireExpiryYear.ts

+const inquireExpiryYear = (expiryYear: string) => {
+  const isValidLength = expiryYear.length === 0 || expiryYear.length === 2;
+  const isValidYear = Number(expiryYear) > 23 && Number(expiryYear) < 41;


✅ 반영 완료

유효성 검증 관련 매직넘버를 모두 변수로 선언했습니다.

관련 커밋
[commit e0b929] refactor(validator): 유효성 검증 관련 상수를 변수로 선언

Parkhanyoung · 2024-04-21T13:03:40Z

중요한건 아니지만 사용자 경험을 위해서 2로 입력해도 02로 변환되면 좋을거 같아요

✅ 반영 완료

월 입력창 blur 시 현재 입력 값이 한 자리 숫자라면, 앞에 '0'을 추가하도록 수정했습니다.

관련 커밋

[commit 7c5131] refactor: 유효기간 월(月) 입력 창 blur 시, 한 자리일 경우 두 자리로 자동 변경 처리

이제 이런 구조에서 다른 검증이나 추가적인 로직이 필요할때 어떻게 변경할지 궁금하네요.
2,3단계에서도 확인 가능하겠지만, 라이언이 생각하는 수정 방안을 말해줘도 좋을거같아요

만약 이미 존재하는 필드에 대해 검증 로직이 추가/수정되어야 한다면, 해당 필드에 대한 validate 함수를 수정할 것 같습니다.
만약 새로운 필드가 추가된다면, 해당 필드에 대한 validate 함수를 생성하고 그것을 useValidation과 함께 사용하여 useSomeField와 같은 형태의 hook을 추가하여 사용할 것 같습니다.
케빈이 궁금하신 맥락에서 적절하게 답변을 드린 건지 잘 모르겠네요. 혹시 다른 의도로 질문주신 거라면 편히 말씀해주세요! 🙂

JeongBin0227 · 2024-04-22T12:52:26Z

CardPreview에서 ExpiryDate와 CardNumber를 별도의 컴포넌트로 분리했는데, 이것이 과도한 분리로 느껴지셨는지 궁금합니다!
컴포넌트를 나누는 단위가 중요하다고 생각하는데요. 저는 연산이 포함되는 부분이거나 포함된 Element의 숫자가 많다면, 크기가 그리 크지 않더라도 별도의 컴포넌트로 분리하는 게 좋다고 판단했습니다. 하나의 컴포넌트가 최소한의 일에 집중하는 형태가 되어야, 이후의 기능 추가나 수정이 원활할 것이라고 생각했기 때문입니다. 그래서 CardPreview에서도 CardNumber와 ExpiryDate를 분리했습니다. 그런데 한편으로는 누군가에게는 이것이 과도한 분리라고 느껴질 수 있겠다는 우려가 들었습니다. 케빈이 보셨을 땐 이 정도의 분리가 적당했나요?

저는 오히려 적절한 분리였다고 생각합니다. CardNumber 와 ExpiryDate 를 다른 값이고 비지니스 로직도 다르게 들어가야하기 때문에 관심 분리나 유지보수 측면으로 봐도 분리하는게 더 좋았다고 생각합니다.

JeongBin0227 · 2024-04-22T12:54:06Z

이미 존재하는 필드에 대해 검증 로직이 추가/수정되어야 한다면, 해당 필드에 대한 validate 함수를 수정할 것 같습니다.
만약 새로운 필드가 추가된다면, 해당 필드에 대한 validate 함수를 생성하고 그것을 useValidation과 함께 사용하여 useSomeField와 같은 형태의 hook을 추가하여 사용할 것 같습니다.

이부분은 지금 얘기하는것보다 2,3단계 미션에서 다시 말해보는것도 더 의미가 있을거 같아요~

JeongBin0227

안녕하세요 라이언!

코멘트 마다 커밋을 남겨주셔서 쉽게 확인할수 있었습니다 감사합니다 👍
수정한 부분 모두 확인하였고, 큰 이견이 없는 부분은 넘어가고 추가 코멘트나 질문에 대해 답변 달았으니 확인해보시면 좋을거 같아요

1단계 미션하느라 고생하셨습니다~ 2단계에서 만나요 💯

rbgksqkr and others added 30 commits April 16, 2024 14:44

chore: prettier 설정

2213063

Co-authored-by: Parkhanyoung <[email protected]>

docs: 기능 요구 사항 분석 및 컴포넌트 설계 작성

890c4c3

Co-authored-by: Parkhanyoung <[email protected]>

chore(README): 오타 수정

b4f99b3

Co-authored-by: Parkhanyoung <[email protected]>

chore(prettier): 확장자 변경

0e697e1

Co-authored-by: Parkhanyoung <[email protected]>

chore(eslint): eslint에 prettier 플러그인 설정

565fdd0

Co-authored-by: Parkhanyoung <[email protected]>

feat(useInput): useInput 커스텀 훅 구현

c8840e2

Co-authored-by: Parkhanyoung <[email protected]>

feat(Input): Input 공통 컴포넌트 구현

33ee180

Co-authored-by: Parkhanyoung <[email protected]>

chore: styled-components 설치

bfcc7ef

Co-authored-by: Parkhanyoung <[email protected]>

docs(REQUIREMENTS): 기능 요구 사항 업데이트

6735000

Co-authored-by: Parkhanyoung <[email protected]>

refactor(useInput): 상태관리 및 에러처리 구조 개선

8545e93

Co-authored-by: Parkhanyoung <[email protected]>

refactor(Input): handleChange를 props로 받도록 수정 및 스타일링 추가

c86cd98

Co-authored-by: Parkhanyoung <[email protected]>

feat(App): 카드 소유자 이름 섹션 구현

6bca6fb

Co-authored-by: Parkhanyoung <[email protected]>

refactor(CardholderNameContainer): 컴포넌트 분리

702f5ca

Co-authored-by: Parkhanyoung <[email protected]>

feat(useInput): input blur 처리될 때유효성 검사

7b92fce

Co-authored-by: Parkhanyoung <[email protected]>

feat(CardholderNameContainer): onBlur 핸들러로updateErrorMessage 등록

c0152c4

Co-authored-by: Parkhanyoung <[email protected]>

feat(CardExpiryDateContainer): 컴포넌트 구현

f490168

Co-authored-by: Parkhanyoung <[email protected]>

refactor(Input): rest 파라미터 적용

c3f2a22

Co-authored-by: Parkhanyoung <[email protected]>

refactor(inquiry): inquire 함수 파일 분리

06d3432

Co-authored-by: Parkhanyoung <[email protected]>

feat(App): CardExpiryDateContainer 컴포넌트 추가

7954511

Co-authored-by: Parkhanyoung <[email protected]>

chore(eslint): any 타입 warn 처리

1535fdd

Co-authored-by: Parkhanyoung <[email protected]>

refactor(inquire): 유효기간 월과 년도의 유효성 검사 조건 수정

157b17e

Co-authored-by: Parkhanyoung <[email protected]>

feat(inquireCardNumber): 카드 번호 inquire 함수 구현

cc9658f

Co-authored-by: Parkhanyoung <[email protected]>

chore(inquireCardHolderName): 불필요한 입력값 길이 검증 로직 제거

89ca4c4

Co-authored-by: Parkhanyoung <[email protected]> Co-authored-by: Parkhanyoung <[email protected]>

chore(useInput): inquireValidity optional로 변경

219b8d0

Co-authored-by: Parkhanyoung <[email protected]>

feat(useInputs): 커스텀 훅 구현

a1fe1fc

Co-authored-by: Parkhanyoung <[email protected]>

feat(CardNumbersContainer): 컴포넌트 구현

2c13b28

Co-authored-by: Parkhanyoung <[email protected]>

refactor(common): ErrorText 스타일 분리

6c5efbe

Co-authored-by: Parkhanyoung <[email protected]>

feat(App): CardNumberContainer 컴포넌트 등록

8c8ca0e

Co-authored-by: Parkhanyoung <[email protected]>

docs(REQUIREMENTS): 구현사항 업데이트

cb5e35e

Co-authored-by: Parkhanyoung <[email protected]>

design(GlobalStyles): reset css 적용

864cf0b

Co-authored-by: Parkhanyoung <[email protected]>

JeongBin0227 requested changes Apr 19, 2024

View reviewed changes

Parkhanyoung added 23 commits April 20, 2024 19:52

refactor(Input): 타입 안정성을 위해 InputProps가InputHTMLAttributes를 상속받도록 수정

d9bc2bf

refactor: useInput/useInputs를 useValidation/useValidations로 대체

7204066

chore(gitignore): eol 추가

0ec98e5

refactor: container 컴포넌트들의 interface를 통일 / 카드번호, 유효기간, 소유자명에 대한 커스텀훅을…

3719a25

… 묶어주는 useCardInfo 추가

chore: 미사용 css 파일 제거

3e58dc3

refactor: 유효성 검증 로직의 네이밍 수정(inquire -> validate)

044882b

refactor(CardPreview): 카드 브랜드 로고를 별도의 컴포넌트로 분리

4a29bc5

refactor(CardPreview): 카드 숫자를 별도의 컴포넌트로 분리

31497e4

chore: 불필요한 css import 문 삭제

786ea1a

feat(CardExpiryDate, CardhokderName): placeholder 문구를 예시 입력으로 수정

45b603f

refactor(CardNumbersContainer): 상수 분리

ed85991

refactor: RegisrationLayout의 네이밍을 InputField로 수정

dfe9df6

fix(validateCardNumber): 카드 번호 유효성 검증을 숫자 4자리만 통과시키도록 수정

5a26bd9

refactor: 유효기간 월(月) 입력 창 blur 시, 한 자리일 경우 두 자리로 자동 변경 처리

7c51312

- useValidation/useValidations의 updateErrorStatus 메서드 인터페이스 수정(검증 대상 값을 인자로도 받을 수 있도록)

refactor(App): XXXContainer 형태의 네이밍을 XXXInputContainer로 수정

d9f3bb4

refactor(CardBrandLogo): 가독성을 위해 변수명 수정

97e5687

refactor(CardNumberContainer): 본래 의미에 더 부합하기 위해 type명 password를 secre…

93e73f6

…t으로 수정

refactor(CardPreview): ExpiryDate를 별도의 컴포넌트로 분리

d79f991

refactor(Input): 반복 사용되는 색상 코드를 변수로 선언

b55be32

refactor(App): 스타일 컴포넌트 CardInfoWrapper -> CardInfoInputWrapper 네이밍 수정

c6bcc66

refactor(convertKeysIntoObject): forEach가 아닌 reduce를 사용하는 방식으로 수정

78905d9

refactor(useValidation, useValidations): 첫 번째 인자 명을 value에서 state로 수정

15659de

refactor(validator): 유효성 검증 관련 상수를 변수로 선언

e0b929a

Parkhanyoung commented Apr 21, 2024

View reviewed changes

JeongBin0227 approved these changes Apr 22, 2024

View reviewed changes

JeongBin0227 merged commit ea512f0 into woowacourse:parkhanyoung Apr 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 페이먼츠] 라이언(박한영) 미션 제출합니다. #366

[1단계 - 페이먼츠] 라이언(박한영) 미션 제출합니다. #366

Parkhanyoung commented Apr 18, 2024 •

edited

Loading

JeongBin0227 left a comment

JeongBin0227 Apr 19, 2024

JeongBin0227 Apr 19, 2024

Parkhanyoung Apr 21, 2024 •

edited

Loading

JeongBin0227 Apr 22, 2024

JeongBin0227 Apr 19, 2024

Parkhanyoung Apr 21, 2024 •

edited

Loading

JeongBin0227 Apr 19, 2024

JeongBin0227 Apr 19, 2024

JeongBin0227 Apr 19, 2024

Parkhanyoung Apr 21, 2024

JeongBin0227 Apr 22, 2024

JeongBin0227 Apr 19, 2024

JeongBin0227 Apr 19, 2024

Parkhanyoung Apr 21, 2024

JeongBin0227 Apr 19, 2024

JeongBin0227 Apr 19, 2024

Parkhanyoung Apr 21, 2024

JeongBin0227 Apr 19, 2024

Parkhanyoung left a comment •

edited

Loading

Parkhanyoung Apr 21, 2024 •

edited

Loading

Parkhanyoung Apr 21, 2024 •

edited

Loading

Parkhanyoung Apr 21, 2024

Parkhanyoung Apr 21, 2024

Parkhanyoung Apr 21, 2024 •

edited

Loading

Parkhanyoung Apr 21, 2024

Parkhanyoung Apr 21, 2024

Parkhanyoung Apr 21, 2024

Parkhanyoung Apr 21, 2024

Parkhanyoung commented Apr 21, 2024

JeongBin0227 commented Apr 22, 2024

JeongBin0227 commented Apr 22, 2024

JeongBin0227 left a comment

		errorMessage: cardNumbersErrorMessage,
		errorStatus: cardNumbersErrorStatus,

		{isVisa && <StyledImage src={VisaCard} />}
		{isMaster && <StyledImage src={MasterCard} />}


		type ErrorMessage = string;

		const convertArrayIntoObject = (keys: string[]) => {

		@@ -0,0 +1,16 @@
		const inquireCardNumber = (cardNumber: string) => {


		type ErrorMessage = string;

		const useInput = (initialValue = '', inquireValidity?: (value: string) => ErrorMessage) => {

[1단계 - 페이먼츠] 라이언(박한영) 미션 제출합니다. #366

[1단계 - 페이먼츠] 라이언(박한영) 미션 제출합니다. #366

Conversation

Parkhanyoung commented Apr 18, 2024 • edited Loading

실행 방법

컴포넌트 소개

이런 점을 신경썼어요

질문 사항

JeongBin0227 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkhanyoung Apr 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkhanyoung Apr 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkhanyoung left a comment • edited Loading

Choose a reason for hiding this comment

Parkhanyoung Apr 21, 2024 • edited Loading

Choose a reason for hiding this comment

Parkhanyoung Apr 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkhanyoung Apr 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkhanyoung commented Apr 21, 2024

JeongBin0227 commented Apr 22, 2024

JeongBin0227 commented Apr 22, 2024

JeongBin0227 left a comment

Choose a reason for hiding this comment

Parkhanyoung commented Apr 18, 2024 •

edited

Loading

Parkhanyoung Apr 21, 2024 •

edited

Loading

Parkhanyoung Apr 21, 2024 •

edited

Loading

Parkhanyoung left a comment •

edited

Loading

Parkhanyoung Apr 21, 2024 •

edited

Loading

Parkhanyoung Apr 21, 2024 •

edited

Loading

Parkhanyoung Apr 21, 2024 •

edited

Loading